[llvm] r274098 - [ValueTracking] Teach computeKnownBits for PHI nodes to compute sign bit for a recurrence with a NSW addition.
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 28 22:45:36 PDT 2016
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
>>
>>
>
>
More information about the llvm-commits
mailing list