[llvm] r203364 - [C++11] Add range based accessors for the Use-Def chain of a Value.

Duncan P. N. Exon Smith dexonsmith at apple.com
Tue Mar 18 16:19:08 PDT 2014


On Mar 8, 2014, at 7:16 PM, Chandler Carruth <chandlerc at gmail.com> wrote:

> Author: chandlerc
> Date: Sat Mar  8 21:16:01 2014
> New Revision: 203364
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=203364&view=rev
> Log:
> [C++11] Add range based accessors for the Use-Def chain of a Value.
> 
> This requires a number of steps.
> 1) Move value_use_iterator into the Value class as an implementation
>   detail
> 2) Change it to actually be a *Use* iterator rather than a *User*
>   iterator.
> 3) Add an adaptor which is a User iterator that always looks through the
>   Use to the User.
> 4) Wrap these in Value::use_iterator and Value::user_iterator typedefs.
> 5) Add the range adaptors as Value::uses() and Value::users().
> 6) Update *all* of the callers to correctly distinguish between whether
>   they wanted a use_iterator (and to explicitly dig out the User when
>   needed), or a user_iterator which makes the Use itself totally
>   opaque.
> 
> Because #6 requires churning essentially everything that walked the
> Use-Def chains, I went ahead and added all of the range adaptors and
> switched them to range-based loops where appropriate. Also because the
> renaming requires at least churning every line of code, it didn't make
> any sense to split these up into multiple commits -- all of which would
> touch all of the same lies of code.
> 
> The result is still not quite optimal. The Value::use_iterator is a nice
> regular iterator, but Value::user_iterator is an iterator over User*s
> rather than over the User objects themselves. As a consequence, it fits
> a bit awkwardly into the range-based world and it has the weird
> extra-dereferencing 'operator->' that so many of our iterators have.
> I think this could be fixed by providing something which transforms
> a range of T&s into a range of T*s, but that *can* be separated into
> another patch, and it isn't yet 100% clear whether this is the right
> move.
> 
> However, this change gets us most of the benefit and cleans up
> a substantial amount of code around Use and User. =]
> 
> Modified:
>    llvm/trunk/include/llvm/IR/CFG.h
>    llvm/trunk/include/llvm/IR/CallSite.h
>    llvm/trunk/include/llvm/IR/Instruction.h
>    llvm/trunk/include/llvm/IR/Instructions.h
>    llvm/trunk/include/llvm/IR/Value.h
>    llvm/trunk/lib/Analysis/CaptureTracking.cpp
>    llvm/trunk/lib/Analysis/IPA/GlobalsModRef.cpp
>    llvm/trunk/lib/Analysis/IPA/InlineCost.cpp
>    llvm/trunk/lib/Analysis/IVUsers.cpp
>    llvm/trunk/lib/Analysis/InstructionSimplify.cpp
>    llvm/trunk/lib/Analysis/LoopInfo.cpp
>    llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>    llvm/trunk/lib/Analysis/PHITransAddr.cpp
>    llvm/trunk/lib/Analysis/PtrUseVisitor.cpp
>    llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>    llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
>    llvm/trunk/lib/Analysis/SparsePropagation.cpp
>    llvm/trunk/lib/Analysis/ValueTracking.cpp
>    llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>    llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp
>    llvm/trunk/lib/Bitcode/Writer/ValueEnumerator.cpp
>    llvm/trunk/lib/CodeGen/CodeGenPrepare.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/FastISel.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
>    llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>    llvm/trunk/lib/CodeGen/SjLjEHPrepare.cpp
>    llvm/trunk/lib/CodeGen/StackProtector.cpp
>    llvm/trunk/lib/IR/AutoUpgrade.cpp
>    llvm/trunk/lib/IR/BasicBlock.cpp
>    llvm/trunk/lib/IR/Constants.cpp
>    llvm/trunk/lib/IR/Core.cpp
>    llvm/trunk/lib/IR/DebugInfo.cpp
>    llvm/trunk/lib/IR/Function.cpp
>    llvm/trunk/lib/IR/Instruction.cpp
>    llvm/trunk/lib/IR/Value.cpp
>    llvm/trunk/lib/IR/Verifier.cpp
>    llvm/trunk/lib/Target/Hexagon/HexagonRemoveSZExtArgs.cpp
>    llvm/trunk/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
>    llvm/trunk/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp
>    llvm/trunk/lib/Target/NVPTX/NVPTXLowerAggrCopies.cpp
>    llvm/trunk/lib/Target/NVPTX/NVVMReflect.cpp
>    llvm/trunk/lib/Target/XCore/XCoreLowerThreadLocal.cpp
>    llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp
>    llvm/trunk/lib/Transforms/IPO/DeadArgumentElimination.cpp
>    llvm/trunk/lib/Transforms/IPO/FunctionAttrs.cpp
>    llvm/trunk/lib/Transforms/IPO/GlobalOpt.cpp
>    llvm/trunk/lib/Transforms/IPO/IPConstantPropagation.cpp
>    llvm/trunk/lib/Transforms/IPO/Inliner.cpp
>    llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>    llvm/trunk/lib/Transforms/IPO/PartialInlining.cpp
>    llvm/trunk/lib/Transforms/IPO/StripSymbols.cpp
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineCasts.cpp
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineCompares.cpp
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
>    llvm/trunk/lib/Transforms/InstCombine/InstCombinePHI.cpp
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
>    llvm/trunk/lib/Transforms/InstCombine/InstCombineWorklist.h
>    llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
>    llvm/trunk/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp
>    llvm/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp
>    llvm/trunk/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
>    llvm/trunk/lib/Transforms/ObjCARC/ProvenanceAnalysis.cpp
>    llvm/trunk/lib/Transforms/Scalar/ConstantHoisting.cpp
>    llvm/trunk/lib/Transforms/Scalar/ConstantProp.cpp
>    llvm/trunk/lib/Transforms/Scalar/GVN.cpp
>    llvm/trunk/lib/Transforms/Scalar/IndVarSimplify.cpp
>    llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
>    llvm/trunk/lib/Transforms/Scalar/LICM.cpp
>    llvm/trunk/lib/Transforms/Scalar/LoopIdiomRecognize.cpp
>    llvm/trunk/lib/Transforms/Scalar/LoopInstSimplify.cpp
>    llvm/trunk/lib/Transforms/Scalar/LoopRerollPass.cpp
>    llvm/trunk/lib/Transforms/Scalar/LoopRotation.cpp
>    llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp
>    llvm/trunk/lib/Transforms/Scalar/LoopUnswitch.cpp
>    llvm/trunk/lib/Transforms/Scalar/MemCpyOptimizer.cpp
>    llvm/trunk/lib/Transforms/Scalar/Reassociate.cpp
>    llvm/trunk/lib/Transforms/Scalar/Reg2Mem.cpp
>    llvm/trunk/lib/Transforms/Scalar/SCCP.cpp
>    llvm/trunk/lib/Transforms/Scalar/SROA.cpp
>    llvm/trunk/lib/Transforms/Scalar/ScalarReplAggregates.cpp
>    llvm/trunk/lib/Transforms/Scalar/Sink.cpp
>    llvm/trunk/lib/Transforms/Scalar/StructurizeCFG.cpp
>    llvm/trunk/lib/Transforms/Scalar/TailRecursionElimination.cpp
>    llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp
>    llvm/trunk/lib/Transforms/Utils/DemoteRegToStack.cpp
>    llvm/trunk/lib/Transforms/Utils/GlobalStatus.cpp
>    llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
>    llvm/trunk/lib/Transforms/Utils/LCSSA.cpp
>    llvm/trunk/lib/Transforms/Utils/Local.cpp
>    llvm/trunk/lib/Transforms/Utils/LowerInvoke.cpp
>    llvm/trunk/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
>    llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
>    llvm/trunk/lib/Transforms/Utils/SimplifyIndVar.cpp
>    llvm/trunk/lib/Transforms/Utils/SimplifyInstructions.cpp
>    llvm/trunk/lib/Transforms/Utils/SimplifyLibCalls.cpp
>    llvm/trunk/lib/Transforms/Vectorize/BBVectorize.cpp
>    llvm/trunk/lib/Transforms/Vectorize/LoopVectorize.cpp
>    llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp
>    llvm/trunk/tools/opt/AnalysisWrappers.cpp
> 


