[llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 28 22:55:13 PDT 2016
Early on in the pipeline my change caused a zext to feed the getelementptrs
instead of a sext. Later in Induction Variable Simplification a trunc was
added in front of the zext. Where as before the sext was completely removed.
~Craig
On Tue, Jun 28, 2016 at 10:45 PM, Sanjoy Das <sanjoy at playingwithpointers.com
> wrote:
>
>
> Craig Topper wrote:
>
>> My change seems to have caused an "and" with 4294967295 to be inserted
>> into some of the getelementptrs. A similar thing
>> happens if you change the type of "i" in the loop to "unsigned". Though
>> "unsigned" causes ands on even more of the
>> getelementptrs.
>>
>
> Given your change only adds information, I suspect this isn't anything
> complicated -- most likely making KnownBits smarter makes something else in
> the pipeline make a transform SCEV does not yet understand. If that is the
> case, then the solution is to make SCEV understand this new pattern, or, in
> some cases, not doing the transform.
>
> I'll revert your revert (locally) and take a look to see if I spot
> something obvious.
>
> -- Sanjoy
>
>
>
>> ~Craig
>>
>> On Tue, Jun 28, 2016 at 10:14 PM, Adam Nemet <anemet at apple.com <mailto:
>> anemet at apple.com>> wrote:
>>
>> As I said in the review, my worry is that we could be regressing some
>> cases that we used to vectorize. I’d like to
>> see some analysis why this happened, i.e. why we were able to get the
>> array bounds before. Is it because we’re now
>> widening some IV and the truncs prevent us forming nice SCEV
>> add-recurrences? (We can only get bounds for
>> add-recurrences.)
>>
>> Adam
>>
>>
>> On Jun 28, 2016, at 10:02 PM, Craig Topper <craig.topper at gmail.com
>>> <mailto:craig.topper at gmail.com>> wrote:
>>>
>>> I see that. Looks like changing N to a constant or starting the loop
>>> at -1 fixes my case. Changing indices to all
>>> +1 like D21773 does not fix it. What do you suggest?
>>>
>>> ~Craig
>>>
>>> On Tue, Jun 28, 2016 at 9:56 PM, Adam Nemet <anemet at apple.com
>>> <mailto:anemet at apple.com>> wrote:
>>>
>>> Looks like this broke the same test the same way that we’ve been
>>> discussing under http://reviews.llvm.org/D21773
>>>
>>> Adam
>>>
>>> > On Jun 28, 2016, at 8:46 PM, Craig Topper via llvm-commits <
>>> llvm-commits at lists.llvm.org
>>> <mailto:llvm-commits at lists.llvm.org>> wrote:
>>> >
>>> > Author: ctopper
>>> > Date: Tue Jun 28 22:46:47 2016
>>> > New Revision: 274098
>>> >
>>> > URL: http://llvm.org/viewvc/llvm-project?rev=274098&view=rev
>>> <http://llvm.org/viewvc/llvm-project?rev=274098&view=rev>
>>> > Log:
>>> > [ValueTracking] Teach computeKnownBits for PHI nodes to
>>> compute sign bit for a recurrence with a NSW addition.
>>> >
>>> > If a operation for a recurrence is an addition with no signed
>>> wrap and both input sign bits are 0, then the
>>> result sign bit must also be 0. Similar for the negative case.
>>> >
>>> > I found this deficiency while playing around with a loop in
>>> the x86 backend that contained a signed division
>>> that could be optimized into an unsigned division if we could
>>> prove both inputs were positive. One of them
>>> being the loop induction variable. With this patch we can
>>> perform the conversion for this case. One of the
>>> test cases here is a contrived variation of the loop I was
>>> looking at.
>>> >
>>> > Differential revision: http://reviews.llvm.org/D21493
>>> >
>>> > Modified:
>>> > llvm/trunk/lib/Analysis/ValueTracking.cpp
>>> > llvm/trunk/test/Transforms/BBVectorize/loop1.ll
>>> > llvm/trunk/test/Transforms/InstCombine/phi.ll
>>> >
>>> > Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
>>> > URL:
>>>
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=274098&r1=274097&r2=274098&view=diff
>>> <
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=274098&r1=274097&r2=274098&view=diff
>>> >
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
>>> > +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Tue Jun 28
>>> 22:46:47 2016
>>> > @@ -1240,6 +1240,18 @@ static void computeKnownBitsFromOperator
>>> > KnownZero = APInt::getLowBitsSet(BitWidth,
>>> >
>>> std::min(KnownZero2.countTrailingOnes(),
>>> >
>>> KnownZero3.countTrailingOnes()));
>>> > +
>>> > + // If the operation is an addition that can't have
>>> signed overflow,
>>> > + // then the sign bit is known to be zero if both
>>> input sign bits
>>> > + // are zero. Similar for two negative inputs.
>>> > + if (Opcode == Instruction::Add &&
>>> > +
>>> cast<OverflowingBinaryOperator>(LU)->hasNoSignedWrap()) {
>>> > + if (KnownZero2.isNegative() &&
>>> KnownZero3.isNegative())
>>> > + KnownZero.setBit(BitWidth-1);
>>> > + if (KnownOne2.isNegative() &&
>>> KnownOne3.isNegative())
>>> > + KnownOne.setBit(BitWidth-1);
>>> > + }
>>> > +
>>> > break;
>>> > }
>>> > }
>>> >
>>> > Modified: llvm/trunk/test/Transforms/BBVectorize/loop1.ll
>>> > URL:
>>>
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BBVectorize/loop1.ll?rev=274098&r1=274097&r2=274098&view=diff
>>> <
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BBVectorize/loop1.ll?rev=274098&r1=274097&r2=274098&view=diff
>>> >
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/test/Transforms/BBVectorize/loop1.ll (original)
>>> > +++ llvm/trunk/test/Transforms/BBVectorize/loop1.ll Tue Jun 28
>>> 22:46:47 2016
>>> > @@ -83,7 +83,7 @@ for.body:
>>> > ; CHECK-UNRL: %add12 = fadd <2 x double> %add7, %mul11
>>> > ; CHECK-UNRL: %4 = bitcast double* %arrayidx14 to <2 x double>*
>>> > ; CHECK-UNRL: store <2 x double> %add12, <2 x double>* %4,
>>> align 8
>>> > -; CHECK-UNRL: %indvars.iv.next.1 = add nsw i64 %indvars.iv, 2
>>> > +; CHECK-UNRL: %indvars.iv.next.1 = add nuw nsw i64
>>> %indvars.iv, 2
>>> > ; CHECK-UNRL: %lftr.wideiv.1 = trunc i64 %indvars.iv.next.1 to
>>> i32
>>> > ; CHECK-UNRL: %exitcond.1 = icmp eq i32 %lftr.wideiv.1, 10
>>> > ; CHECK-UNRL: br i1 %exitcond.1, label %for.end, label
>>> %for.body
>>> >
>>> > Modified: llvm/trunk/test/Transforms/InstCombine/phi.ll
>>> > URL:
>>>
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/phi.ll?rev=274098&r1=274097&r2=274098&view=diff
>>> <
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/phi.ll?rev=274098&r1=274097&r2=274098&view=diff
>>> >
>>> >
>>> ==============================================================================
>>> > --- llvm/trunk/test/Transforms/InstCombine/phi.ll (original)
>>> > +++ llvm/trunk/test/Transforms/InstCombine/phi.ll Tue Jun 28
>>> 22:46:47 2016
>>> > @@ -879,3 +879,118 @@ if.end:
>>> > %cmp1 = icmp ne i32 %a.0, 0
>>> > ret i1 %cmp1
>>> > }
>>> > +
>>> > +
>>> > +; This test makes sure we can determine that the inputs to
>>> the sdiv in the loop
>>> > +; are non-negative and can become a udiv. This requires that
>>> we recognize that
>>> > +; the loop induction can never have its sign bit set.
>>> > +;
>>> > +; CHECK-LABEL: @phi_nsw_induction_sdiv_udiv
>>> > +; CHECK: udiv
>>> > +; CHECK: udiv
>>> > +define i32 @phi_nsw_induction_sdiv_udiv(i32 %NumElts, i32
>>> %ScalarSize) {
>>> > +entry:
>>> > + %div = udiv i32 128, %ScalarSize
>>> > + br label %for.cond
>>> > +
>>> > +for.cond: ; preds =
>>> %for.inc, %entry
>>> > + %Sum.0 = phi i32 [ 0, %entry ], [ %add, %for.inc ]
>>> > + %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.inc ]
>>> > + %cmp = icmp ne i32 %i.0, %NumElts
>>> > + br i1 %cmp, label %for.body, label %for.end
>>> > +
>>> > +for.body: ; preds =
>>> %for.cond
>>> > + ; this should become a udiv
>>> > + %div1 = sdiv i32 %i.0, %div
>>> > + %add = add nsw i32 %Sum.0, %div1
>>> > + br label %for.inc
>>> > +
>>> > +for.inc: ; preds =
>>> %for.body
>>> > + %inc = add nsw i32 %i.0, 1
>>> > + br label %for.cond
>>> > +
>>> > +for.end: ; preds =
>>> %for.cond
>>> > + ret i32 %Sum.0
>>> > +}
>>> > +
>>> > +
>>> > +; CHECK-LABEL: test_positive_nsw_recurrence
>>> > +; CHECK-NOT: bar
>>> > +; CHECK: foo
>>> > +; CHECK-NOT: bar
>>> > +; CHECK: ret
>>> > +define void @test_positive_nsw_recurrence(i32 %N) {
>>> > +entry:
>>> > + br label %for.cond
>>> > +
>>> > +for.cond: ; preds =
>>> %for.inc, %entry
>>> > + %i.0 = phi i32 [ 0, %entry ], [ %inc, %for.inc ]
>>> > + %cmp = icmp ne i32 %i.0, %N
>>> > + br i1 %cmp, label %for.body, label %for.end
>>> > +
>>> > +for.body: ; preds =
>>> %for.cond
>>> > + %and = and i32 %i.0, -2147483648 <tel:2147483648>
>>>
>>> > + %tobool = icmp ne i32 %and, 0
>>> > + br i1 %tobool, label %if.then, label %if.else
>>> > +
>>> > +if.then: ; preds =
>>> %for.body
>>> > + ; this call should be deleted as %i.0 can never be negative
>>> due to no signed wrap
>>> > + call void @bar()
>>> > + br label %if.end
>>> > +
>>> > +if.else: ; preds =
>>> %for.body
>>> > + call void @foo()
>>> > + br label %if.end
>>> > +
>>> > +if.end: ; preds =
>>> %if.else, %if.then
>>> > + br label %for.inc
>>> > +
>>> > +for.inc: ; preds =
>>> %if.end
>>> > + %inc = add nsw i32 %i.0, 1
>>> > + br label %for.cond
>>> > +
>>> > +for.end: ; preds =
>>> %for.cond
>>> > + ret void
>>> > +}
>>> > +
>>> > +; CHECK-LABEL: test_negative_nsw_recurrence
>>> > +; CHECK-NOT: foo
>>> > +; CHECK: bar
>>> > +; CHECK-NOT: foo
>>> > +; CHECK: ret
>>> > +define void @test_negative_nsw_recurrence(i32 %N) {
>>> > +entry:
>>> > + br label %for.cond
>>> > +
>>> > +for.cond: ; preds =
>>> %for.inc, %entry
>>> > + %i.0 = phi i32 [ -1, %entry ], [ %inc, %for.inc ]
>>> > + %cmp = icmp ne i32 %i.0, %N
>>> > + br i1 %cmp, label %for.body, label %for.end
>>> > +
>>> > +for.body: ; preds =
>>> %for.cond
>>> > + %and = and i32 %i.0, -2147483648 <tel:2147483648>
>>>
>>> > + %tobool = icmp ne i32 %and, 0
>>> > + br i1 %tobool, label %if.then, label %if.else
>>> > +
>>> > +if.then: ; preds =
>>> %for.body
>>> > + call void @bar()
>>> > + br label %if.end
>>> > +
>>> > +if.else: ; preds =
>>> %for.body
>>> > + ; this call should be deleted as %i.0 can never be positive
>>> due to no signed wrap
>>> > + call void @foo()
>>> > + br label %if.end
>>> > +
>>> > +if.end: ; preds =
>>> %if.else, %if.then
>>> > + br label %for.inc
>>> > +
>>> > +for.inc: ; preds =
>>> %if.end
>>> > + %inc = add nsw i32 %i.0, -1
>>> > + br label %for.cond
>>> > +
>>> > +for.end: ; preds =
>>> %for.cond
>>> > + ret void
>>> > +}
>>> > +
>>> > +declare void @bar()
>>> > +declare void @foo()
>>> >
>>> >
>>> > _______________________________________________
>>> > llvm-commits mailing list
>>> > llvm-commits at lists.llvm.org <mailto:
>>> llvm-commits at lists.llvm.org>
>>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>>
>>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160628/6f838ff9/attachment-0001.html>
More information about the llvm-commits
mailing list