[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