X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=blobdiff_plain;f=lib%2FTransforms%2FScalar%2FRewriteStatepointsForGC.cpp;h=db127c3f7b4ebe9d17efb81811a31de002c284f0;hp=7563cea9a0465f1a7309108d78e70f8fb5fe5033;hb=1ae1fbe339498bb2eb25c5f3526d396d1ace755f;hpb=3f945c295e6add83a10058f9ec85d36bdf97cf96 diff --git a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index 7563cea9a04..db127c3f7b4 100644 --- a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -21,6 +21,7 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/MapVector.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/CallSite.h" #include "llvm/IR/Dominators.h" @@ -71,6 +72,12 @@ static cl::opt ClobberNonLiveOverride("rs4gc-clobber-non-live", cl::location(ClobberNonLive), cl::Hidden); +static cl::opt UseDeoptBundles("rs4gc-use-deopt-bundles", cl::Hidden, + cl::init(false)); +static cl::opt + AllowStatepointWithNoDeoptInfo("rs4gc-allow-statepoint-with-no-deopt-info", + cl::Hidden, cl::init(true)); + namespace { struct RewriteStatepointsForGC : public ModulePass { static char ID; // Pass identification, replacement for typeid @@ -85,10 +92,10 @@ struct RewriteStatepointsForGC : public ModulePass { Changed |= runOnFunction(F); if (Changed) { - // stripDereferenceabilityInfo asserts that shouldRewriteStatepointsIn + // stripNonValidAttributes asserts that shouldRewriteStatepointsIn // returns true for at least one function in the module. Since at least // one function changed, we know that the precondition is satisfied. - stripDereferenceabilityInfo(M); + stripNonValidAttributes(M); } return Changed; @@ -105,15 +112,16 @@ struct RewriteStatepointsForGC : public ModulePass { /// dereferenceability that are no longer valid/correct after /// RewriteStatepointsForGC has run. This is because semantically, after /// RewriteStatepointsForGC runs, all calls to gc.statepoint "free" the entire - /// heap. stripDereferenceabilityInfo (conservatively) restores correctness + /// heap. stripNonValidAttributes (conservatively) restores correctness /// by erasing all attributes in the module that externally imply /// dereferenceability. - /// - void stripDereferenceabilityInfo(Module &M); + /// Similar reasoning also applies to the noalias attributes. gc.statepoint + /// can touch the entire heap including noalias objects. + void stripNonValidAttributes(Module &M); - // Helpers for stripDereferenceabilityInfo - void stripDereferenceabilityInfoFromBody(Function &F); - void stripDereferenceabilityInfoFromPrototype(Function &F); + // Helpers for stripNonValidAttributes + void stripNonValidAttributesFromBody(Function &F); + void stripNonValidAttributesFromPrototype(Function &F); }; } // namespace @@ -157,15 +165,16 @@ struct GCPtrLivenessData { // base relation will remain. Internally, we add a mixture of the two // types, then update all the second type to the first type typedef DenseMap DefiningValueMapTy; -typedef DenseSet StatepointLiveSetTy; -typedef DenseMap RematerializedValueMapTy; +typedef DenseSet StatepointLiveSetTy; +typedef DenseMap, AssertingVH> + RematerializedValueMapTy; struct PartiallyConstructedSafepointRecord { /// The set of values known to be live across this safepoint - StatepointLiveSetTy liveset; + StatepointLiveSetTy LiveSet; /// Mapping from live pointers to a base-defining-value - DenseMap PointerToBase; + DenseMap PointerToBase; /// The *new* gc.statepoint instruction itself. This produces the token /// that normal path gc.relocates and the gc.result are tied to. @@ -176,12 +185,26 @@ struct PartiallyConstructedSafepointRecord { Instruction *UnwindToken; /// Record live values we are rematerialized instead of relocating. - /// They are not included into 'liveset' field. + /// They are not included into 'LiveSet' field. /// Maps rematerialized copy to it's original value. RematerializedValueMapTy RematerializedValues; }; } +static ArrayRef GetDeoptBundleOperands(ImmutableCallSite CS) { + assert(UseDeoptBundles && "Should not be called otherwise!"); + + Optional DeoptBundle = CS.getOperandBundle("deopt"); + + if (!DeoptBundle.hasValue()) { + assert(AllowStatepointWithNoDeoptInfo && + "Found non-leaf call without deopt info!"); + return None; + } + + return DeoptBundle.getValue().Inputs; +} + /// Compute the live-in set for every basic block in the function static void computeLiveInValues(DominatorTree &DT, Function &F, GCPtrLivenessData &Data); @@ -192,7 +215,7 @@ static void findLiveSetAtInst(Instruction *inst, GCPtrLivenessData &Data, StatepointLiveSetTy &out); // TODO: Once we can get to the GCStrategy, this becomes -// Optional isGCManagedPointer(const Value *V) const override { +// Optional isGCManagedPointer(const Type *Ty) const override { static bool isGCPointerType(Type *T) { if (auto *PT = dyn_cast(T)) @@ -230,9 +253,8 @@ static bool containsGCPtrType(Type *Ty) { if (ArrayType *AT = dyn_cast(Ty)) return containsGCPtrType(AT->getElementType()); if (StructType *ST = dyn_cast(Ty)) - return std::any_of( - ST->subtypes().begin(), ST->subtypes().end(), - [](Type *SubType) { return containsGCPtrType(SubType); }); + return std::any_of(ST->subtypes().begin(), ST->subtypes().end(), + containsGCPtrType); return false; } @@ -244,7 +266,7 @@ static bool isUnhandledGCPointerType(Type *Ty) { } #endif -static bool order_by_name(llvm::Value *a, llvm::Value *b) { +static bool order_by_name(Value *a, Value *b) { if (a->hasName() && b->hasName()) { return -1 == a->getName().compare(b->getName()); } else if (a->hasName() && !b->hasName()) { @@ -257,6 +279,13 @@ static bool order_by_name(llvm::Value *a, llvm::Value *b) { } } +// Return the name of the value suffixed with the provided value, or if the +// value didn't have a name, the default value specified. +static std::string suffixed_name_or(Value *V, StringRef Suffix, + StringRef DefaultName) { + return V->hasName() ? (V->getName() + Suffix).str() : DefaultName.str(); +} + // Conservatively identifies any definitions which might be live at the // given instruction. The analysis is performed immediately before the // given instruction. Values defined by that instruction are not considered @@ -266,15 +295,15 @@ static void analyzeParsePointLiveness( const CallSite &CS, PartiallyConstructedSafepointRecord &result) { Instruction *inst = CS.getInstruction(); - StatepointLiveSetTy liveset; - findLiveSetAtInst(inst, OriginalLivenessData, liveset); + StatepointLiveSetTy LiveSet; + findLiveSetAtInst(inst, OriginalLivenessData, LiveSet); if (PrintLiveSet) { // Note: This output is used by several of the test cases // The order of elements in a set is not stable, put them in a vec and sort // by name SmallVector Temp; - Temp.insert(Temp.end(), liveset.begin(), liveset.end()); + Temp.insert(Temp.end(), LiveSet.begin(), LiveSet.end()); std::sort(Temp.begin(), Temp.end(), order_by_name); errs() << "Live Variables:\n"; for (Value *V : Temp) @@ -282,12 +311,40 @@ static void analyzeParsePointLiveness( } if (PrintLiveSetSize) { errs() << "Safepoint For: " << CS.getCalledValue()->getName() << "\n"; - errs() << "Number live values: " << liveset.size() << "\n"; + errs() << "Number live values: " << LiveSet.size() << "\n"; + } + result.LiveSet = LiveSet; +} + +static bool isKnownBaseResult(Value *V); +namespace { +/// A single base defining value - An immediate base defining value for an +/// instruction 'Def' is an input to 'Def' whose base is also a base of 'Def'. +/// For instructions which have multiple pointer [vector] inputs or that +/// transition between vector and scalar types, there is no immediate base +/// defining value. The 'base defining value' for 'Def' is the transitive +/// closure of this relation stopping at the first instruction which has no +/// immediate base defining value. The b.d.v. might itself be a base pointer, +/// but it can also be an arbitrary derived pointer. +struct BaseDefiningValueResult { + /// Contains the value which is the base defining value. + Value * const BDV; + /// True if the base defining value is also known to be an actual base + /// pointer. + const bool IsKnownBase; + BaseDefiningValueResult(Value *BDV, bool IsKnownBase) + : BDV(BDV), IsKnownBase(IsKnownBase) { +#ifndef NDEBUG + // Check consistency between new and old means of checking whether a BDV is + // a base. + bool MustBeBase = isKnownBaseResult(BDV); + assert(!MustBeBase || MustBeBase == IsKnownBase); +#endif } - result.liveset = liveset; +}; } -static Value *findBaseDefiningValue(Value *I); +static BaseDefiningValueResult findBaseDefiningValue(Value *I); /// Return a base defining value for the 'Index' element of the given vector /// instruction 'I'. If Index is null, returns a BDV for the entire vector @@ -298,8 +355,8 @@ static Value *findBaseDefiningValue(Value *I); /// vector returned is a BDV (and possibly a base) of the entire vector 'I'. /// If the later, the return pointer is a BDV (or possibly a base) for the /// particular element in 'I'. -static std::pair -findBaseDefiningValueOfVector(Value *I, Value *Index = nullptr) { +static BaseDefiningValueResult +findBaseDefiningValueOfVector(Value *I) { assert(I->getType()->isVectorTy() && cast(I->getType())->getElementType()->isPointerTy() && "Illegal to ask for the base pointer of a non-pointer type"); @@ -309,7 +366,7 @@ findBaseDefiningValueOfVector(Value *I, Value *Index = nullptr) { if (isa(I)) // An incoming argument to the function is a base pointer - return std::make_pair(I, true); + return BaseDefiningValueResult(I, true); // We shouldn't see the address of a global as a vector value? assert(!isa(I) && @@ -320,7 +377,7 @@ findBaseDefiningValueOfVector(Value *I, Value *Index = nullptr) { if (isa(I)) // utterly meaningless, but useful for dealing with partially optimized // code. - return std::make_pair(I, true); + return BaseDefiningValueResult(I, true); // Due to inheritance, this must be _after_ the global variable and undef // checks @@ -328,31 +385,17 @@ findBaseDefiningValueOfVector(Value *I, Value *Index = nullptr) { assert(!isa(I) && !isa(I) && "order of checks wrong!"); assert(Con->isNullValue() && "null is the only case which makes sense"); - return std::make_pair(Con, true); + return BaseDefiningValueResult(Con, true); } if (isa(I)) - return std::make_pair(I, true); - - // For an insert element, we might be able to look through it if we know - // something about the indexes. - if (InsertElementInst *IEI = dyn_cast(I)) { - if (Index) { - Value *InsertIndex = IEI->getOperand(2); - // This index is inserting the value, look for its BDV - if (InsertIndex == Index) - return std::make_pair(findBaseDefiningValue(IEI->getOperand(1)), false); - // Both constant, and can't be equal per above. This insert is definitely - // not relevant, look back at the rest of the vector and keep trying. - if (isa(Index) && isa(InsertIndex)) - return findBaseDefiningValueOfVector(IEI->getOperand(0), Index); - } - + return BaseDefiningValueResult(I, true); + + if (isa(I)) // We don't know whether this vector contains entirely base pointers or // not. To be conservatively correct, we treat it as a BDV and will // duplicate code as needed to construct a parallel vector of bases. - return std::make_pair(IEI, false); - } + return BaseDefiningValueResult(I, false); if (isa(I)) // We don't know whether this vector contains entirely base pointers or @@ -360,24 +403,22 @@ findBaseDefiningValueOfVector(Value *I, Value *Index = nullptr) { // duplicate code as needed to construct a parallel vector of bases. // TODO: There a number of local optimizations which could be applied here // for particular sufflevector patterns. - return std::make_pair(I, false); + return BaseDefiningValueResult(I, false); // A PHI or Select is a base defining value. The outer findBasePointer // algorithm is responsible for constructing a base value for this BDV. assert((isa(I) || isa(I)) && "unknown vector instruction - no base found for vector element"); - return std::make_pair(I, false); + return BaseDefiningValueResult(I, false); } -static bool isKnownBaseResult(Value *V); - /// Helper function for findBasePointer - Will return a value which either a) /// defines the base pointer for the input, b) blocks the simple search /// (i.e. a PHI or Select of two derived pointers), or c) involves a change /// from pointer to vector type or back. -static Value *findBaseDefiningValue(Value *I) { +static BaseDefiningValueResult findBaseDefiningValue(Value *I) { if (I->getType()->isVectorTy()) - return findBaseDefiningValueOfVector(I).first; + return findBaseDefiningValueOfVector(I); assert(I->getType()->isPointerTy() && "Illegal to ask for the base pointer of a non-pointer type"); @@ -385,39 +426,39 @@ static Value *findBaseDefiningValue(Value *I) { if (isa(I)) // An incoming argument to the function is a base pointer // We should have never reached here if this argument isn't an gc value - return I; + return BaseDefiningValueResult(I, true); if (isa(I)) // base case - return I; + return BaseDefiningValueResult(I, true); // inlining could possibly introduce phi node that contains // undef if callee has multiple returns if (isa(I)) // utterly meaningless, but useful for dealing with // partially optimized code. - return I; + return BaseDefiningValueResult(I, true); // Due to inheritance, this must be _after_ the global variable and undef // checks - if (Constant *Con = dyn_cast(I)) { + if (isa(I)) { assert(!isa(I) && !isa(I) && "order of checks wrong!"); - // Note: Finding a constant base for something marked for relocation - // doesn't really make sense. The most likely case is either a) some - // screwed up the address space usage or b) your validating against - // compiled C++ code w/o the proper separation. The only real exception - // is a null pointer. You could have generic code written to index of - // off a potentially null value and have proven it null. We also use - // null pointers in dead paths of relocation phis (which we might later - // want to find a base pointer for). - assert(isa(Con) && - "null is the only case which makes sense"); - return Con; + // Note: Even for frontends which don't have constant references, we can + // see constants appearing after optimizations. A simple example is + // specialization of an address computation on null feeding into a merge + // point where the actual use of the now-constant input is protected by + // another null check. (e.g. test4 in constants.ll) + return BaseDefiningValueResult(I, true); } if (CastInst *CI = dyn_cast(I)) { Value *Def = CI->stripPointerCasts(); + // If stripping pointer casts changes the address space there is an + // addrspacecast in between. + assert(cast(Def->getType())->getAddressSpace() == + cast(CI->getType())->getAddressSpace() && + "unsupported addrspacecast"); // If we find a cast instruction here, it means we've found a cast which is // not simply a pointer cast (i.e. an inttoptr). We don't know how to // handle int->ptr conversion. @@ -426,7 +467,9 @@ static Value *findBaseDefiningValue(Value *I) { } if (isa(I)) - return I; // The value loaded is an gc base itself + // The value loaded is an gc base itself + return BaseDefiningValueResult(I, true); + if (GetElementPtrInst *GEP = dyn_cast(I)) // The base of this GEP is the base @@ -434,14 +477,11 @@ static Value *findBaseDefiningValue(Value *I) { if (IntrinsicInst *II = dyn_cast(I)) { switch (II->getIntrinsicID()) { - case Intrinsic::experimental_gc_result_ptr: default: // fall through to general call handling break; case Intrinsic::experimental_gc_statepoint: - case Intrinsic::experimental_gc_result_float: - case Intrinsic::experimental_gc_result_int: - llvm_unreachable("these don't produce pointers"); + llvm_unreachable("statepoints don't produce pointers"); case Intrinsic::experimental_gc_relocate: { // Rerunning safepoint insertion after safepoints are already // inserted is not supported. It could probably be made to work, @@ -460,7 +500,7 @@ static Value *findBaseDefiningValue(Value *I) { // pointers. This should probably be generalized via attributes to support // both source language and internal functions. if (isa(I) || isa(I)) - return I; + return BaseDefiningValueResult(I, true); // I have absolutely no idea how to implement this part yet. It's not // necessarily hard, I just haven't really looked at it yet. @@ -470,7 +510,7 @@ static Value *findBaseDefiningValue(Value *I) { // A CAS is effectively a atomic store and load combined under a // predicate. From the perspective of base pointers, we just treat it // like a load. - return I; + return BaseDefiningValueResult(I, true); assert(!isa(I) && "Xchg handled above, all others are " "binary ops which don't apply to pointers"); @@ -479,7 +519,7 @@ static Value *findBaseDefiningValue(Value *I) { // stack, but in either case, this is simply a field load. As a result, // this is a defining definition of the base just like a load is. if (isa(I)) - return I; + return BaseDefiningValueResult(I, true); // We should never see an insert vector since that would require we be // tracing back a struct value not a pointer value. @@ -490,28 +530,11 @@ static Value *findBaseDefiningValue(Value *I) { // We may need to insert a parallel instruction to extract the appropriate // element out of the base vector corresponding to the input. Given this, // it's analogous to the phi and select case even though it's not a merge. - if (auto *EEI = dyn_cast(I)) { - Value *VectorOperand = EEI->getVectorOperand(); - Value *Index = EEI->getIndexOperand(); - std::pair pair = - findBaseDefiningValueOfVector(VectorOperand, Index); - Value *VectorBase = pair.first; - if (VectorBase->getType()->isPointerTy()) - // We found a BDV for this specific element with the vector. This is an - // optimization, but in practice it covers most of the useful cases - // created via scalarization. Note: The peephole optimization here is - // currently needed for correctness since the general algorithm doesn't - // yet handle insertelements. That will change shortly. - return VectorBase; - else { - assert(VectorBase->getType()->isVectorTy()); - // Otherwise, we have an instruction which potentially produces a - // derived pointer and we need findBasePointers to clone code for us - // such that we can create an instruction which produces the - // accompanying base pointer. - return EEI; - } - } + if (isa(I)) + // Note: There a lot of obvious peephole cases here. This are deliberately + // handled after the main base pointer inference algorithm to make writing + // test cases to exercise that code easier. + return BaseDefiningValueResult(I, false); // The last two cases here don't return a base pointer. Instead, they // return a value which dynamically selects from among several base @@ -519,14 +542,14 @@ static Value *findBaseDefiningValue(Value *I) { // the caller to resolve these. assert((isa(I) || isa(I)) && "missing instruction case in findBaseDefiningValing"); - return I; + return BaseDefiningValueResult(I, false); } /// Returns the base defining value for this value. static Value *findBaseDefiningValueCached(Value *I, DefiningValueMapTy &Cache) { Value *&Cached = Cache[I]; if (!Cached) { - Cached = findBaseDefiningValue(I); + Cached = findBaseDefiningValue(I).BDV; DEBUG(dbgs() << "fBDV-cached: " << I->getName() << " -> " << Cached->getName() << "\n"); } @@ -550,7 +573,9 @@ static Value *findBaseOrBDV(Value *I, DefiningValueMapTy &Cache) { /// Given the result of a call to findBaseDefiningValue, or findBaseOrBDV, /// is it known to be a base pointer? Or do we need to continue searching. static bool isKnownBaseResult(Value *V) { - if (!isa(V) && !isa(V) && !isa(V)) { + if (!isa(V) && !isa(V) && + !isa(V) && !isa(V) && + !isa(V)) { // no recursion possible return true; } @@ -613,17 +638,18 @@ public: private: Status status; - Value *base; // non null only if status == base + AssertingVH base; // non null only if status == base }; +} #ifndef NDEBUG -inline raw_ostream &operator<<(raw_ostream &OS, const BDVState &State) { +static raw_ostream &operator<<(raw_ostream &OS, const BDVState &State) { State.print(OS); return OS; } #endif -typedef DenseMap ConflictStateMapTy; +namespace { // Values of type BDVState form a lattice, and this is a helper // class that implementes the meet operation. The meat of the meet // operation is implemented in MeetBDVStates::pureMeet @@ -680,6 +706,8 @@ private: } }; } + + /// For a given value or instruction, figure out what base ptr it's derived /// from. For gc objects, this is simply itself. On success, returns a value /// which is the base pointer. (This is reliable and can be used for @@ -715,20 +743,24 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { #ifndef NDEBUG auto isExpectedBDVType = [](Value *BDV) { - return isa(BDV) || isa(BDV) || isa(BDV); + return isa(BDV) || isa(BDV) || + isa(BDV) || isa(BDV); }; #endif // Once populated, will contain a mapping from each potentially non-base BDV // to a lattice value (described above) which corresponds to that BDV. - ConflictStateMapTy states; - // Recursively fill in all phis & selects reachable from the initial one - // for which we don't already know a definite base value for + // We use the order of insertion (DFS over the def/use graph) to provide a + // stable deterministic ordering for visiting DenseMaps (which are unordered) + // below. This is important for deterministic compilation. + MapVector States; + + // Recursively fill in all base defining values reachable from the initial + // one for which we don't already know a definite base value for /* scope */ { - DenseSet Visited; SmallVector Worklist; Worklist.push_back(def); - Visited.insert(def); + States.insert(std::make_pair(def, BDVState())); while (!Worklist.empty()) { Value *Current = Worklist.pop_back_val(); assert(!isKnownBaseResult(Current) && "why did it get added?"); @@ -741,7 +773,7 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { return; assert(isExpectedBDVType(Base) && "the only non-base values " "we see should be base defining values"); - if (Visited.insert(Base).second) + if (States.insert(std::make_pair(Base, BDVState())).second) Worklist.push_back(Base); }; if (PHINode *Phi = dyn_cast(Current)) { @@ -752,51 +784,47 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { visitIncomingValue(Sel->getFalseValue()); } else if (auto *EE = dyn_cast(Current)) { visitIncomingValue(EE->getVectorOperand()); + } else if (auto *IE = dyn_cast(Current)) { + visitIncomingValue(IE->getOperand(0)); // vector operand + visitIncomingValue(IE->getOperand(1)); // scalar operand } else { - // There are two classes of instructions we know we don't handle. - assert(isa(Current) || - isa(Current)); + // There is one known class of instructions we know we don't handle. + assert(isa(Current)); llvm_unreachable("unimplemented instruction case"); } } - // The frontier of visited instructions are the ones we might need to - // duplicate, so fill in the starting state for the optimistic algorithm - // that follows. - for (Value *BDV : Visited) { - states[BDV] = BDVState(); - } } #ifndef NDEBUG DEBUG(dbgs() << "States after initialization:\n"); - for (auto Pair : states) { + for (auto Pair : States) { DEBUG(dbgs() << " " << Pair.second << " for " << *Pair.first << "\n"); } #endif - // TODO: come back and revisit the state transitions around inputs which - // have reached conflict state. The current version seems too conservative. - // Return a phi state for a base defining value. We'll generate a new // base state for known bases and expect to find a cached state otherwise. auto getStateForBDV = [&](Value *baseValue) { if (isKnownBaseResult(baseValue)) return BDVState(baseValue); - auto I = states.find(baseValue); - assert(I != states.end() && "lookup failed!"); + auto I = States.find(baseValue); + assert(I != States.end() && "lookup failed!"); return I->second; }; bool progress = true; while (progress) { #ifndef NDEBUG - size_t oldSize = states.size(); + const size_t oldSize = States.size(); #endif progress = false; - // We're only changing keys in this loop, thus safe to keep iterators - for (auto Pair : states) { - Value *v = Pair.first; - assert(!isKnownBaseResult(v) && "why did it get added?"); + // We're only changing values in this loop, thus safe to keep iterators. + // Since this is computing a fixed point, the order of visit does not + // effect the result. TODO: We could use a worklist here and make this run + // much faster. + for (auto Pair : States) { + Value *BDV = Pair.first; + assert(!isKnownBaseResult(BDV) && "why did it get added?"); // Given an input value for the current instruction, return a BDVState // instance which represents the BDV of that value. @@ -806,54 +834,48 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { }; MeetBDVStates calculateMeet; - if (SelectInst *select = dyn_cast(v)) { + if (SelectInst *select = dyn_cast(BDV)) { calculateMeet.meetWith(getStateForInput(select->getTrueValue())); calculateMeet.meetWith(getStateForInput(select->getFalseValue())); - } else if (PHINode *Phi = dyn_cast(v)) { + } else if (PHINode *Phi = dyn_cast(BDV)) { for (Value *Val : Phi->incoming_values()) calculateMeet.meetWith(getStateForInput(Val)); - } else { + } else if (auto *EE = dyn_cast(BDV)) { // The 'meet' for an extractelement is slightly trivial, but it's still // useful in that it drives us to conflict if our input is. - auto *EE = cast(v); calculateMeet.meetWith(getStateForInput(EE->getVectorOperand())); + } else { + // Given there's a inherent type mismatch between the operands, will + // *always* produce Conflict. + auto *IE = cast(BDV); + calculateMeet.meetWith(getStateForInput(IE->getOperand(0))); + calculateMeet.meetWith(getStateForInput(IE->getOperand(1))); } - - BDVState oldState = states[v]; + BDVState oldState = States[BDV]; BDVState newState = calculateMeet.getResult(); if (oldState != newState) { progress = true; - states[v] = newState; + States[BDV] = newState; } } - assert(oldSize <= states.size()); - assert(oldSize == states.size() || progress); + assert(oldSize == States.size() && + "fixed point shouldn't be adding any new nodes to state"); } #ifndef NDEBUG DEBUG(dbgs() << "States after meet iteration:\n"); - for (auto Pair : states) { + for (auto Pair : States) { DEBUG(dbgs() << " " << Pair.second << " for " << *Pair.first << "\n"); } #endif // Insert Phis for all conflicts - // We want to keep naming deterministic in the loop that follows, so - // sort the keys before iteration. This is useful in allowing us to - // write stable tests. Note that there is no invalidation issue here. - SmallVector Keys; - Keys.reserve(states.size()); - for (auto Pair : states) { - Value *V = Pair.first; - Keys.push_back(V); - } - std::sort(Keys.begin(), Keys.end(), order_by_name); // TODO: adjust naming patterns to avoid this order of iteration dependency - for (Value *V : Keys) { - Instruction *I = cast(V); - BDVState State = states[I]; + for (auto Pair : States) { + Instruction *I = cast(Pair.first); + BDVState State = Pair.second; assert(!isKnownBaseResult(I) && "why did it get added?"); assert(!State.isUnknown() && "Optimistic algorithm didn't complete!"); @@ -871,7 +893,14 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { EE->getIndexOperand(), "base_ee", EE); BaseInst->setMetadata("is_base_value", MDNode::get(I->getContext(), {})); - states[I] = BDVState(BDVState::Base, BaseInst); + States[I] = BDVState(BDVState::Base, BaseInst); + } + + // Since we're joining a vector and scalar base, they can never be the + // same. As a result, we should always see insert element having reached + // the conflict state. + if (isa(I)) { + assert(State.isConflict()); } if (!State.isConflict()) @@ -884,43 +913,77 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { BasicBlock *BB = I->getParent(); int NumPreds = std::distance(pred_begin(BB), pred_end(BB)); assert(NumPreds > 0 && "how did we reach here"); - std::string Name = I->hasName() ? - (I->getName() + ".base").str() : "base_phi"; + std::string Name = suffixed_name_or(I, ".base", "base_phi"); return PHINode::Create(I->getType(), NumPreds, Name, I); } else if (SelectInst *Sel = dyn_cast(I)) { // The undef will be replaced later UndefValue *Undef = UndefValue::get(Sel->getType()); - std::string Name = I->hasName() ? - (I->getName() + ".base").str() : "base_select"; + std::string Name = suffixed_name_or(I, ".base", "base_select"); return SelectInst::Create(Sel->getCondition(), Undef, Undef, Name, Sel); - } else { - auto *EE = cast(I); + } else if (auto *EE = dyn_cast(I)) { UndefValue *Undef = UndefValue::get(EE->getVectorOperand()->getType()); - std::string Name = I->hasName() ? - (I->getName() + ".base").str() : "base_ee"; + std::string Name = suffixed_name_or(I, ".base", "base_ee"); return ExtractElementInst::Create(Undef, EE->getIndexOperand(), Name, EE); + } else { + auto *IE = cast(I); + UndefValue *VecUndef = UndefValue::get(IE->getOperand(0)->getType()); + UndefValue *ScalarUndef = UndefValue::get(IE->getOperand(1)->getType()); + std::string Name = suffixed_name_or(I, ".base", "base_ie"); + return InsertElementInst::Create(VecUndef, ScalarUndef, + IE->getOperand(2), Name, IE); } + }; Instruction *BaseInst = MakeBaseInstPlaceholder(I); // Add metadata marking this as a base value BaseInst->setMetadata("is_base_value", MDNode::get(I->getContext(), {})); - states[I] = BDVState(BDVState::Conflict, BaseInst); - } + States[I] = BDVState(BDVState::Conflict, BaseInst); + } + + // Returns a instruction which produces the base pointer for a given + // instruction. The instruction is assumed to be an input to one of the BDVs + // seen in the inference algorithm above. As such, we must either already + // know it's base defining value is a base, or have inserted a new + // instruction to propagate the base of it's BDV and have entered that newly + // introduced instruction into the state table. In either case, we are + // assured to be able to determine an instruction which produces it's base + // pointer. + auto getBaseForInput = [&](Value *Input, Instruction *InsertPt) { + Value *BDV = findBaseOrBDV(Input, cache); + Value *Base = nullptr; + if (isKnownBaseResult(BDV)) { + Base = BDV; + } else { + // Either conflict or base. + assert(States.count(BDV)); + Base = States[BDV].getBase(); + } + assert(Base && "can't be null"); + // The cast is needed since base traversal may strip away bitcasts + if (Base->getType() != Input->getType() && + InsertPt) { + Base = new BitCastInst(Base, Input->getType(), "cast", + InsertPt); + } + return Base; + }; - // Fixup all the inputs of the new PHIs - for (auto Pair : states) { - Instruction *v = cast(Pair.first); - BDVState state = Pair.second; + // Fixup all the inputs of the new PHIs. Visit order needs to be + // deterministic and predictable because we're naming newly created + // instructions. + for (auto Pair : States) { + Instruction *BDV = cast(Pair.first); + BDVState State = Pair.second; - assert(!isKnownBaseResult(v) && "why did it get added?"); - assert(!state.isUnknown() && "Optimistic algorithm didn't complete!"); - if (!state.isConflict()) + assert(!isKnownBaseResult(BDV) && "why did it get added?"); + assert(!State.isUnknown() && "Optimistic algorithm didn't complete!"); + if (!State.isConflict()) continue; - if (PHINode *basephi = dyn_cast(state.getBase())) { - PHINode *phi = cast(v); + if (PHINode *basephi = dyn_cast(State.getBase())) { + PHINode *phi = cast(BDV); unsigned NumPHIValues = phi->getNumIncomingValues(); for (unsigned i = 0; i < NumPHIValues; i++) { Value *InVal = phi->getIncomingValue(i); @@ -939,15 +1002,9 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { if (blockIndex != -1) { Value *oldBase = basephi->getIncomingValue(blockIndex); basephi->addIncoming(oldBase, InBB); + #ifndef NDEBUG - Value *base = findBaseOrBDV(InVal, cache); - if (!isKnownBaseResult(base)) { - // Either conflict or base. - assert(states.count(base)); - base = states[base].getBase(); - assert(base != nullptr && "unknown BDVState!"); - } - + Value *Base = getBaseForInput(InVal, nullptr); // In essence this assert states: the only way two // values incoming from the same basic block may be // different is by being different bitcasts of the same @@ -955,67 +1012,48 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { // findBaseOrBDV to return an llvm::Value of the correct // type (and still remain pure). This will remove the // need to add bitcasts. - assert(base->stripPointerCasts() == oldBase->stripPointerCasts() && + assert(Base->stripPointerCasts() == oldBase->stripPointerCasts() && "sanity -- findBaseOrBDV should be pure!"); #endif continue; } - // Find either the defining value for the PHI or the normal base for - // a non-phi node - Value *base = findBaseOrBDV(InVal, cache); - if (!isKnownBaseResult(base)) { - // Either conflict or base. - assert(states.count(base)); - base = states[base].getBase(); - assert(base != nullptr && "unknown BDVState!"); - } - assert(base && "can't be null"); - // Must use original input BB since base may not be Instruction - // The cast is needed since base traversal may strip away bitcasts - if (base->getType() != basephi->getType()) { - base = new BitCastInst(base, basephi->getType(), "cast", - InBB->getTerminator()); - } - basephi->addIncoming(base, InBB); + // Find the instruction which produces the base for each input. We may + // need to insert a bitcast in the incoming block. + // TODO: Need to split critical edges if insertion is needed + Value *Base = getBaseForInput(InVal, InBB->getTerminator()); + basephi->addIncoming(Base, InBB); } assert(basephi->getNumIncomingValues() == NumPHIValues); - } else if (SelectInst *basesel = dyn_cast(state.getBase())) { - SelectInst *sel = cast(v); + } else if (SelectInst *BaseSel = dyn_cast(State.getBase())) { + SelectInst *Sel = cast(BDV); // Operand 1 & 2 are true, false path respectively. TODO: refactor to // something more safe and less hacky. for (int i = 1; i <= 2; i++) { - Value *InVal = sel->getOperand(i); - // Find either the defining value for the PHI or the normal base for - // a non-phi node - Value *base = findBaseOrBDV(InVal, cache); - if (!isKnownBaseResult(base)) { - // Either conflict or base. - assert(states.count(base)); - base = states[base].getBase(); - assert(base != nullptr && "unknown BDVState!"); - } - assert(base && "can't be null"); - // Must use original input BB since base may not be Instruction - // The cast is needed since base traversal may strip away bitcasts - if (base->getType() != basesel->getType()) { - base = new BitCastInst(base, basesel->getType(), "cast", basesel); - } - basesel->setOperand(i, base); - } - } else { - auto *BaseEE = cast(state.getBase()); - Value *InVal = cast(v)->getVectorOperand(); - Value *Base = findBaseOrBDV(InVal, cache); - if (!isKnownBaseResult(Base)) { - // Either conflict or base. - assert(states.count(Base)); - Base = states[Base].getBase(); - assert(Base != nullptr && "unknown BDVState!"); + Value *InVal = Sel->getOperand(i); + // Find the instruction which produces the base for each input. We may + // need to insert a bitcast. + Value *Base = getBaseForInput(InVal, BaseSel); + BaseSel->setOperand(i, Base); } - assert(Base && "can't be null"); + } else if (auto *BaseEE = dyn_cast(State.getBase())) { + Value *InVal = cast(BDV)->getVectorOperand(); + // Find the instruction which produces the base for each input. We may + // need to insert a bitcast. + Value *Base = getBaseForInput(InVal, BaseEE); BaseEE->setOperand(0, Base); + } else { + auto *BaseIE = cast(State.getBase()); + auto *BdvIE = cast(BDV); + auto UpdateOperand = [&](int OperandIdx) { + Value *InVal = BdvIE->getOperand(OperandIdx); + Value *Base = getBaseForInput(InVal, BaseIE); + BaseIE->setOperand(OperandIdx, Base); + }; + UpdateOperand(0); // vector operand + UpdateOperand(1); // scalar operand } + } // Now that we're done with the algorithm, see if we can optimize the @@ -1025,17 +1063,22 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { DenseMap ReverseMap; SmallPtrSet NewInsts; SmallSetVector, 16> Worklist; - for (auto Item : states) { - Value *V = Item.first; - Value *Base = Item.second.getBase(); - assert(V && Base); - assert(!isKnownBaseResult(V) && "why did it get added?"); + // Note: We need to visit the states in a deterministic order. We uses the + // Keys we sorted above for this purpose. Note that we are papering over a + // bigger problem with the algorithm above - it's visit order is not + // deterministic. A larger change is needed to fix this. + for (auto Pair : States) { + auto *BDV = Pair.first; + auto State = Pair.second; + Value *Base = State.getBase(); + assert(BDV && Base); + assert(!isKnownBaseResult(BDV) && "why did it get added?"); assert(isKnownBaseResult(Base) && "must be something we 'know' is a base pointer"); - if (!Item.second.isConflict()) + if (!State.isConflict()) continue; - ReverseMap[Base] = V; + ReverseMap[Base] = BDV; if (auto *BaseI = dyn_cast(Base)) { NewInsts.insert(BaseI); Worklist.insert(BaseI); @@ -1052,10 +1095,10 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { NewInsts.erase(BaseI); ReverseMap.erase(BaseI); BaseI->replaceAllUsesWith(Replacement); + assert(States.count(BDV)); + assert(States[BDV].isConflict() && States[BDV].getBase() == BaseI); + States[BDV] = BDVState(BDVState::Conflict, Replacement); BaseI->eraseFromParent(); - assert(states.count(BDV)); - assert(states[BDV].isConflict() && states[BDV].getBase() == BaseI); - states[BDV] = BDVState(BDVState::Conflict, Replacement); }; const DataLayout &DL = cast(def)->getModule()->getDataLayout(); while (!Worklist.empty()) { @@ -1078,28 +1121,26 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { // Cache all of our results so we can cheaply reuse them // NOTE: This is actually two caches: one of the base defining value // relation and one of the base pointer relation! FIXME - for (auto item : states) { - Value *v = item.first; - Value *base = item.second.getBase(); - assert(v && base); - - std::string fromstr = - cache.count(v) ? (cache[v]->hasName() ? cache[v]->getName() : "") - : "none"; + for (auto Pair : States) { + auto *BDV = Pair.first; + Value *base = Pair.second.getBase(); + assert(BDV && base); + + std::string fromstr = cache.count(BDV) ? cache[BDV]->getName() : "none"; DEBUG(dbgs() << "Updating base value cache" - << " for: " << (v->hasName() ? v->getName() : "") + << " for: " << BDV->getName() << " from: " << fromstr - << " to: " << (base->hasName() ? base->getName() : "") << "\n"); + << " to: " << base->getName() << "\n"); - if (cache.count(v)) { + if (cache.count(BDV)) { // Once we transition from the BDV relation being store in the cache to // the base relation being stored, it must be stable - assert((!isKnownBaseResult(cache[v]) || cache[v] == base) && + assert((!isKnownBaseResult(cache[BDV]) || cache[BDV] == base) && "base relation should be stable"); } - cache[v] = base; + cache[BDV] = base; } - assert(cache.find(def) != cache.end()); + assert(cache.count(def)); return cache[def]; } @@ -1120,7 +1161,7 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache) { // pointer was a base pointer. static void findBasePointers(const StatepointLiveSetTy &live, - DenseMap &PointerToBase, + DenseMap &PointerToBase, DominatorTree *DT, DefiningValueMapTy &DVCache) { // For the naming of values inserted to be deterministic - which makes for // much cleaner and more stable tests - we need to assign an order to the @@ -1152,8 +1193,8 @@ findBasePointers(const StatepointLiveSetTy &live, static void findBasePointers(DominatorTree &DT, DefiningValueMapTy &DVCache, const CallSite &CS, PartiallyConstructedSafepointRecord &result) { - DenseMap PointerToBase; - findBasePointers(result.liveset, PointerToBase, &DT, DVCache); + DenseMap PointerToBase; + findBasePointers(result.LiveSet, PointerToBase, &DT, DVCache); if (PrintBasePointers) { // Note: Need to print these in a stable order since this is checked in @@ -1167,8 +1208,11 @@ static void findBasePointers(DominatorTree &DT, DefiningValueMapTy &DVCache, std::sort(Temp.begin(), Temp.end(), order_by_name); for (Value *Ptr : Temp) { Value *Base = PointerToBase[Ptr]; - errs() << " derived %" << Ptr->getName() << " base %" << Base->getName() - << "\n"; + errs() << " derived "; + Ptr->printAsOperand(errs(), false); + errs() << " base "; + Base->printAsOperand(errs(), false); + errs() << "\n";; } } @@ -1182,7 +1226,7 @@ static void recomputeLiveInValues(GCPtrLivenessData &RevisedLivenessData, PartiallyConstructedSafepointRecord &result); static void recomputeLiveInValues( - Function &F, DominatorTree &DT, Pass *P, ArrayRef toUpdate, + Function &F, DominatorTree &DT, ArrayRef toUpdate, MutableArrayRef records) { // TODO-PERF: reuse the original liveness, then simply run the dataflow // again. The old values are still live and will help it stabilize quickly. @@ -1195,69 +1239,66 @@ static void recomputeLiveInValues( } } -// When inserting gc.relocate calls, we need to ensure there are no uses -// of the original value between the gc.statepoint and the gc.relocate call. -// One case which can arise is a phi node starting one of the successor blocks. -// We also need to be able to insert the gc.relocates only on the path which -// goes through the statepoint. We might need to split an edge to make this -// possible. +// When inserting gc.relocate and gc.result calls, we need to ensure there are +// no uses of the original value / return value between the gc.statepoint and +// the gc.relocate / gc.result call. One case which can arise is a phi node +// starting one of the successor blocks. We also need to be able to insert the +// gc.relocates only on the path which goes through the statepoint. We might +// need to split an edge to make this possible. static BasicBlock * normalizeForInvokeSafepoint(BasicBlock *BB, BasicBlock *InvokeParent, DominatorTree &DT) { BasicBlock *Ret = BB; - if (!BB->getUniquePredecessor()) { + if (!BB->getUniquePredecessor()) Ret = SplitBlockPredecessors(BB, InvokeParent, "", &DT); - } - // Now that 'ret' has unique predecessor we can safely remove all phi nodes + // Now that 'Ret' has unique predecessor we can safely remove all phi nodes // from it FoldSingleEntryPHINodes(Ret); - assert(!isa(Ret->begin())); + assert(!isa(Ret->begin()) && + "All PHI nodes should have been removed!"); - // At this point, we can safely insert a gc.relocate as the first instruction - // in Ret if needed. + // At this point, we can safely insert a gc.relocate or gc.result as the first + // instruction in Ret if needed. return Ret; } -static int find_index(ArrayRef livevec, Value *val) { - auto itr = std::find(livevec.begin(), livevec.end(), val); - assert(livevec.end() != itr); - size_t index = std::distance(livevec.begin(), itr); - assert(index < livevec.size()); - return index; -} - // Create new attribute set containing only attributes which can be transferred // from original call to the safepoint. static AttributeSet legalizeCallAttributes(AttributeSet AS) { - AttributeSet ret; + AttributeSet Ret; for (unsigned Slot = 0; Slot < AS.getNumSlots(); Slot++) { - unsigned index = AS.getSlotIndex(Slot); + unsigned Index = AS.getSlotIndex(Slot); - if (index == AttributeSet::ReturnIndex || - index == AttributeSet::FunctionIndex) { + if (Index == AttributeSet::ReturnIndex || + Index == AttributeSet::FunctionIndex) { - for (auto it = AS.begin(Slot), it_end = AS.end(Slot); it != it_end; - ++it) { - Attribute attr = *it; + for (Attribute Attr : make_range(AS.begin(Slot), AS.end(Slot))) { // Do not allow certain attributes - just skip them // Safepoint can not be read only or read none. - if (attr.hasAttribute(Attribute::ReadNone) || - attr.hasAttribute(Attribute::ReadOnly)) + if (Attr.hasAttribute(Attribute::ReadNone) || + Attr.hasAttribute(Attribute::ReadOnly)) + continue; + + // These attributes control the generation of the gc.statepoint call / + // invoke itself; and once the gc.statepoint is in place, they're of no + // use. + if (Attr.hasAttribute("statepoint-num-patch-bytes") || + Attr.hasAttribute("statepoint-id")) continue; - ret = ret.addAttributes( - AS.getContext(), index, - AttributeSet::get(AS.getContext(), index, AttrBuilder(attr))); + Ret = Ret.addAttributes( + AS.getContext(), Index, + AttributeSet::get(AS.getContext(), Index, AttrBuilder(Attr))); } } // Just skip parameter attributes for now } - return ret; + return Ret; } /// Helper function to place all gc relocates necessary for the given @@ -1269,14 +1310,22 @@ static AttributeSet legalizeCallAttributes(AttributeSet AS) { /// statepointToken - statepoint instruction to which relocates should be /// bound. /// Builder - Llvm IR builder to be used to construct new calls. -static void CreateGCRelocates(ArrayRef LiveVariables, +static void CreateGCRelocates(ArrayRef LiveVariables, const int LiveStart, - ArrayRef BasePtrs, + ArrayRef BasePtrs, Instruction *StatepointToken, IRBuilder<> Builder) { if (LiveVariables.empty()) return; - + + auto FindIndex = [](ArrayRef LiveVec, Value *Val) { + auto ValIt = std::find(LiveVec.begin(), LiveVec.end(), Val); + assert(ValIt != LiveVec.end() && "Val not found in LiveVec!"); + size_t Index = std::distance(LiveVec.begin(), ValIt); + assert(Index < LiveVec.size() && "Bug in std::find?"); + return Index; + }; + // All gc_relocate are set to i8 addrspace(1)* type. We originally generated // unique declarations for each pointer type, but this proved problematic // because the intrinsic mangling code is incomplete and fragile. Since @@ -1292,192 +1341,259 @@ static void CreateGCRelocates(ArrayRef LiveVariables, for (unsigned i = 0; i < LiveVariables.size(); i++) { // Generate the gc.relocate call and save the result Value *BaseIdx = - Builder.getInt32(LiveStart + find_index(LiveVariables, BasePtrs[i])); - Value *LiveIdx = - Builder.getInt32(LiveStart + find_index(LiveVariables, LiveVariables[i])); + Builder.getInt32(LiveStart + FindIndex(LiveVariables, BasePtrs[i])); + Value *LiveIdx = Builder.getInt32(LiveStart + i); // only specify a debug name if we can give a useful one CallInst *Reloc = Builder.CreateCall( GCRelocateDecl, {StatepointToken, BaseIdx, LiveIdx}, - LiveVariables[i]->hasName() ? LiveVariables[i]->getName() + ".relocated" - : ""); + suffixed_name_or(LiveVariables[i], ".relocated", "")); // Trick CodeGen into thinking there are lots of free registers at this // fake call. Reloc->setCallingConv(CallingConv::Cold); } } -static void -makeStatepointExplicitImpl(const CallSite &CS, /* to replace */ - const SmallVectorImpl &basePtrs, - const SmallVectorImpl &liveVariables, - Pass *P, - PartiallyConstructedSafepointRecord &result) { - assert(basePtrs.size() == liveVariables.size()); - assert(isStatepoint(CS) && - "This method expects to be rewriting a statepoint"); +namespace { + +/// This struct is used to defer RAUWs and `eraseFromParent` s. Using this +/// avoids having to worry about keeping around dangling pointers to Values. +class DeferredReplacement { + AssertingVH Old; + AssertingVH New; + +public: + explicit DeferredReplacement(Instruction *Old, Instruction *New) : + Old(Old), New(New) { + assert(Old != New && "Not allowed!"); + } - BasicBlock *BB = CS.getInstruction()->getParent(); - assert(BB); - Function *F = BB->getParent(); - assert(F && "must be set"); - Module *M = F->getParent(); - (void)M; - assert(M && "must be set"); + /// Does the task represented by this instance. + void doReplacement() { + Instruction *OldI = Old; + Instruction *NewI = New; - // We're not changing the function signature of the statepoint since the gc - // arguments go into the var args section. - Function *gc_statepoint_decl = CS.getCalledFunction(); + assert(OldI != NewI && "Disallowed at construction?!"); + + Old = nullptr; + New = nullptr; + + if (NewI) + OldI->replaceAllUsesWith(NewI); + OldI->eraseFromParent(); + } +}; +} + +static void +makeStatepointExplicitImpl(const CallSite CS, /* to replace */ + const SmallVectorImpl &BasePtrs, + const SmallVectorImpl &LiveVariables, + PartiallyConstructedSafepointRecord &Result, + std::vector &Replacements) { + assert(BasePtrs.size() == LiveVariables.size()); + assert((UseDeoptBundles || isStatepoint(CS)) && + "This method expects to be rewriting a statepoint"); // Then go ahead and use the builder do actually do the inserts. We insert // immediately before the previous instruction under the assumption that all // arguments will be available here. We can't insert afterwards since we may // be replacing a terminator. - Instruction *insertBefore = CS.getInstruction(); - IRBuilder<> Builder(insertBefore); - // Copy all of the arguments from the original statepoint - this includes the - // target, call args, and deopt args - SmallVector args; - args.insert(args.end(), CS.arg_begin(), CS.arg_end()); - // TODO: Clear the 'needs rewrite' flag - - // add all the pointers to be relocated (gc arguments) - // Capture the start of the live variable list for use in the gc_relocates - const int live_start = args.size(); - args.insert(args.end(), liveVariables.begin(), liveVariables.end()); + Instruction *InsertBefore = CS.getInstruction(); + IRBuilder<> Builder(InsertBefore); + + ArrayRef GCArgs(LiveVariables); + uint64_t StatepointID = 0xABCDEF00; + uint32_t NumPatchBytes = 0; + uint32_t Flags = uint32_t(StatepointFlags::None); + + ArrayRef CallArgs; + ArrayRef DeoptArgs; + ArrayRef TransitionArgs; + + Value *CallTarget = nullptr; + + if (UseDeoptBundles) { + CallArgs = {CS.arg_begin(), CS.arg_end()}; + DeoptArgs = GetDeoptBundleOperands(CS); + // TODO: we don't fill in TransitionArgs or Flags in this branch, but we + // could have an operand bundle for that too. + AttributeSet OriginalAttrs = CS.getAttributes(); + + Attribute AttrID = OriginalAttrs.getAttribute(AttributeSet::FunctionIndex, + "statepoint-id"); + if (AttrID.isStringAttribute()) + AttrID.getValueAsString().getAsInteger(10, StatepointID); + + Attribute AttrNumPatchBytes = OriginalAttrs.getAttribute( + AttributeSet::FunctionIndex, "statepoint-num-patch-bytes"); + if (AttrNumPatchBytes.isStringAttribute()) + AttrNumPatchBytes.getValueAsString().getAsInteger(10, NumPatchBytes); + + CallTarget = CS.getCalledValue(); + } else { + // This branch will be gone soon, and we will soon only support the + // UseDeoptBundles == true configuration. + Statepoint OldSP(CS); + StatepointID = OldSP.getID(); + NumPatchBytes = OldSP.getNumPatchBytes(); + Flags = OldSP.getFlags(); + + CallArgs = {OldSP.arg_begin(), OldSP.arg_end()}; + DeoptArgs = {OldSP.vm_state_begin(), OldSP.vm_state_end()}; + TransitionArgs = {OldSP.gc_transition_args_begin(), + OldSP.gc_transition_args_end()}; + CallTarget = OldSP.getCalledValue(); + } // Create the statepoint given all the arguments - Instruction *token = nullptr; - AttributeSet return_attributes; + Instruction *Token = nullptr; + AttributeSet ReturnAttrs; if (CS.isCall()) { - CallInst *toReplace = cast(CS.getInstruction()); - CallInst *call = - Builder.CreateCall(gc_statepoint_decl, args, "safepoint_token"); - call->setTailCall(toReplace->isTailCall()); - call->setCallingConv(toReplace->getCallingConv()); + CallInst *ToReplace = cast(CS.getInstruction()); + CallInst *Call = Builder.CreateGCStatepointCall( + StatepointID, NumPatchBytes, CallTarget, Flags, CallArgs, + TransitionArgs, DeoptArgs, GCArgs, "safepoint_token"); + + Call->setTailCall(ToReplace->isTailCall()); + Call->setCallingConv(ToReplace->getCallingConv()); // Currently we will fail on parameter attributes and on certain // function attributes. - AttributeSet new_attrs = legalizeCallAttributes(toReplace->getAttributes()); + AttributeSet NewAttrs = legalizeCallAttributes(ToReplace->getAttributes()); // In case if we can handle this set of attributes - set up function attrs // directly on statepoint and return attrs later for gc_result intrinsic. - call->setAttributes(new_attrs.getFnAttributes()); - return_attributes = new_attrs.getRetAttributes(); + Call->setAttributes(NewAttrs.getFnAttributes()); + ReturnAttrs = NewAttrs.getRetAttributes(); - token = call; + Token = Call; // Put the following gc_result and gc_relocate calls immediately after the // the old call (which we're about to delete) - BasicBlock::iterator next(toReplace); - assert(BB->end() != next && "not a terminator, must have next"); - next++; - Instruction *IP = &*(next); - Builder.SetInsertPoint(IP); - Builder.SetCurrentDebugLocation(IP->getDebugLoc()); - + assert(ToReplace->getNextNode() && "Not a terminator, must have next!"); + Builder.SetInsertPoint(ToReplace->getNextNode()); + Builder.SetCurrentDebugLocation(ToReplace->getNextNode()->getDebugLoc()); } else { - InvokeInst *toReplace = cast(CS.getInstruction()); + InvokeInst *ToReplace = cast(CS.getInstruction()); // Insert the new invoke into the old block. We'll remove the old one in a // moment at which point this will become the new terminator for the // original block. - InvokeInst *invoke = InvokeInst::Create( - gc_statepoint_decl, toReplace->getNormalDest(), - toReplace->getUnwindDest(), args, "statepoint_token", toReplace->getParent()); - invoke->setCallingConv(toReplace->getCallingConv()); + InvokeInst *Invoke = Builder.CreateGCStatepointInvoke( + StatepointID, NumPatchBytes, CallTarget, ToReplace->getNormalDest(), + ToReplace->getUnwindDest(), Flags, CallArgs, TransitionArgs, DeoptArgs, + GCArgs, "statepoint_token"); + + Invoke->setCallingConv(ToReplace->getCallingConv()); // Currently we will fail on parameter attributes and on certain // function attributes. - AttributeSet new_attrs = legalizeCallAttributes(toReplace->getAttributes()); + AttributeSet NewAttrs = legalizeCallAttributes(ToReplace->getAttributes()); // In case if we can handle this set of attributes - set up function attrs // directly on statepoint and return attrs later for gc_result intrinsic. - invoke->setAttributes(new_attrs.getFnAttributes()); - return_attributes = new_attrs.getRetAttributes(); + Invoke->setAttributes(NewAttrs.getFnAttributes()); + ReturnAttrs = NewAttrs.getRetAttributes(); - token = invoke; + Token = Invoke; // Generate gc relocates in exceptional path - BasicBlock *unwindBlock = toReplace->getUnwindDest(); - assert(!isa(unwindBlock->begin()) && - unwindBlock->getUniquePredecessor() && + BasicBlock *UnwindBlock = ToReplace->getUnwindDest(); + assert(!isa(UnwindBlock->begin()) && + UnwindBlock->getUniquePredecessor() && "can't safely insert in this block!"); - Instruction *IP = &*(unwindBlock->getFirstInsertionPt()); - Builder.SetInsertPoint(IP); - Builder.SetCurrentDebugLocation(toReplace->getDebugLoc()); + Builder.SetInsertPoint(&*UnwindBlock->getFirstInsertionPt()); + Builder.SetCurrentDebugLocation(ToReplace->getDebugLoc()); - // Extract second element from landingpad return value. We will attach - // exceptional gc relocates to it. - const unsigned idx = 1; - Instruction *exceptional_token = - cast(Builder.CreateExtractValue( - unwindBlock->getLandingPadInst(), idx, "relocate_token")); - result.UnwindToken = exceptional_token; + // Attach exceptional gc relocates to the landingpad. + Instruction *ExceptionalToken = UnwindBlock->getLandingPadInst(); + Result.UnwindToken = ExceptionalToken; - CreateGCRelocates(liveVariables, live_start, basePtrs, - exceptional_token, Builder); + const unsigned LiveStartIdx = Statepoint(Token).gcArgsStartIdx(); + CreateGCRelocates(LiveVariables, LiveStartIdx, BasePtrs, ExceptionalToken, + Builder); // Generate gc relocates and returns for normal block - BasicBlock *normalDest = toReplace->getNormalDest(); - assert(!isa(normalDest->begin()) && - normalDest->getUniquePredecessor() && + BasicBlock *NormalDest = ToReplace->getNormalDest(); + assert(!isa(NormalDest->begin()) && + NormalDest->getUniquePredecessor() && "can't safely insert in this block!"); - IP = &*(normalDest->getFirstInsertionPt()); - Builder.SetInsertPoint(IP); + Builder.SetInsertPoint(&*NormalDest->getFirstInsertionPt()); // gc relocates will be generated later as if it were regular call // statepoint } - assert(token); - - // Take the name of the original value call if it had one. - token->takeName(CS.getInstruction()); + assert(Token && "Should be set in one of the above branches!"); + + if (UseDeoptBundles) { + Token->setName("statepoint_token"); + if (!CS.getType()->isVoidTy() && !CS.getInstruction()->use_empty()) { + StringRef Name = + CS.getInstruction()->hasName() ? CS.getInstruction()->getName() : ""; + CallInst *GCResult = Builder.CreateGCResult(Token, CS.getType(), Name); + GCResult->setAttributes(CS.getAttributes().getRetAttributes()); + + // We cannot RAUW or delete CS.getInstruction() because it could be in the + // live set of some other safepoint, in which case that safepoint's + // PartiallyConstructedSafepointRecord will hold a raw pointer to this + // llvm::Instruction. Instead, we defer the replacement and deletion to + // after the live sets have been made explicit in the IR, and we no longer + // have raw pointers to worry about. + Replacements.emplace_back(CS.getInstruction(), GCResult); + } else { + Replacements.emplace_back(CS.getInstruction(), nullptr); + } + } else { + assert(!CS.getInstruction()->hasNUsesOrMore(2) && + "only valid use before rewrite is gc.result"); + assert(!CS.getInstruction()->hasOneUse() || + isGCResult(cast(*CS.getInstruction()->user_begin()))); -// The GCResult is already inserted, we just need to find it -#ifndef NDEBUG - Instruction *toReplace = CS.getInstruction(); - assert((toReplace->hasNUses(0) || toReplace->hasNUses(1)) && - "only valid use before rewrite is gc.result"); - assert(!toReplace->hasOneUse() || - isGCResult(cast(*toReplace->user_begin()))); -#endif + // Take the name of the original statepoint token if there was one. + Token->takeName(CS.getInstruction()); - // Update the gc.result of the original statepoint (if any) to use the newly - // inserted statepoint. This is safe to do here since the token can't be - // considered a live reference. - CS.getInstruction()->replaceAllUsesWith(token); + // Update the gc.result of the original statepoint (if any) to use the newly + // inserted statepoint. This is safe to do here since the token can't be + // considered a live reference. + CS.getInstruction()->replaceAllUsesWith(Token); + CS.getInstruction()->eraseFromParent(); + } - result.StatepointToken = token; + Result.StatepointToken = Token; // Second, create a gc.relocate for every live variable - CreateGCRelocates(liveVariables, live_start, basePtrs, token, Builder); + const unsigned LiveStartIdx = Statepoint(Token).gcArgsStartIdx(); + CreateGCRelocates(LiveVariables, LiveStartIdx, BasePtrs, Token, Builder); } namespace { -struct name_ordering { - Value *base; - Value *derived; - bool operator()(name_ordering const &a, name_ordering const &b) { - return -1 == a.derived->getName().compare(b.derived->getName()); +struct NameOrdering { + Value *Base; + Value *Derived; + + bool operator()(NameOrdering const &a, NameOrdering const &b) { + return -1 == a.Derived->getName().compare(b.Derived->getName()); } }; } -static void stablize_order(SmallVectorImpl &basevec, - SmallVectorImpl &livevec) { - assert(basevec.size() == livevec.size()); - - SmallVector temp; - for (size_t i = 0; i < basevec.size(); i++) { - name_ordering v; - v.base = basevec[i]; - v.derived = livevec[i]; - temp.push_back(v); - } - std::sort(temp.begin(), temp.end(), name_ordering()); - for (size_t i = 0; i < basevec.size(); i++) { - basevec[i] = temp[i].base; - livevec[i] = temp[i].derived; + +static void StabilizeOrder(SmallVectorImpl &BaseVec, + SmallVectorImpl &LiveVec) { + assert(BaseVec.size() == LiveVec.size()); + + SmallVector Temp; + for (size_t i = 0; i < BaseVec.size(); i++) { + NameOrdering v; + v.Base = BaseVec[i]; + v.Derived = LiveVec[i]; + Temp.push_back(v); + } + + std::sort(Temp.begin(), Temp.end(), NameOrdering()); + for (size_t i = 0; i < BaseVec.size(); i++) { + BaseVec[i] = Temp[i].Base; + LiveVec[i] = Temp[i].Derived; } } @@ -1487,39 +1603,39 @@ static void stablize_order(SmallVectorImpl &basevec, // WARNING: Does not do any fixup to adjust users of the original live // values. That's the callers responsibility. static void -makeStatepointExplicit(DominatorTree &DT, const CallSite &CS, Pass *P, - PartiallyConstructedSafepointRecord &result) { - auto liveset = result.liveset; - auto PointerToBase = result.PointerToBase; +makeStatepointExplicit(DominatorTree &DT, const CallSite &CS, + PartiallyConstructedSafepointRecord &Result, + std::vector &Replacements) { + const auto &LiveSet = Result.LiveSet; + const auto &PointerToBase = Result.PointerToBase; // Convert to vector for efficient cross referencing. - SmallVector basevec, livevec; - livevec.reserve(liveset.size()); - basevec.reserve(liveset.size()); - for (Value *L : liveset) { - livevec.push_back(L); + SmallVector BaseVec, LiveVec; + LiveVec.reserve(LiveSet.size()); + BaseVec.reserve(LiveSet.size()); + for (Value *L : LiveSet) { + LiveVec.push_back(L); assert(PointerToBase.count(L)); - Value *base = PointerToBase[L]; - basevec.push_back(base); + Value *Base = PointerToBase.find(L)->second; + BaseVec.push_back(Base); } - assert(livevec.size() == basevec.size()); + assert(LiveVec.size() == BaseVec.size()); // To make the output IR slightly more stable (for use in diffs), ensure a // fixed order of the values in the safepoint (by sorting the value name). // The order is otherwise meaningless. - stablize_order(basevec, livevec); + StabilizeOrder(BaseVec, LiveVec); // Do the actual rewriting and delete the old statepoint - makeStatepointExplicitImpl(CS, basevec, livevec, P, result); - CS.getInstruction()->eraseFromParent(); + makeStatepointExplicitImpl(CS, BaseVec, LiveVec, Result, Replacements); } // Helper function for the relocationViaAlloca. -// It receives iterator to the statepoint gc relocates and emits store to the -// assigned -// location (via allocaMap) for the each one of them. -// Add visited values into the visitedLiveValues set we will later use them -// for sanity check. +// +// It receives iterator to the statepoint gc relocates and emits a store to the +// assigned location (via allocaMap) for the each one of them. It adds the +// visited values into the visitedLiveValues set, which we will later use them +// for sanity checking. static void insertRelocationStores(iterator_range GCRelocs, DenseMap &AllocaMap, @@ -1544,13 +1660,15 @@ insertRelocationStores(iterator_range GCRelocs, Value *Alloca = AllocaMap[OriginalValue]; // Emit store into the related alloca - // All gc_relocate are i8 addrspace(1)* typed, and it must be bitcasted to + // All gc_relocates are i8 addrspace(1)* typed, and it must be bitcasted to // the correct type according to alloca. - assert(RelocatedValue->getNextNode() && "Should always have one since it's not a terminator"); + assert(RelocatedValue->getNextNode() && + "Should always have one since it's not a terminator"); IRBuilder<> Builder(RelocatedValue->getNextNode()); Value *CastedRelocatedValue = - Builder.CreateBitCast(RelocatedValue, cast(Alloca)->getAllocatedType(), - RelocatedValue->hasName() ? RelocatedValue->getName() + ".casted" : ""); + Builder.CreateBitCast(RelocatedValue, + cast(Alloca)->getAllocatedType(), + suffixed_name_or(RelocatedValue, ".casted", "")); StoreInst *Store = new StoreInst(CastedRelocatedValue, Alloca); Store->insertAfter(cast(CastedRelocatedValue)); @@ -1586,10 +1704,10 @@ insertRematerializationStores( } } -/// do all the relocation update via allocas and mem2reg +/// Do all the relocation update via allocas and mem2reg static void relocationViaAlloca( Function &F, DominatorTree &DT, ArrayRef Live, - ArrayRef Records) { + ArrayRef Records) { #ifndef NDEBUG // record initial number of (static) allocas; we'll check we have the same // number when we get done. @@ -1616,15 +1734,12 @@ static void relocationViaAlloca( PromotableAllocas.push_back(Alloca); }; - // emit alloca for each live gc pointer - for (unsigned i = 0; i < Live.size(); i++) { - emitAllocaFor(Live[i]); - } - - // emit allocas for rematerialized values - for (size_t i = 0; i < Records.size(); i++) { - const struct PartiallyConstructedSafepointRecord &Info = Records[i]; + // Emit alloca for each live gc pointer + for (Value *V : Live) + emitAllocaFor(V); + // Emit allocas for rematerialized values + for (const auto &Info : Records) for (auto RematerializedValuePair : Info.RematerializedValues) { Value *OriginalValue = RematerializedValuePair.second; if (AllocaMap.count(OriginalValue) != 0) @@ -1633,20 +1748,17 @@ static void relocationViaAlloca( emitAllocaFor(OriginalValue); ++NumRematerializedValues; } - } // The next two loops are part of the same conceptual operation. We need to // insert a store to the alloca after the original def and at each // redefinition. We need to insert a load before each use. These are split // into distinct loops for performance reasons. - // update gc pointer after each statepoint - // either store a relocated value or null (if no relocated value found for - // this gc pointer and it is not a gc_result) - // this must happen before we update the statepoint with load of alloca - // otherwise we lose the link between statepoint and old def - for (size_t i = 0; i < Records.size(); i++) { - const struct PartiallyConstructedSafepointRecord &Info = Records[i]; + // Update gc pointer after each statepoint: either store a relocated value or + // null (if no relocated value was found for this gc pointer and it is not a + // gc_result). This must happen before we update the statepoint with load of + // alloca otherwise we lose the link between statepoint and old def. + for (const auto &Info : Records) { Value *Statepoint = Info.StatepointToken; // This will be used for consistency check @@ -1697,23 +1809,22 @@ static void relocationViaAlloca( // Insert the clobbering stores. These may get intermixed with the // gc.results and gc.relocates, but that's fine. if (auto II = dyn_cast(Statepoint)) { - InsertClobbersAt(II->getNormalDest()->getFirstInsertionPt()); - InsertClobbersAt(II->getUnwindDest()->getFirstInsertionPt()); + InsertClobbersAt(&*II->getNormalDest()->getFirstInsertionPt()); + InsertClobbersAt(&*II->getUnwindDest()->getFirstInsertionPt()); } else { - BasicBlock::iterator Next(cast(Statepoint)); - Next++; - InsertClobbersAt(Next); + InsertClobbersAt(cast(Statepoint)->getNextNode()); } } } - // update use with load allocas and add store for gc_relocated + + // Update use with load allocas and add store for gc_relocated. for (auto Pair : AllocaMap) { Value *Def = Pair.first; Value *Alloca = Pair.second; - // we pre-record the uses of allocas so that we dont have to worry about - // later update - // that change the user information. + // We pre-record the uses of allocas so that we dont have to worry about + // later update that changes the user information.. + SmallVector Uses; // PERF: trade a linear scan for repeated reallocation Uses.reserve(std::distance(Def->user_begin(), Def->user_end())); @@ -1748,9 +1859,9 @@ static void relocationViaAlloca( } } - // emit store for the initial gc value - // store must be inserted after load, otherwise store will be in alloca's - // use list and an extra load will be inserted before it + // Emit store for the initial gc value. Store must be inserted after load, + // otherwise store will be in alloca's use list and an extra load will be + // inserted before it. StoreInst *Store = new StoreInst(Def, Alloca); if (Instruction *Inst = dyn_cast(Def)) { if (InvokeInst *Invoke = dyn_cast(Inst)) { @@ -1773,14 +1884,13 @@ static void relocationViaAlloca( assert(PromotableAllocas.size() == Live.size() + NumRematerializedValues && "we must have the same allocas with lives"); if (!PromotableAllocas.empty()) { - // apply mem2reg to promote alloca to SSA + // Apply mem2reg to promote alloca to SSA PromoteMemToReg(PromotableAllocas, DT); } #ifndef NDEBUG - for (auto I = F.getEntryBlock().begin(), E = F.getEntryBlock().end(); I != E; - I++) - if (isa(*I)) + for (auto &I : F.getEntryBlock()) + if (isa(I)) InitialAllocaNum--; assert(InitialAllocaNum == 0 && "We must not introduce any extra allocas"); #endif @@ -1804,28 +1914,27 @@ static void insertUseHolderAfter(CallSite &CS, const ArrayRef Values, // No values to hold live, might as well not insert the empty holder return; - Module *M = CS.getInstruction()->getParent()->getParent()->getParent(); + Module *M = CS.getInstruction()->getModule(); // Use a dummy vararg function to actually hold the values live Function *Func = cast(M->getOrInsertFunction( "__tmp_use", FunctionType::get(Type::getVoidTy(M->getContext()), true))); if (CS.isCall()) { // For call safepoints insert dummy calls right after safepoint - BasicBlock::iterator Next(CS.getInstruction()); - Next++; - Holders.push_back(CallInst::Create(Func, Values, "", Next)); + Holders.push_back(CallInst::Create(Func, Values, "", + &*++CS.getInstruction()->getIterator())); return; } // For invoke safepooints insert dummy calls both in normal and // exceptional destination blocks auto *II = cast(CS.getInstruction()); Holders.push_back(CallInst::Create( - Func, Values, "", II->getNormalDest()->getFirstInsertionPt())); + Func, Values, "", &*II->getNormalDest()->getFirstInsertionPt())); Holders.push_back(CallInst::Create( - Func, Values, "", II->getUnwindDest()->getFirstInsertionPt())); + Func, Values, "", &*II->getUnwindDest()->getFirstInsertionPt())); } static void findLiveReferences( - Function &F, DominatorTree &DT, Pass *P, ArrayRef toUpdate, + Function &F, DominatorTree &DT, ArrayRef toUpdate, MutableArrayRef records) { GCPtrLivenessData OriginalLivenessData; computeLiveInValues(DT, F, OriginalLivenessData); @@ -1836,8 +1945,8 @@ static void findLiveReferences( } } -/// Remove any vector of pointers from the liveset by scalarizing them over the -/// statepoint instruction. Adds the scalarized pieces to the liveset. It +/// Remove any vector of pointers from the live set by scalarizing them over the +/// statepoint instruction. Adds the scalarized pieces to the live set. It /// would be preferable to include the vector in the statepoint itself, but /// the lowering code currently does not handle that. Extending it would be /// slightly non-trivial since it requires a format change. Given how rare @@ -1992,16 +2101,12 @@ static bool findRematerializableChainToBasePointer( } if (CastInst *CI = dyn_cast(CurrentValue)) { - Value *Def = CI->stripPointerCasts(); - - // This two checks are basically similar. First one is here for the - // consistency with findBasePointers logic. - assert(!isa(Def) && "not a pointer cast found"); if (!CI->isNoopCast(CI->getModule()->getDataLayout())) return false; ChainToBase.push_back(CI); - return findRematerializableChainToBasePointer(ChainToBase, Def, BaseValue); + return findRematerializableChainToBasePointer(ChainToBase, + CI->getOperand(0), BaseValue); } // Not supported instruction in the chain @@ -2042,8 +2147,8 @@ chainToBasePointerCost(SmallVectorImpl &Chain, return Cost; } -// From the statepoint liveset pick values that are cheaper to recompute then to -// relocate. Remove this values from the liveset, rematerialize them after +// From the statepoint live set pick values that are cheaper to recompute then +// to relocate. Remove this values from the live set, rematerialize them after // statepoint and record them in "Info" structure. Note that similar to // relocated values we don't do any user adjustments here. static void rematerializeLiveValues(CallSite CS, @@ -2055,7 +2160,7 @@ static void rematerializeLiveValues(CallSite CS, // We can not di this in following loop due to iterator invalidation. SmallVector LiveValuesToBeDeleted; - for (Value *LiveValue: Info.liveset) { + for (Value *LiveValue: Info.LiveSet) { // For each live pointer find it's defining chain SmallVector ChainToBase; assert(Info.PointerToBase.count(LiveValue)); @@ -2144,9 +2249,9 @@ static void rematerializeLiveValues(CallSite CS, InvokeInst *Invoke = cast(CS.getInstruction()); Instruction *NormalInsertBefore = - Invoke->getNormalDest()->getFirstInsertionPt(); + &*Invoke->getNormalDest()->getFirstInsertionPt(); Instruction *UnwindInsertBefore = - Invoke->getUnwindDest()->getFirstInsertionPt(); + &*Invoke->getUnwindDest()->getFirstInsertionPt(); Instruction *NormalRematerializedValue = rematerializeChain(NormalInsertBefore); @@ -2160,22 +2265,23 @@ static void rematerializeLiveValues(CallSite CS, // Remove rematerializaed values from the live set for (auto LiveValue: LiveValuesToBeDeleted) { - Info.liveset.erase(LiveValue); + Info.LiveSet.erase(LiveValue); } } -static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P, - SmallVectorImpl &toUpdate) { +static bool insertParsePoints(Function &F, DominatorTree &DT, + TargetTransformInfo &TTI, + SmallVectorImpl &ToUpdate) { #ifndef NDEBUG // sanity check the input - std::set uniqued; - uniqued.insert(toUpdate.begin(), toUpdate.end()); - assert(uniqued.size() == toUpdate.size() && "no duplicates please!"); + std::set Uniqued; + Uniqued.insert(ToUpdate.begin(), ToUpdate.end()); + assert(Uniqued.size() == ToUpdate.size() && "no duplicates please!"); - for (size_t i = 0; i < toUpdate.size(); i++) { - CallSite &CS = toUpdate[i]; + for (CallSite CS : ToUpdate) { assert(CS.getInstruction()->getParent()->getParent() == &F); - assert(isStatepoint(CS) && "expected to already be a deopt statepoint"); + assert((UseDeoptBundles || isStatepoint(CS)) && + "expected to already be a deopt statepoint"); } #endif @@ -2183,50 +2289,45 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P, // the top of the successor blocks. See the comment on // normalForInvokeSafepoint on exactly what is needed. Note that this step // may restructure the CFG. - for (CallSite CS : toUpdate) { + for (CallSite CS : ToUpdate) { if (!CS.isInvoke()) continue; - InvokeInst *invoke = cast(CS.getInstruction()); - normalizeForInvokeSafepoint(invoke->getNormalDest(), invoke->getParent(), - DT); - normalizeForInvokeSafepoint(invoke->getUnwindDest(), invoke->getParent(), - DT); + auto *II = cast(CS.getInstruction()); + normalizeForInvokeSafepoint(II->getNormalDest(), II->getParent(), DT); + normalizeForInvokeSafepoint(II->getUnwindDest(), II->getParent(), DT); } // A list of dummy calls added to the IR to keep various values obviously // live in the IR. We'll remove all of these when done. - SmallVector holders; + SmallVector Holders; // Insert a dummy call with all of the arguments to the vm_state we'll need // for the actual safepoint insertion. This ensures reference arguments in // the deopt argument list are considered live through the safepoint (and // thus makes sure they get relocated.) - for (size_t i = 0; i < toUpdate.size(); i++) { - CallSite &CS = toUpdate[i]; - Statepoint StatepointCS(CS); - + for (CallSite CS : ToUpdate) { SmallVector DeoptValues; - for (Use &U : StatepointCS.vm_state_args()) { - Value *Arg = cast(&U); + + iterator_range DeoptStateRange = + UseDeoptBundles + ? iterator_range(GetDeoptBundleOperands(CS)) + : iterator_range(Statepoint(CS).vm_state_args()); + + for (Value *Arg : DeoptStateRange) { assert(!isUnhandledGCPointerType(Arg->getType()) && "support for FCA unimplemented"); if (isHandledGCPointerType(Arg->getType())) DeoptValues.push_back(Arg); } - insertUseHolderAfter(CS, DeoptValues, holders); - } - SmallVector records; - records.reserve(toUpdate.size()); - for (size_t i = 0; i < toUpdate.size(); i++) { - struct PartiallyConstructedSafepointRecord info; - records.push_back(info); + insertUseHolderAfter(CS, DeoptValues, Holders); } - assert(records.size() == toUpdate.size()); + + SmallVector Records(ToUpdate.size()); // A) Identify all gc pointers which are statically live at the given call // site. - findLiveReferences(F, DT, P, toUpdate, records); + findLiveReferences(F, DT, ToUpdate, Records); // B) Find the base pointers for each live pointer /* scope for caching */ { @@ -2235,10 +2336,9 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P, // large numbers of duplicate base_phis. DefiningValueMapTy DVCache; - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; - CallSite &CS = toUpdate[i]; - findBasePointers(DT, DVCache, CS, info); + for (size_t i = 0; i < Records.size(); i++) { + PartiallyConstructedSafepointRecord &info = Records[i]; + findBasePointers(DT, DVCache, ToUpdate[i], info); } } // end of cache scope @@ -2255,63 +2355,75 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P, // the base pointers which were identified for that safepoint. We'll then // ask liveness for _every_ base inserted to see what is now live. Then we // remove the dummy calls. - holders.reserve(holders.size() + records.size()); - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; - CallSite &CS = toUpdate[i]; + Holders.reserve(Holders.size() + Records.size()); + for (size_t i = 0; i < Records.size(); i++) { + PartiallyConstructedSafepointRecord &Info = Records[i]; SmallVector Bases; - for (auto Pair : info.PointerToBase) { + for (auto Pair : Info.PointerToBase) Bases.push_back(Pair.second); - } - insertUseHolderAfter(CS, Bases, holders); + + insertUseHolderAfter(ToUpdate[i], Bases, Holders); } // By selecting base pointers, we've effectively inserted new uses. Thus, we // need to rerun liveness. We may *also* have inserted new defs, but that's // not the key issue. - recomputeLiveInValues(F, DT, P, toUpdate, records); + recomputeLiveInValues(F, DT, ToUpdate, Records); if (PrintBasePointers) { - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; + for (auto &Info : Records) { errs() << "Base Pairs: (w/Relocation)\n"; - for (auto Pair : info.PointerToBase) { - errs() << " derived %" << Pair.first->getName() << " base %" - << Pair.second->getName() << "\n"; + for (auto Pair : Info.PointerToBase) { + errs() << " derived "; + Pair.first->printAsOperand(errs(), false); + errs() << " base "; + Pair.second->printAsOperand(errs(), false); + errs() << "\n"; } } } - for (size_t i = 0; i < holders.size(); i++) { - holders[i]->eraseFromParent(); - holders[i] = nullptr; - } - holders.clear(); + + // It is possible that non-constant live variables have a constant base. For + // example, a GEP with a variable offset from a global. In this case we can + // remove it from the liveset. We already don't add constants to the liveset + // because we assume they won't move at runtime and the GC doesn't need to be + // informed about them. The same reasoning applies if the base is constant. + // Note that the relocation placement code relies on this filtering for + // correctness as it expects the base to be in the liveset, which isn't true + // if the base is constant. + for (auto &Info : Records) + for (auto &BasePair : Info.PointerToBase) + if (isa(BasePair.second)) + Info.LiveSet.erase(BasePair.first); + + for (CallInst *CI : Holders) + CI->eraseFromParent(); + + Holders.clear(); // Do a limited scalarization of any live at safepoint vector values which // contain pointers. This enables this pass to run after vectorization at // the cost of some possible performance loss. TODO: it would be nice to // natively support vectors all the way through the backend so we don't need // to scalarize here. - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; - Instruction *statepoint = toUpdate[i].getInstruction(); - splitVectorValues(cast(statepoint), info.liveset, - info.PointerToBase, DT); + for (size_t i = 0; i < Records.size(); i++) { + PartiallyConstructedSafepointRecord &Info = Records[i]; + Instruction *Statepoint = ToUpdate[i].getInstruction(); + splitVectorValues(cast(Statepoint), Info.LiveSet, + Info.PointerToBase, DT); } // In order to reduce live set of statepoint we might choose to rematerialize // some values instead of relocating them. This is purely an optimization and // does not influence correctness. - TargetTransformInfo &TTI = - P->getAnalysis().getTTI(F); + for (size_t i = 0; i < Records.size(); i++) + rematerializeLiveValues(ToUpdate[i], Records[i], TTI); - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; - CallSite &CS = toUpdate[i]; - - rematerializeLiveValues(CS, info, TTI); - } + // We need this to safely RAUW and delete call or invoke return values that + // may themselves be live over a statepoint. For details, please see usage in + // makeStatepointExplicitImpl. + std::vector Replacements; // Now run through and replace the existing statepoints with new ones with // the live variables listed. We do not yet update uses of the values being @@ -2319,61 +2431,77 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P, // survive to the last iteration of this loop. (By construction, the // previous statepoint can not be a live variable, thus we can and remove // the old statepoint calls as we go.) - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; - CallSite &CS = toUpdate[i]; - makeStatepointExplicit(DT, CS, P, info); + for (size_t i = 0; i < Records.size(); i++) + makeStatepointExplicit(DT, ToUpdate[i], Records[i], Replacements); + + ToUpdate.clear(); // prevent accident use of invalid CallSites + + for (auto &PR : Replacements) + PR.doReplacement(); + + Replacements.clear(); + + for (auto &Info : Records) { + // These live sets may contain state Value pointers, since we replaced calls + // with operand bundles with calls wrapped in gc.statepoint, and some of + // those calls may have been def'ing live gc pointers. Clear these out to + // avoid accidentally using them. + // + // TODO: We should create a separate data structure that does not contain + // these live sets, and migrate to using that data structure from this point + // onward. + Info.LiveSet.clear(); + Info.PointerToBase.clear(); } - toUpdate.clear(); // prevent accident use of invalid CallSites // Do all the fixups of the original live variables to their relocated selves - SmallVector live; - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; + SmallVector Live; + for (size_t i = 0; i < Records.size(); i++) { + PartiallyConstructedSafepointRecord &Info = Records[i]; + // We can't simply save the live set from the original insertion. One of // the live values might be the result of a call which needs a safepoint. // That Value* no longer exists and we need to use the new gc_result. - // Thankfully, the liveset is embedded in the statepoint (and updated), so + // Thankfully, the live set is embedded in the statepoint (and updated), so // we just grab that. - Statepoint statepoint(info.StatepointToken); - live.insert(live.end(), statepoint.gc_args_begin(), - statepoint.gc_args_end()); + Statepoint Statepoint(Info.StatepointToken); + Live.insert(Live.end(), Statepoint.gc_args_begin(), + Statepoint.gc_args_end()); #ifndef NDEBUG // Do some basic sanity checks on our liveness results before performing // relocation. Relocation can and will turn mistakes in liveness results // into non-sensical code which is must harder to debug. // TODO: It would be nice to test consistency as well - assert(DT.isReachableFromEntry(info.StatepointToken->getParent()) && + assert(DT.isReachableFromEntry(Info.StatepointToken->getParent()) && "statepoint must be reachable or liveness is meaningless"); - for (Value *V : statepoint.gc_args()) { + for (Value *V : Statepoint.gc_args()) { if (!isa(V)) // Non-instruction values trivial dominate all possible uses continue; - auto LiveInst = cast(V); + auto *LiveInst = cast(V); assert(DT.isReachableFromEntry(LiveInst->getParent()) && "unreachable values should never be live"); - assert(DT.dominates(LiveInst, info.StatepointToken) && + assert(DT.dominates(LiveInst, Info.StatepointToken) && "basic SSA liveness expectation violated by liveness analysis"); } #endif } - unique_unsorted(live); + unique_unsorted(Live); #ifndef NDEBUG // sanity check - for (auto ptr : live) { - assert(isGCPointerType(ptr->getType()) && "must be a gc pointer type"); - } + for (auto *Ptr : Live) + assert(isGCPointerType(Ptr->getType()) && "must be a gc pointer type"); #endif - relocationViaAlloca(F, DT, live, records); - return !records.empty(); + relocationViaAlloca(F, DT, Live, Records); + return !Records.empty(); } // Handles both return values and arguments for Functions and CallSites. template -static void RemoveDerefAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH, - unsigned Index) { +static void RemoveNonValidAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH, + unsigned Index) { AttrBuilder R; if (AH.getDereferenceableBytes(Index)) R.addAttribute(Attribute::get(Ctx, Attribute::Dereferenceable, @@ -2381,6 +2509,8 @@ static void RemoveDerefAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH, if (AH.getDereferenceableOrNullBytes(Index)) R.addAttribute(Attribute::get(Ctx, Attribute::DereferenceableOrNull, AH.getDereferenceableOrNullBytes(Index))); + if (AH.doesNotAlias(Index)) + R.addAttribute(Attribute::NoAlias); if (!R.empty()) AH.setAttributes(AH.getAttributes().removeAttributes( @@ -2388,18 +2518,18 @@ static void RemoveDerefAttrAtIndex(LLVMContext &Ctx, AttrHolder &AH, } void -RewriteStatepointsForGC::stripDereferenceabilityInfoFromPrototype(Function &F) { +RewriteStatepointsForGC::stripNonValidAttributesFromPrototype(Function &F) { LLVMContext &Ctx = F.getContext(); for (Argument &A : F.args()) if (isa(A.getType())) - RemoveDerefAttrAtIndex(Ctx, F, A.getArgNo() + 1); + RemoveNonValidAttrAtIndex(Ctx, F, A.getArgNo() + 1); if (isa(F.getReturnType())) - RemoveDerefAttrAtIndex(Ctx, F, AttributeSet::ReturnIndex); + RemoveNonValidAttrAtIndex(Ctx, F, AttributeSet::ReturnIndex); } -void RewriteStatepointsForGC::stripDereferenceabilityInfoFromBody(Function &F) { +void RewriteStatepointsForGC::stripNonValidAttributesFromBody(Function &F) { if (F.empty()) return; @@ -2429,9 +2559,9 @@ void RewriteStatepointsForGC::stripDereferenceabilityInfoFromBody(Function &F) { if (CallSite CS = CallSite(&I)) { for (int i = 0, e = CS.arg_size(); i != e; i++) if (isa(CS.getArgument(i)->getType())) - RemoveDerefAttrAtIndex(Ctx, CS, i + 1); + RemoveNonValidAttrAtIndex(Ctx, CS, i + 1); if (isa(CS.getType())) - RemoveDerefAttrAtIndex(Ctx, CS, AttributeSet::ReturnIndex); + RemoveNonValidAttrAtIndex(Ctx, CS, AttributeSet::ReturnIndex); } } } @@ -2450,17 +2580,17 @@ static bool shouldRewriteStatepointsIn(Function &F) { return false; } -void RewriteStatepointsForGC::stripDereferenceabilityInfo(Module &M) { +void RewriteStatepointsForGC::stripNonValidAttributes(Module &M) { #ifndef NDEBUG assert(std::any_of(M.begin(), M.end(), shouldRewriteStatepointsIn) && "precondition!"); #endif for (Function &F : M) - stripDereferenceabilityInfoFromPrototype(F); + stripNonValidAttributesFromPrototype(F); for (Function &F : M) - stripDereferenceabilityInfoFromBody(F); + stripNonValidAttributesFromBody(F); } bool RewriteStatepointsForGC::runOnFunction(Function &F) { @@ -2474,6 +2604,18 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F) { return false; DominatorTree &DT = getAnalysis(F).getDomTree(); + TargetTransformInfo &TTI = + getAnalysis().getTTI(F); + + auto NeedsRewrite = [](Instruction &I) { + if (UseDeoptBundles) { + if (ImmutableCallSite CS = ImmutableCallSite(&I)) + return !callsGCLeafFunction(CS); + return false; + } + + return isStatepoint(I); + }; // Gather all the statepoints which need rewritten. Be careful to only // consider those in reachable code since we need to ask dominance queries @@ -2482,7 +2624,7 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F) { bool HasUnreachableStatepoint = false; for (Instruction &I : instructions(F)) { // TODO: only the ones with the flag set! - if (isStatepoint(I)) { + if (NeedsRewrite(I)) { if (DT.isReachableFromEntry(I.getParent())) ParsePointNeeded.push_back(CallSite(&I)); else @@ -2544,7 +2686,7 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F) { } } - MadeChange |= insertParsePoints(F, DT, this, ParsePointNeeded); + MadeChange |= insertParsePoints(F, DT, TTI, ParsePointNeeded); return MadeChange; } @@ -2736,7 +2878,7 @@ static void findLiveSetAtInst(Instruction *Inst, GCPtrLivenessData &Data, // call result is not live (normal), nor are it's arguments // (unless they're used again later). This adjustment is // specifically what we need to relocate - BasicBlock::reverse_iterator rend(Inst); + BasicBlock::reverse_iterator rend(Inst->getIterator()); computeLiveInValues(BB->rbegin(), rend, LiveOut); LiveOut.erase(Inst); Out.insert(LiveOut.begin(), LiveOut.end()); @@ -2785,5 +2927,5 @@ static void recomputeLiveInValues(GCPtrLivenessData &RevisedLivenessData, assert(Updated.count(KVPair.first) && "record for non-live value"); #endif - Info.liveset = Updated; + Info.LiveSet = Updated; }