> Modified: llvm/trunk/lib/Target/NVPTX/NVPTXLowerAggrCopies.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/NVPTX/NVPTXLowerAggrCopies.cpp?rev=203364&r1=203363&r2=203364&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/NVPTX/NVPTXLowerAggrCopies.cpp (original)
> +++ llvm/trunk/lib/Target/NVPTX/NVPTXLowerAggrCopies.cpp Sat Mar  8 21:16:01 2014
> @@ -123,7 +123,7 @@ bool NVPTXLowerAggrCopies::runOnFunction
>         if (DL->getTypeStoreSize(load->getType()) < MaxAggrCopySize)
>           continue;
> 
> -        User *use = *(load->use_begin());
> +        User *use = load->user_back();

This looks strange (begin => back).  Are you sure?

>         if (StoreInst *store = dyn_cast<StoreInst>(use)) {
>           if (store->getOperand(0) != load) //getValueOperand
>             continue;
> @@ -163,7 +163,7 @@ bool NVPTXLowerAggrCopies::runOnFunction
>   //
>   for (unsigned i = 0, e = aggrLoads.size(); i != e; ++i) {
>     LoadInst *load = aggrLoads[i];
> -    StoreInst *store = dyn_cast<StoreInst>(*load->use_begin());
> +    StoreInst *store = dyn_cast<StoreInst>(*load->user_begin());
>     Value *srcAddr = load->getOperand(0);
>     Value *dstAddr = store->getOperand(1);
>     unsigned numLoads = DL->getTypeStoreSize(load->getType());
> 
> 

> Modified: llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp?rev=203364&r1=203363&r2=203364&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp Sat Mar  8 21:16:01 2014
> @@ -641,10 +641,9 @@ Instruction *InstCombiner::FoldOpIntoPhi
>   // uses into the PHI.
>   if (!PN->hasOneUse()) {
>     // Walk the use list for the instruction, comparing them to I.
> -    for (Value::use_iterator UI = PN->use_begin(), E = PN->use_end();
> -         UI != E; ++UI) {
> -      Instruction *User = cast<Instruction>(*UI);
> -      if (User != &I && !I.isIdenticalTo(User))
> +    for (User *U : PN->users()) {
> +      Instruction *UI = cast<Instruction>(U);
> +      if (UI != &I && !I.isIdenticalTo(UI))
>         return 0;
>     }
>     // Otherwise, we can replace *all* users with the new PHI we form.
> @@ -759,8 +758,7 @@ Instruction *InstCombiner::FoldOpIntoPhi
>     }
>   }
> 
> -  for (Value::use_iterator UI = PN->use_begin(), E = PN->use_end();
> -       UI != E; ) {
> +  for (auto UI = PN->user_begin(), E = PN->user_end(); UI != E;) {
>     Instruction *User = cast<Instruction>(*UI++);
>     if (User == &I) continue;
>     ReplaceInstUsesWith(*User, NewPN);
> @@ -1080,7 +1078,7 @@ Value *InstCombiner::Descale(Value *Val,
> 
>     // Move up one level in the expression.
>     assert(Ancestor->hasOneUse() && "Drilled down when more than one use!");
> -    Ancestor = Ancestor->use_back();
> +    Ancestor = Ancestor->user_back();
>   } while (1);
> }
> 
> @@ -1425,9 +1423,8 @@ isAllocSiteRemovable(Instruction *AI, Sm
> 
>   do {
>     Instruction *PI = Worklist.pop_back_val();
> -    for (Value::use_iterator UI = PI->use_begin(), UE = PI->use_end(); UI != UE;
> -         ++UI) {
> -      Instruction *I = cast<Instruction>(*UI);
> +    for (User *U : PI->users()) {
> +      Instruction *I = cast<Instruction>(U);
>       switch (I->getOpcode()) {
>       default:
>         // Give up the moment we see something we can't handle.
> @@ -2404,12 +2401,12 @@ bool InstCombiner::DoOneIteration(Functi
>     // See if we can trivially sink this instruction to a successor basic block.
>     if (I->hasOneUse()) {
>       BasicBlock *BB = I->getParent();
> -      Instruction *UserInst = cast<Instruction>(I->use_back());
> +      Instruction *UserInst = cast<Instruction>(*I->user_begin());

Same thing here.  Maybe it’s correct, it’s just a little strange.

>       BasicBlock *UserParent;
> 
>       // Get the block the use occurs in.
>       if (PHINode *PN = dyn_cast<PHINode>(UserInst))
> -        UserParent = PN->getIncomingBlock(I->use_begin().getUse());
> +        UserParent = PN->getIncomingBlock(*I->use_begin());
>       else
>         UserParent = UserInst->getParent();
> 
> 
> 


> Modified: llvm/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp?rev=203364&r1=203363&r2=203364&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp (original)
> +++ llvm/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp Sat Mar  8 21:16:01 2014
> @@ -440,17 +440,17 @@ bool ObjCARCContract::runOnFunction(Func
> 
>     // Don't use GetObjCArg because we don't want to look through bitcasts
>     // and such; to do the replacement, the argument must have type i8*.
> -    const Value *Arg = cast<CallInst>(Inst)->getArgOperand(0);
> +    Value *Arg = cast<CallInst>(Inst)->getArgOperand(0);
>     for (;;) {
>       // If we're compiling bugpointed code, don't get in trouble.
>       if (!isa<Instruction>(Arg) && !isa<Argument>(Arg))
>         break;
>       // Look through the uses of the pointer.
> -      for (Value::const_use_iterator UI = Arg->use_begin(), UE = Arg->use_end();
> +      for (Value::use_iterator UI = Arg->use_begin(), UE = Arg->use_end();
>            UI != UE; ) {
> -        Use &U = UI.getUse();
> -        unsigned OperandNo = UI.getOperandNo();
> -        ++UI; // Increment UI now, because we may unlink its element.
> +        // Increment UI now, because we may unlink its element.
> +        Use &U = *UI++;
> +        unsigned OperandNo = U.getOperandNo();
> 
>         // If the call's return value dominates a use of the call's argument
>         // value, rewrite the use to use the return value. We check for
> @@ -476,8 +476,7 @@ bool ObjCARCContract::runOnFunction(Func
>               if (PHI->getIncomingBlock(i) == BB) {
>                 // Keep the UI iterator valid.
>                 if (&PHI->getOperandUse(
> -                      PHINode::getOperandNumForIncomingValue(i)) ==
> -                    &UI.getUse())
> +                        PHINode::getOperandNumForIncomingValue(i)) == &U)

This introduces a subtle bug, since UI has been incremented already.  Fixed in
r204195.

>                   ++UI;
>                 PHI->setIncomingValue(i, Replacement);
>               }
> 
> 

> Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=203364&r1=203363&r2=203364&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Sat Mar  8 21:16:01 2014
> @@ -1590,10 +1590,9 @@ static bool BlockIsSimpleEnoughToThreadT
> 
>     // We can only support instructions that do not define values that are
>     // live outside of the current basic block.
> -    for (Value::use_iterator UI = BBI->use_begin(), E = BBI->use_end();
> -         UI != E; ++UI) {
> -      Instruction *U = cast<Instruction>(*UI);
> -      if (U->getParent() != BB || isa<PHINode>(U)) return false;
> +    for (User *U : BBI->users()) {
> +      Instruction *UI = cast<Instruction>(U);
> +      if (UI->getParent() != BB || isa<PHINode>(UI)) return false;
>     }
> 
>     // Looks ok, continue checking.
> @@ -2016,7 +2015,7 @@ bool llvm::FoldBranchToCommonDest(Branch
>   // register pressure or inhibit out-of-order execution.
>   Instruction *BonusInst = 0;
>   if (&*FrontIt != Cond &&
> -      FrontIt->hasOneUse() && *FrontIt->use_begin() == Cond &&
> +      FrontIt->hasOneUse() && FrontIt->user_back() == Cond &&

Another begin => back.

>       isSafeToSpeculativelyExecute(FrontIt)) {
>     BonusInst = &*FrontIt;
>     ++FrontIt;
> @@ -2095,7 +2094,7 @@ bool llvm::FoldBranchToCommonDest(Branch
>     // instructions that are used by the terminator's condition because it
>     // exposes more merging opportunities.
>     bool UsedByBranch = (BonusInst && BonusInst->hasOneUse() &&
> -                         *BonusInst->use_begin() == Cond);
> +                         BonusInst->user_back() == Cond);

Another begin => back.

> 
>     if (BonusInst && !UsedByBranch) {
>       // Collect the values used by the bonus inst
> @@ -2689,7 +2688,7 @@ static bool TryToSimplifyUncondBranchWit
>   // The use of the icmp has to be in the 'end' block, by the only PHI node in
>   // the block.
>   BasicBlock *SuccBlock = BB->getTerminator()->getSuccessor(0);
> -  PHINode *PHIUse = dyn_cast<PHINode>(ICI->use_back());
> +  PHINode *PHIUse = dyn_cast<PHINode>(ICI->user_back());
>   if (PHIUse == 0 || PHIUse != &SuccBlock->front() ||
>       isa<PHINode>(++BasicBlock::iterator(PHIUse)))
>     return false;
> @@ -3807,8 +3806,8 @@ static bool SwitchToLookupTable(SwitchIn
> 
>     // If the result is used to return immediately from the function, we want to
>     // do that right here.
> -    if (PHI->hasOneUse() && isa<ReturnInst>(*PHI->use_begin()) &&
> -        *PHI->use_begin() == CommonDest->getFirstNonPHIOrDbg()) {
> +    if (PHI->hasOneUse() && isa<ReturnInst>(*PHI->user_begin()) &&
> +        PHI->user_back() == CommonDest->getFirstNonPHIOrDbg()) {
>       Builder.CreateRet(Result);
>       ReturnedEarly = true;
>       break;
> @@ -4043,7 +4042,7 @@ static bool passingValueIsAlwaysUndefine
> 
>   if (C->isNullValue()) {
>     // Only look at the first use, avoid hurting compile time with long uselists
> -    User *Use = *I->use_begin();
> +    User *Use = *I->user_begin();
> 
>     // Now make sure that there are no instructions in between that can alter
>     // control flow (eg. calls)
> 
> 

If the begin => back *is* a problem, I probably didn’t catch them all. I was
mainly checking for other subtle problems with incrementing past the end.



More information about the llvm-commits mailing list