[llvm] r323077 - [SCEV] Fix isLoopEntryGuardedByCond usage
Serguei Katkov via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 23 05:20:54 PST 2018
Yes, it makes sense to me. Implemented: https://reviews.llvm.org/D42417
Thank you,
Serguei.
> -----Original Message-----
> From: Philip Reames [mailto:listmail at philipreames.com]
> Sent: Tuesday, January 23, 2018 1:30 AM
> To: Serguei Katkov <serguei.katkov at azul.com>; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r323077 - [SCEV] Fix isLoopEntryGuardedByCond usage
>
> Given this appears to be most every callsite of isLoopEntryGuardedByCond,
> would it makes sense to just have the function return false if the values
> aren't available?
>
> Philip
>
>
> On 01/21/2018 11:31 PM, Serguei Katkov via llvm-commits wrote:
> > Author: skatkov
> > Date: Sun Jan 21 23:31:41 2018
> > New Revision: 323077
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=323077&view=rev
> > Log:
> > [SCEV] Fix isLoopEntryGuardedByCond usage
> >
> > ScalarEvolution::isKnownPredicate invokes isLoopEntryGuardedByCond
> > without check that SCEV is available at entry point of the loop. It is incorrect
> and fixed by patch.
> >
> > Reviewers: sanjoy, mkazantsev, anna, dorit Reviewed By: mkazantsev
> > Subscribers: llvm-commits
> > Differential Revision: https://reviews.llvm.org/D42165
> >
> > Added:
> > llvm/trunk/test/Transforms/IndVarSimplify/inner-loop.ll
> > Modified:
> > llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> >
> > llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
> >
> > Modified: llvm/trunk/lib/Analysis/ScalarEvolution.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvol
> > ution.cpp?rev=323077&r1=323076&r2=323077&view=diff
> >
> ==========================================================
> ============
> > ========
> > --- llvm/trunk/lib/Analysis/ScalarEvolution.cpp (original)
> > +++ llvm/trunk/lib/Analysis/ScalarEvolution.cpp Sun Jan 21 23:31:41
> > +++ 2018
> > @@ -8669,7 +8669,8 @@ bool ScalarEvolution::isKnownPredicate(I
> > bool RightGuarded = false;
> > if (LAR) {
> > const Loop *L = LAR->getLoop();
> > - if (isLoopEntryGuardedByCond(L, Pred, LAR->getStart(), RHS) &&
> > + if (isAvailableAtLoopEntry(RHS, L) &&
> > + isLoopEntryGuardedByCond(L, Pred, LAR->getStart(), RHS) &&
> > isLoopBackedgeGuardedByCond(L, Pred, LAR->getPostIncExpr(*this),
> RHS)) {
> > if (!RAR) return true;
> > LeftGuarded = true;
> > @@ -8677,7 +8678,8 @@ bool ScalarEvolution::isKnownPredicate(I
> > }
> > if (RAR) {
> > const Loop *L = RAR->getLoop();
> > - if (isLoopEntryGuardedByCond(L, Pred, LHS, RAR->getStart()) &&
> > + if (isAvailableAtLoopEntry(LHS, L) &&
> > + isLoopEntryGuardedByCond(L, Pred, LHS, RAR->getStart()) &&
> > isLoopBackedgeGuardedByCond(L, Pred, LHS, RAR-
> >getPostIncExpr(*this))) {
> > if (!LAR) return true;
> > RightGuarded = true;
> > @@ -9058,6 +9060,12 @@ bool
> > ScalarEvolution::isLoopEntryGuardedByCond(const Loop *L,
> > ICmpInst::Predicate Pred,
> > const SCEV *LHS, const
> > SCEV *RHS) {
> > + // Both LHS and RHS must be available at loop entry.
> > + assert(isAvailableAtLoopEntry(LHS, L) &&
> > + "LHS is not available at Loop Entry");
> > + assert(isAvailableAtLoopEntry(RHS, L) &&
> > + "RHS is not available at Loop Entry");
> > +
> > // Interpret a null as meaning no loop, where there is obviously no guard
> > // (interprocedural conditions notwithstanding).
> > if (!L) return false;
> >
> > Modified:
> > llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/I
> >
> nductiveRangeCheckElimination.cpp?rev=323077&r1=323076&r2=323077&vie
> w=
> > diff
> >
> ==========================================================
> ============
> > ========
> > ---
> > llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cpp
> > (original)
> > +++ llvm/trunk/lib/Transforms/Scalar/InductiveRangeCheckElimination.cp
> > +++ p Sun Jan 21 23:31:41 2018
> > @@ -934,9 +934,9 @@ LoopStructure::parseLoopStructure(Scalar
> > return None;
> > }
> >
> > - if (!SE.isLoopEntryGuardedByCond(
> > - &L, BoundPred, IndVarStart,
> > - SE.getAddExpr(RightSCEV, Step))) {
> > + if (!SE.isAvailableAtLoopEntry(RightSCEV, &L) ||
> > + !SE.isLoopEntryGuardedByCond(&L, BoundPred, IndVarStart,
> > + SE.getAddExpr(RightSCEV,
> > + Step))) {
> > FailureReason = "Induction variable start not bounded by upper limit";
> > return None;
> > }
> > @@ -948,7 +948,8 @@ LoopStructure::parseLoopStructure(Scalar
> > RightValue = B.CreateAdd(RightValue, One);
> > }
> > } else {
> > - if (!SE.isLoopEntryGuardedByCond(&L, BoundPred, IndVarStart,
> RightSCEV)) {
> > + if (!SE.isAvailableAtLoopEntry(RightSCEV, &L) ||
> > + !SE.isLoopEntryGuardedByCond(&L, BoundPred, IndVarStart,
> > + RightSCEV)) {
> > FailureReason = "Induction variable start not bounded by upper limit";
> > return None;
> > }
> > @@ -1014,9 +1015,10 @@ LoopStructure::parseLoopStructure(Scalar
> > return None;
> > }
> >
> > - if (!SE.isLoopEntryGuardedByCond(
> > - &L, BoundPred, IndVarStart,
> > - SE.getMinusSCEV(RightSCEV, SE.getOne(RightSCEV->getType()))))
> {
> > + if (!SE.isAvailableAtLoopEntry(RightSCEV, &L) ||
> > + !SE.isLoopEntryGuardedByCond(
> > + &L, BoundPred, IndVarStart,
> > + SE.getMinusSCEV(RightSCEV,
> > + SE.getOne(RightSCEV->getType())))) {
> > FailureReason = "Induction variable start not bounded by lower limit";
> > return None;
> > }
> > @@ -1028,7 +1030,8 @@ LoopStructure::parseLoopStructure(Scalar
> > RightValue = B.CreateSub(RightValue, One);
> > }
> > } else {
> > - if (!SE.isLoopEntryGuardedByCond(&L, BoundPred, IndVarStart,
> RightSCEV)) {
> > + if (!SE.isAvailableAtLoopEntry(RightSCEV, &L) ||
> > + !SE.isLoopEntryGuardedByCond(&L, BoundPred, IndVarStart,
> > + RightSCEV)) {
> > FailureReason = "Induction variable start not bounded by lower limit";
> > return None;
> > }
> >
> > Added: llvm/trunk/test/Transforms/IndVarSimplify/inner-loop.ll
> > URL:
> > http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarS
> > implify/inner-loop.ll?rev=323077&view=auto
> >
> ==========================================================
> ============
> > ========
> > --- llvm/trunk/test/Transforms/IndVarSimplify/inner-loop.ll (added)
> > +++ llvm/trunk/test/Transforms/IndVarSimplify/inner-loop.ll Sun Jan 21
> > +++ 23:31:41 2018
> > @@ -0,0 +1,54 @@
> > +; RUN: opt < %s -indvars -S | FileCheck %s
> > +
> > +; This is regression test for the bug in ScalarEvolution::isKnownPredicate.
> > +; It does not check whether SCEV is available at loop entry before
> > +invoking ; and utility function isLoopEntryGuardedByCond and that leads
> to miscompile.
> > +
> > +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-ni:1"
> > +target triple = "x86_64-unknown-linux-gnu"
> > +
> > +declare void @foo(i64)
> > +declare void @bar(i32)
> > +
> > +define void @test(i8* %arr) {
> > +entry:
> > + br label %outer_header
> > +
> > +outer_header:
> > + %i = phi i32 [40, %entry], [%i.next, %outer_latch]
> > + %i.64 = sext i32 %i to i64
> > + br label %inner_header
> > +
> > +inner_header:
> > + %j = phi i32 [27, %outer_header], [%j.next, %inner_backedge]
> > + %j1 = zext i32 %j to i64
> > +; The next 4 lines are required for avoid widening of %j and ; SCEV
> > +at %cmp would not be AddRec.
> > + %gep = getelementptr inbounds i8, i8* %arr, i64 %j1
> > + %ld = load i8, i8* %gep
> > + %ec = icmp eq i8 %ld, 0
> > + br i1 %ec, label %return, label %inner_backedge
> > +
> > +inner_backedge:
> > + %cmp = icmp ult i32 %j, %i
> > + %s = select i1 %cmp, i32 %i, i32 %j ; Select should not be
> > +simplified because if ; %i == 26 and %j == 27, %s should be equal to
> > +%j.
> > +; In case of a bug the instruction is simplified to ; %s = select i1
> > +true, i32 %0, i32 %j ; CHECK-NOT: %s = select i1 true
> > + call void @bar(i32 %s)
> > + %j.next = add nsw i32 %j, -2
> > + %cond = icmp ult i32 %j, 3
> > + br i1 %cond, label %outer_latch, label %inner_header
> > +
> > +outer_latch:
> > + %i.next = add i32 %i, -1
> > + %cond2 = icmp sgt i32 %i.next, 13
> > +; This line is just for forcing widening of %i
> > + call void @foo(i64 %i.64)
> > + br i1 %cond2, label %outer_header, label %return
> > +
> > +return:
> > + ret void
> > +}
> >
> >
> > _______________________________________________
> > 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