From 6a2f1276e50a462dfafdc579955684f6516f3cdb Mon Sep 17 00:00:00 2001 From: Sanjoy Das Date: Wed, 7 Oct 2015 02:39:18 +0000 Subject: [PATCH] [RS4GC] Cosmetic cleanup, NFC Summary: A series of cosmetic cleanup changes to RewriteStatepointsForGC: - Rename variables to LLVM style - Remove some redundant asserts - Remove an unsued `Pass *` parameter - Remove unnecessary variables - Use C++11 idioms where applicable - Pass CallSite by value, not reference Reviewers: reames, swaroop.sridhar Subscribers: llvm-commits, sanjoy Differential Revision: http://reviews.llvm.org/D13370 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@249508 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Scalar/RewriteStatepointsForGC.cpp | 456 ++++++++---------- 1 file changed, 211 insertions(+), 245 deletions(-) diff --git a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp index 2bb0918f6eb..17d969ed02a 100644 --- a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp +++ b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp @@ -158,15 +158,15 @@ 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 DenseSet StatepointLiveSetTy; typedef DenseMap 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. @@ -177,7 +177,7 @@ 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; }; @@ -245,7 +245,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()) { @@ -274,15 +274,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) @@ -290,9 +290,9 @@ 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; + result.LiveSet = LiveSet; } static bool isKnownBaseResult(Value *V); @@ -1143,7 +1143,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 @@ -1175,8 +1175,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 @@ -1292,9 +1292,9 @@ 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()) @@ -1330,176 +1330,170 @@ static void CreateGCRelocates(ArrayRef LiveVariables, } 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 { -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; } } @@ -1509,39 +1503,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) { + 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.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[L]; + 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); + makeStatepointExplicitImpl(CS, BaseVec, LiveVec, Result); CS.getInstruction()->eraseFromParent(); } // 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, @@ -1566,9 +1560,10 @@ 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, @@ -1609,10 +1604,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. @@ -1639,15 +1634,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) @@ -1656,20 +1648,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 @@ -1723,20 +1712,19 @@ static void relocationViaAlloca( 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())); @@ -1771,9 +1759,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)) { @@ -1796,14 +1784,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 @@ -1859,8 +1846,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 @@ -2065,8 +2052,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, @@ -2078,7 +2065,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)); @@ -2183,20 +2170,19 @@ 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) { + 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 +2192,23 @@ 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]; + for (CallSite CS : ToUpdate) { Statepoint StatepointCS(CS); SmallVector DeoptValues; @@ -2236,20 +2219,14 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P, 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 +2235,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 @@ -2278,49 +2254,46 @@ 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, 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,12 +2302,8 @@ static bool insertParsePoints(Function &F, DominatorTree &DT, Pass *P, 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++) + rematerializeLiveValues(ToUpdate[i], Records[i], TTI); // 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 @@ -2342,55 +2311,52 @@ 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); - } - toUpdate.clear(); // prevent accident use of invalid CallSites + for (size_t i = 0; i < Records.size(); i++) + makeStatepointExplicit(DT, ToUpdate[i], Records[i]); + + 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 +2774,5 @@ static void recomputeLiveInValues(GCPtrLivenessData &RevisedLivenessData, assert(Updated.count(KVPair.first) && "record for non-live value"); #endif - Info.liveset = Updated; + Info.LiveSet = Updated; } -- 2.34.1