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