[llvm] r247497 - Clean up: Refactoring the hardcoded value of 6 for FindAvailableLoadedValue()'s parameter MaxInstsToScan.
Daniel Berlin via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 10:59:10 PDT 2015
Why did this get submitted without code review?
On Fri, Sep 11, 2015 at 6:41 PM, Larisse Voufo via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: lvoufo
> Date: Fri Sep 11 20:41:55 2015
> New Revision: 247497
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247497&view=rev
> Log:
> Clean up: Refactoring the hardcoded value of 6 for FindAvailableLoadedValue()'s parameter MaxInstsToScan.
>
> Modified:
> llvm/trunk/include/llvm/Analysis/Loads.h
> llvm/trunk/lib/Analysis/Lint.cpp
> llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
>
> Modified: llvm/trunk/include/llvm/Analysis/Loads.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/Loads.h?rev=247497&r1=247496&r2=247497&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/Loads.h (original)
> +++ llvm/trunk/include/llvm/Analysis/Loads.h Fri Sep 11 20:41:55 2015
> @@ -29,15 +29,28 @@ class MDNode;
> bool isSafeToLoadUnconditionally(Value *V, Instruction *ScanFrom,
> unsigned Align);
>
> +/// DEF_MAX_INSTS_TO_SCAN - the default number of maximum instructions
> +/// to scan in the block, used by FindAvailableLoadedValue().
> +/// FindAvailableLoadedValue() was introduced in r60148, to improve jump
> +/// threading in part by eliminating partially redundant loads.
> +/// At that point, the value of MaxInstsToScan was already set to '6'
> +/// without documented explanation.
> +/// FIXME: Ask r60148 author for details, and complete this documentation.
> +/// NOTE: As of now, every use of FindAvailableLoadedValue() uses this default
> +/// value of '6'.
> +#ifndef DEF_MAX_INSTS_TO_SCAN
> +#define DEF_MAX_INSTS_TO_SCAN 6
> +#endif
> +
> /// FindAvailableLoadedValue - Scan the ScanBB block backwards (starting at
> /// the instruction before ScanFrom) checking to see if we have the value at
> /// the memory address *Ptr locally available within a small number of
> /// instructions. If the value is available, return it.
> ///
> -/// If not, return the iterator for the last validated instruction that the
> +/// If not, return the iterator for the last validated instruction that the
> /// value would be live through. If we scanned the entire block and didn't
> /// find something that invalidates *Ptr or provides it, ScanFrom would be
> -/// left at begin() and this returns null. ScanFrom could also be left
> +/// left at begin() and this returns null. ScanFrom could also be left
> ///
> /// MaxInstsToScan specifies the maximum instructions to scan in the block.
> /// If it is set to 0, it will scan the whole block. You can also optionally
> @@ -48,7 +61,7 @@ bool isSafeToLoadUnconditionally(Value *
> /// is found, it is left unmodified.
> Value *FindAvailableLoadedValue(Value *Ptr, BasicBlock *ScanBB,
> BasicBlock::iterator &ScanFrom,
> - unsigned MaxInstsToScan = 6,
> + unsigned MaxInstsToScan = DEF_MAX_INSTS_TO_SCAN,
> AliasAnalysis *AA = nullptr,
> AAMDNodes *AATags = nullptr);
>
>
> Modified: llvm/trunk/lib/Analysis/Lint.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/Lint.cpp?rev=247497&r1=247496&r2=247497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Analysis/Lint.cpp (original)
> +++ llvm/trunk/lib/Analysis/Lint.cpp Fri Sep 11 20:41:55 2015
> @@ -829,8 +829,9 @@ Value *Lint::findValueImpl(Value *V, boo
> for (;;) {
> if (!VisitedBlocks.insert(BB).second)
> break;
> - if (Value *U = FindAvailableLoadedValue(L->getPointerOperand(),
> - BB, BBI, 6, AA))
> + if (Value *U =
> + FindAvailableLoadedValue(L->getPointerOperand(),
> + BB, BBI, DEF_MAX_INSTS_TO_SCAN, AA))
> return findValueImpl(U, OffsetOk, Visited);
> if (BBI != BB->begin()) break;
> BB = BB->getUniquePredecessor();
>
> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=247497&r1=247496&r2=247497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)
> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Fri Sep 11 20:41:55 2015
> @@ -750,8 +750,9 @@ Instruction *InstCombiner::visitLoadInst
> // separated by a few arithmetic operations.
> BasicBlock::iterator BBI = &LI;
> AAMDNodes AATags;
> - if (Value *AvailableVal = FindAvailableLoadedValue(Op, LI.getParent(), BBI,
> - 6, AA, &AATags)) {
> + if (Value *AvailableVal =
> + FindAvailableLoadedValue(Op, LI.getParent(), BBI,
> + DEF_MAX_INSTS_TO_SCAN, AA, &AATags)) {
> if (LoadInst *NLI = dyn_cast<LoadInst>(AvailableVal)) {
> unsigned KnownIDs[] = {
> LLVMContext::MD_tbaa,
> @@ -822,7 +823,7 @@ Instruction *InstCombiner::visitLoadInst
> }
>
> // load (select (cond, null, P)) -> load P
> - if (isa<ConstantPointerNull>(SI->getOperand(1)) &&
> + if (isa<ConstantPointerNull>(SI->getOperand(1)) &&
> LI.getPointerAddressSpace() == 0) {
> LI.setOperand(0, SI->getOperand(2));
> return &LI;
>
> Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=247497&r1=247496&r2=247497&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Fri Sep 11 20:41:55 2015
> @@ -769,7 +769,7 @@ bool JumpThreading::ProcessBlock(BasicBl
> // If we're branching on a conditional, LVI might be able to determine
> // it's value at the branch instruction. We only handle comparisons
> // against a constant at this time.
> - // TODO: This should be extended to handle switches as well.
> + // TODO: This should be extended to handle switches as well.
> BranchInst *CondBr = dyn_cast<BranchInst>(BB->getTerminator());
> Constant *CondConst = dyn_cast<Constant>(CondCmp->getOperand(1));
> if (CondBr && CondConst && CondBr->isConditional()) {
> @@ -877,7 +877,7 @@ bool JumpThreading::SimplifyPartiallyRed
> BasicBlock::iterator BBIt = LI;
>
> if (Value *AvailableVal =
> - FindAvailableLoadedValue(LoadedPtr, LoadBB, BBIt, 6)) {
> + FindAvailableLoadedValue(LoadedPtr, LoadBB, BBIt)) {
> // If the value if the load is locally available within the block, just use
> // it. This frequently occurs for reg2mem'd allocas.
> //cerr << "LOAD ELIMINATED:\n" << *BBIt << *LI << "\n";
> @@ -922,7 +922,8 @@ bool JumpThreading::SimplifyPartiallyRed
> // Scan the predecessor to see if the value is available in the pred.
> BBIt = PredBB->end();
> AAMDNodes ThisAATags;
> - Value *PredAvailable = FindAvailableLoadedValue(LoadedPtr, PredBB, BBIt, 6,
> + Value *PredAvailable = FindAvailableLoadedValue(LoadedPtr, PredBB, BBIt,
> + DEF_MAX_INSTS_TO_SCAN,
> nullptr, &ThisAATags);
> if (!PredAvailable) {
> OneUnavailablePred = PredBB;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list