[llvm] r316054 - [ScalarEvolution] Handling for ICmp occuring in the evolution chain.

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 17 22:34:17 PDT 2017


Hi Jatin,

You're correct about the policy, in that an LGTM from one reviewer is
usually enough unless the change is large in scope or complexity.
However, at the risk of sounding a bit crass, I'd like to point out
that junryoungju has not yet made any commits to LLVM; and landing a
change based on an LGTM from her/him is only following the policy in
letter, but not in spirit.

-- Sanjoy

On Tue, Oct 17, 2017 at 10:19 PM, Jatin Bhateja <jatin.bhateja at gmail.com> wrote:
>
> It was accepted by one reviewer which is why it was landed as per policy.
>
> We can do a post commit review also as per developer's policy.
>
> Do let me know what comments you have and it could be incorporated.
>
>
> On 18 Oct 2017 10:22, "Sanjoy Das" <sanjoy at playingwithpointers.com> wrote:
>>
>> Hi Jatin,
>>
>> This hasn't been LGTMed. Please revert it for now while we continue the
>> review.
>>
>> -- Sanjoy
>>
>> On Tue, Oct 17, 2017 at 6:36 PM Jatin Bhateja via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>>
>>> Author: jbhateja
>>> Date: Tue Oct 17 18:36:16 2017
>>> New Revision: 316054
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=316054&view=rev
>>> Log:
>>> [ScalarEvolution] Handling for ICmp occuring in the evolution chain.
>>>
>>> Summary:
>>>  If a compare instruction is same or inverse of the compare in the
>>>  branch of the loop latch, then return a constant evolution node.
>>>  Currently scope of evaluation is limited to SCEV computation for
>>>  PHI nodes.
>>>
>>>  This shall facilitate computations of loop exit counts in cases
>>>  where compare appears in the evolution chain of induction variables.
>>>
>>>  Will fix PR 34538
>>> Reviewers: sanjoy, hfinkel, junryoungju
>>>
>>> Reviewed By: junryoungju
>>>
>>> Subscribers: javed.absar, llvm-commits
>>>
>>> Differential Revision: https://reviews.llvm.org/D38494
>>>
>>> Added:
>>>     llvm/trunk/test/Analysis/ScalarEvolution/pr34538.ll
>>> Modified:
>>>     llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
>>>     llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>>>     llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp
>>>
>>> Modified: llvm/trunk/include/llvm/Analysis/ScalarEvolution.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ScalarEvolution.h?rev=316054&r1=316053&r2=316054&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Analysis/ScalarEvolution.h (original)
>>> +++ llvm/trunk/include/llvm/Analysis/ScalarEvolution.h Tue Oct 17
>>> 18:36:16 2017
>>> @@ -1378,6 +1378,9 @@ private:
>>>    /// Helper function called from createNodeForPHI.
>>>    const SCEV *createAddRecFromPHI(PHINode *PN);
>>>
>>> +  /// Evaluate ICmpInst to a constant node for special patterns.
>>> +  const SCEV *evaluateForICmp(ICmpInst *IC);
>>> +
>>>    /// A helper function for createAddRecFromPHI to handle simple cases.
>>>    const SCEV *createSimpleAffineAddRec(PHINode *PN, Value *BEValueV,
>>>                                              Value *StartValueV);
>>>
>>> Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolution.cpp?rev=316054&r1=316053&r2=316054&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
>>> +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Tue Oct 17 18:36:16 2017
>>> @@ -4756,11 +4756,26 @@ const SCEV *ScalarEvolution::createAddRe
>>>            Ops.push_back(Add->getOperand(i));
>>>        const SCEV *Accum = getAddExpr(Ops);
>>>
>>> +      bool InvariantF = isLoopInvariant(Accum, L);
>>> +
>>> +      if (!InvariantF && Accum->getSCEVType() == scZeroExtend) {
>>> +        const SCEV *Op =
>>> dyn_cast<SCEVZeroExtendExpr>(Accum)->getOperand();
>>> +        const SCEVUnknown *Un = dyn_cast<SCEVUnknown>(Op);
>>> +        if (Un && Un->getValue() && isa<Instruction>(Un->getValue()) &&
>>> +            dyn_cast<Instruction>(Un->getValue())->getOpcode() ==
>>> +                Instruction::ICmp) {
>>> +          const SCEV *ICmpSC =
>>> evaluateForICmp(cast<ICmpInst>(Un->getValue()));
>>> +          bool IsConstSC = ICmpSC->getSCEVType() == scConstant;
>>> +          Accum =
>>> +              IsConstSC ? getZeroExtendExpr(ICmpSC, Accum->getType()) :
>>> Accum;
>>> +          InvariantF = IsConstSC ? true : false;
>>> +        }
>>> +      }
>>> +
>>>        // This is not a valid addrec if the step amount is varying each
>>>        // loop iteration, but is not itself an addrec in this loop.
>>> -      if (isLoopInvariant(Accum, L) ||
>>> -          (isa<SCEVAddRecExpr>(Accum) &&
>>> -           cast<SCEVAddRecExpr>(Accum)->getLoop() == L)) {
>>> +      if (InvariantF || (isa<SCEVAddRecExpr>(Accum) &&
>>> +                         cast<SCEVAddRecExpr>(Accum)->getLoop() == L)) {
>>>          SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap;
>>>
>>>          if (auto BO = MatchBinaryOp(BEValueV, DT)) {
>>> @@ -6443,6 +6458,30 @@ void ScalarEvolution::forgetLoop(const L
>>>    }
>>>  }
>>>
>>> +
>>> +const SCEV *ScalarEvolution::evaluateForICmp(ICmpInst *IC) {
>>> +  BasicBlock *Latch = nullptr;
>>> +  const Loop *L = LI.getLoopFor(IC->getParent());
>>> +
>>> +  // If compare instruction is same or inverse of the compare in the
>>> +  // branch of the loop latch, then return a constant evolution
>>> +  // node. This shall facilitate computations of loop exit counts
>>> +  // in cases where compare appears in the evolution chain of induction
>>> +  // variables.
>>> +  if (L && (Latch = L->getLoopLatch())) {
>>> +    BranchInst *BI = dyn_cast<BranchInst>(Latch->getTerminator());
>>> +    if (BI && BI->isConditional() && BI->getCondition() == IC) {
>>> +      if (BI->getSuccessor(0) != L->getHeader())
>>> +        return getZero(Type::getInt1Ty(getContext()));
>>> +      else
>>> +        return getOne(Type::getInt1Ty(getContext()));
>>> +    }
>>> +  }
>>> +
>>> +  return getUnknown(IC);
>>> +}
>>> +
>>> +
>>>  void ScalarEvolution::forgetValue(Value *V) {
>>>    Instruction *I = dyn_cast<Instruction>(V);
>>>    if (!I) return;
>>>
>>> Modified: llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp?rev=316054&r1=316053&r2=316054&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp (original)
>>> +++ llvm/trunk/lib/Transforms/Scalar/LoopStrengthReduce.cpp Tue Oct 17
>>> 18:36:16 2017
>>> @@ -2973,8 +2973,11 @@ void LSRInstance::CollectChains() {
>>>        // Ignore users that are part of a SCEV expression. This way we
>>> only
>>>        // consider leaf IV Users. This effectively rediscovers a portion
>>> of
>>>        // IVUsers analysis but in program order this time.
>>> -      if (SE.isSCEVable(I.getType()) &&
>>> !isa<SCEVUnknown>(SE.getSCEV(&I)))
>>> -        continue;
>>> +      if (SE.isSCEVable(I.getType())) {
>>> +        const SCEV *SI = SE.getSCEV(&I);
>>> +        if (!isa<SCEVUnknown>(SI) && !isa<SCEVConstant>(SI))
>>> +          continue;
>>> +      }
>>>
>>>        // Remove this instruction from any NearUsers set it may be in.
>>>        for (unsigned ChainIdx = 0, NChains = IVChainVec.size();
>>>
>>> Added: llvm/trunk/test/Analysis/ScalarEvolution/pr34538.ll
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Analysis/ScalarEvolution/pr34538.ll?rev=316054&view=auto
>>>
>>> ==============================================================================
>>> --- llvm/trunk/test/Analysis/ScalarEvolution/pr34538.ll (added)
>>> +++ llvm/trunk/test/Analysis/ScalarEvolution/pr34538.ll Tue Oct 17
>>> 18:36:16 2017
>>> @@ -0,0 +1,19 @@
>>> +; RUN: opt -S -scalar-evolution -loop-deletion -simplifycfg -analyze <
>>> %s | FileCheck %s --check-prefix=CHECK-ANALYSIS
>>> +
>>> +define i32 @foo() local_unnamed_addr #0 {
>>> +; CHECK-ANALYSIS: Loop %do.body: backedge-taken count is 10000
>>> +; CHECK-ANALYSIS: Loop %do.body: max backedge-taken count is 10000
>>> +; CHECK-ANALYSIS: Loop %do.body: Predicated backedge-taken count is
>>> 10000
>>> +entry:
>>> +  br label %do.body
>>> +
>>> +do.body:                                          ; preds = %do.body,
>>> %entry
>>> +  %start.0 = phi i32 [ 0, %entry ], [ %inc.start.0, %do.body ]
>>> +  %cmp = icmp slt i32 %start.0, 10000
>>> +  %inc = zext i1 %cmp to i32
>>> +  %inc.start.0 = add nsw i32 %start.0, %inc
>>> +  br i1 %cmp, label %do.body, label %do.end
>>> +
>>> +do.end:                                           ; preds = %do.body
>>> +  ret i32 0
>>> +}
>>>
>>>
>>> _______________________________________________
>>> 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