Index: lib/Transforms/Scalar/RewriteStatepointsForGC.cpp =================================================================== --- lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -163,7 +163,7 @@ 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; @@ -177,7 +177,7 @@ 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; }; @@ -274,15 +274,15 @@ 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) @@ -290,9 +290,9 @@ } 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; + result.LiveSet = LiveSet; } static bool isKnownBaseResult(Value *V); @@ -1176,7 +1176,7 @@ const CallSite &CS, PartiallyConstructedSafepointRecord &result) { DenseMap PointerToBase; - findBasePointers(result.liveset, PointerToBase, &DT, DVCache); + findBasePointers(result.LiveSet, PointerToBase, &DT, DVCache); if (PrintBasePointers) { // Note: Need to print these in a stable order since this is checked in @@ -1330,150 +1330,141 @@ } static void -makeStatepointExplicitImpl(const CallSite &CS, /* to replace */ - const SmallVectorImpl &basePtrs, - const SmallVectorImpl &liveVariables, - Pass *P, - PartiallyConstructedSafepointRecord &result) { - assert(basePtrs.size() == liveVariables.size()); +makeStatepointExplicitImpl(const CallSite CS, /* to replace */ + const SmallVectorImpl &BasePtrs, + const SmallVectorImpl &LiveVariables, + PartiallyConstructedSafepointRecord &Result) { + assert(BasePtrs.size() == LiveVariables.size()); assert(isStatepoint(CS) && "This method expects to be rewriting a statepoint"); 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"); // 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(); + Function *GCStatepointDecl = CS.getCalledFunction(); // 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); + 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()); + 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()); + // Add all the pointers to be relocated (gc arguments) and capture the start + // of the live variable list for use in the gc_relocates + const int LiveStartIdx = Args.size(); + Args.insert(Args.end(), LiveVariables.begin(), LiveVariables.end()); // 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.CreateCall(GCStatepointDecl, Args, "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 = + InvokeInst::Create(GCStatepointDecl, ToReplace->getNormalDest(), + ToReplace->getUnwindDest(), Args, "statepoint_token", + ToReplace->getParent()); + 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 = + Instruction *ExceptionalToken = cast(Builder.CreateExtractValue( - unwindBlock->getLandingPadInst(), idx, "relocate_token")); - result.UnwindToken = exceptional_token; + UnwindBlock->getLandingPadInst(), 1, "relocate_token")); + Result.UnwindToken = ExceptionalToken; - CreateGCRelocates(liveVariables, live_start, basePtrs, - exceptional_token, Builder); + 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); + assert(Token && "Should be set in one of the above branches!"); // Take the name of the original value call if it had one. - token->takeName(CS.getInstruction()); + Token->takeName(CS.getInstruction()); // The GCResult is already inserted, we just need to find it #ifndef NDEBUG - Instruction *toReplace = CS.getInstruction(); - assert((toReplace->hasNUses(0) || toReplace->hasNUses(1)) && + Instruction *ToReplace = CS.getInstruction(); + assert(!ToReplace->hasNUsesOrMore(2) && "only valid use before rewrite is gc.result"); - assert(!toReplace->hasOneUse() || - isGCResult(cast(*toReplace->user_begin()))); + assert(!ToReplace->hasOneUse() || + isGCResult(cast(*ToReplace->user_begin()))); #endif // 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()->replaceAllUsesWith(Token); - result.StatepointToken = token; + Result.StatepointToken = Token; // Second, create a gc.relocate for every live variable - CreateGCRelocates(liveVariables, live_start, basePtrs, token, Builder); + CreateGCRelocates(LiveVariables, LiveStartIdx, BasePtrs, Token, Builder); } namespace { @@ -1511,14 +1502,14 @@ static void makeStatepointExplicit(DominatorTree &DT, const CallSite &CS, Pass *P, PartiallyConstructedSafepointRecord &result) { - auto liveset = result.liveset; + auto LiveSet = result.LiveSet; 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.reserve(LiveSet.size()); + basevec.reserve(LiveSet.size()); + for (Value *L : LiveSet) { livevec.push_back(L); assert(PointerToBase.count(L)); Value *base = PointerToBase[L]; @@ -1532,7 +1523,7 @@ stablize_order(basevec, livevec); // Do the actual rewriting and delete the old statepoint - makeStatepointExplicitImpl(CS, basevec, livevec, P, result); + makeStatepointExplicitImpl(CS, basevec, livevec, result); CS.getInstruction()->eraseFromParent(); } @@ -1859,8 +1850,8 @@ } } -/// 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 @@ -2065,8 +2056,8 @@ 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, @@ -2078,7 +2069,7 @@ // 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)); @@ -2183,20 +2174,19 @@ // 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) { + 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"); } @@ -2206,26 +2196,23 @@ // 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]; + for (CallSite CS : ToUpdate) { Statepoint StatepointCS(CS); SmallVector DeoptValues; @@ -2236,20 +2223,14 @@ if (isHandledGCPointerType(Arg->getType())) DeoptValues.push_back(Arg); } - insertUseHolderAfter(CS, DeoptValues, holders); + 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); - } - 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, P, ToUpdate, Records); // B) Find the base pointers for each live pointer /* scope for caching */ { @@ -2258,10 +2239,9 @@ // 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 @@ -2278,49 +2258,46 @@ // 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, P, 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) { + for (auto Pair : Info.PointerToBase) errs() << " derived %" << Pair.first->getName() << " base %" << Pair.second->getName() << "\n"; - } } } - for (size_t i = 0; i < holders.size(); i++) { - holders[i]->eraseFromParent(); - holders[i] = nullptr; - } - holders.clear(); + + 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 @@ -2329,11 +2306,9 @@ TargetTransformInfo &TTI = P->getAnalysis().getTTI(F); - for (size_t i = 0; i < records.size(); i++) { - struct PartiallyConstructedSafepointRecord &info = records[i]; - CallSite &CS = toUpdate[i]; - - rematerializeLiveValues(CS, info, TTI); + for (size_t i = 0; i < Records.size(); i++) { + PartiallyConstructedSafepointRecord &Info = Records[i]; + rematerializeLiveValues(ToUpdate[i], Info, TTI); } // Now run through and replace the existing statepoints with new ones with @@ -2342,55 +2317,53 @@ // 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++) { + PartiallyConstructedSafepointRecord &Info = Records[i]; + makeStatepointExplicit(DT, ToUpdate[i], P, Info); } - toUpdate.clear(); // prevent accident use of invalid CallSites + 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. @@ -2808,5 +2781,5 @@ assert(Updated.count(KVPair.first) && "record for non-live value"); #endif - Info.liveset = Updated; + Info.LiveSet = Updated; }