<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Oct 15, 2014 at 9:13 AM, Timur Iskhodzhanov <span dir="ltr"><<a href="mailto:timurrrr@google.com" target="_blank">timurrrr@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">There was a segfault that was probably fixed by r219639. David?<br></blockquote><div><br></div><div>Not that I am aware of. r219639 fixed an independent bug that somebody filed against InstCombine.</div><div>r219634 caused clang to need to be updated because the APInt API changed, this was r219637.</div><div><br></div><div>Reid and I both believe that our internal build was somehow flaky and that the failure you observed was a red herring or is otherwise unrelated to r219634 .</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><br>
2014-10-15 20:10 GMT+04:00 Bob Wilson <<a href="mailto:bob.wilson@apple.com">bob.wilson@apple.com</a>>:<br>
> Are you talking about a performance regression or a correctness problem?<br>
><br>
> On Oct 14, 2014, at 5:48 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com">timurrrr@google.com</a>> wrote:<br>
><br>
> This has regressed on our internal build, I'll send you the details off the<br>
> list.<br>
><br>
> 2014-10-14 1:48 GMT+04:00 David Majnemer <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>>:<br>
>><br>
>> Author: majnemer<br>
>> Date: Mon Oct 13 16:48:30 2014<br>
>> New Revision: 219634<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=219634&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=219634&view=rev</a><br>
>> Log:<br>
>> InstCombine: Don't miscompile (x lshr C1) udiv C2<br>
>><br>
>> We have a transform that changes:<br>
>> (x lshr C1) udiv C2<br>
>> into:<br>
>> x udiv (C2 << C1)<br>
>><br>
>> However, it is unsafe to do so if C2 << C1 discards any of C2's bits.<br>
>><br>
>> This fixes PR21255.<br>
>><br>
>> Modified:<br>
>> llvm/trunk/include/llvm/ADT/APInt.h<br>
>> llvm/trunk/lib/Support/APInt.cpp<br>
>> llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp<br>
>> llvm/trunk/test/Transforms/InstCombine/div.ll<br>
>><br>
>> Modified: llvm/trunk/include/llvm/ADT/APInt.h<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=219634&r1=219633&r2=219634&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=219634&r1=219633&r2=219634&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/include/llvm/ADT/APInt.h (original)<br>
>> +++ llvm/trunk/include/llvm/ADT/APInt.h Mon Oct 13 16:48:30 2014<br>
>> @@ -945,7 +945,8 @@ public:<br>
>> APInt sdiv_ov(const APInt &RHS, bool &Overflow) const;<br>
>> APInt smul_ov(const APInt &RHS, bool &Overflow) const;<br>
>> APInt umul_ov(const APInt &RHS, bool &Overflow) const;<br>
>> - APInt sshl_ov(unsigned Amt, bool &Overflow) const;<br>
>> + APInt sshl_ov(const APInt &Amt, bool &Overflow) const;<br>
>> + APInt ushl_ov(const APInt &Amt, bool &Overflow) const;<br>
>><br>
>> /// \brief Array-indexing support.<br>
>> ///<br>
>><br>
>> Modified: llvm/trunk/lib/Support/APInt.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=219634&r1=219633&r2=219634&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=219634&r1=219633&r2=219634&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Support/APInt.cpp (original)<br>
>> +++ llvm/trunk/lib/Support/APInt.cpp Mon Oct 13 16:48:30 2014<br>
>> @@ -2064,19 +2064,29 @@ APInt APInt::umul_ov(const APInt &RHS, b<br>
>> return Res;<br>
>> }<br>
>><br>
>> -APInt APInt::sshl_ov(unsigned ShAmt, bool &Overflow) const {<br>
>> - Overflow = ShAmt >= getBitWidth();<br>
>> +APInt APInt::sshl_ov(const APInt &ShAmt, bool &Overflow) const {<br>
>> + Overflow = ShAmt.uge(getBitWidth());<br>
>> if (Overflow)<br>
>> - ShAmt = getBitWidth()-1;<br>
>> + return APInt(BitWidth, 0);<br>
>><br>
>> if (isNonNegative()) // Don't allow sign change.<br>
>> - Overflow = ShAmt >= countLeadingZeros();<br>
>> + Overflow = ShAmt.uge(countLeadingZeros());<br>
>> else<br>
>> - Overflow = ShAmt >= countLeadingOnes();<br>
>> + Overflow = ShAmt.uge(countLeadingOnes());<br>
>><br>
>> return *this << ShAmt;<br>
>> }<br>
>><br>
>> +APInt APInt::ushl_ov(const APInt &ShAmt, bool &Overflow) const {<br>
>> + Overflow = ShAmt.uge(getBitWidth());<br>
>> + if (Overflow)<br>
>> + return APInt(BitWidth, 0);<br>
>> +<br>
>> + Overflow = ShAmt.ugt(countLeadingZeros());<br>
>> +<br>
>> + return *this << ShAmt;<br>
>> +}<br>
>> +<br>
>><br>
>><br>
>><br>
>><br>
>> Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=219634&r1=219633&r2=219634&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=219634&r1=219633&r2=219634&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp<br>
>> (original)<br>
>> +++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Mon Oct<br>
>> 13 16:48:30 2014<br>
>> @@ -965,11 +965,17 @@ Instruction *InstCombiner::visitUDiv(Bin<br>
>> return Common;<br>
>><br>
>> // (x lshr C1) udiv C2 --> x udiv (C2 << C1)<br>
>> - if (Constant *C2 = dyn_cast<Constant>(Op1)) {<br>
>> + {<br>
>> Value *X;<br>
>> - Constant *C1;<br>
>> - if (match(Op0, m_LShr(m_Value(X), m_Constant(C1))))<br>
>> - return BinaryOperator::CreateUDiv(X, ConstantExpr::getShl(C2, C1));<br>
>> + const APInt *C1, *C2;<br>
>> + if (match(Op0, m_LShr(m_Value(X), m_APInt(C1))) &&<br>
>> + match(Op1, m_APInt(C2))) {<br>
>> + bool Overflow;<br>
>> + APInt C2ShlC1 = C2->ushl_ov(*C1, Overflow);<br>
>> + if (!Overflow)<br>
>> + return BinaryOperator::CreateUDiv(<br>
>> + X, ConstantInt::get(X->getType(), C2ShlC1));<br>
>> + }<br>
>> }<br>
>><br>
>> // (zext A) udiv (zext B) --> zext (A udiv B)<br>
>><br>
>> Modified: llvm/trunk/test/Transforms/InstCombine/div.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/div.ll?rev=219634&r1=219633&r2=219634&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/div.ll?rev=219634&r1=219633&r2=219634&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/Transforms/InstCombine/div.ll (original)<br>
>> +++ llvm/trunk/test/Transforms/InstCombine/div.ll Mon Oct 13 16:48:30 2014<br>
>> @@ -132,11 +132,11 @@ define i32 @test15(i32 %a, i32 %b) nounw<br>
>> }<br>
>><br>
>> define <2 x i64> @test16(<2 x i64> %x) nounwind {<br>
>> - %shr = lshr <2 x i64> %x, <i64 3, i64 5><br>
>> - %div = udiv <2 x i64> %shr, <i64 4, i64 6><br>
>> + %shr = lshr <2 x i64> %x, <i64 5, i64 5><br>
>> + %div = udiv <2 x i64> %shr, <i64 6, i64 6><br>
>> ret <2 x i64> %div<br>
>> ; CHECK-LABEL: @test16(<br>
>> -; CHECK-NEXT: udiv <2 x i64> %x, <i64 32, i64 192><br>
>> +; CHECK-NEXT: udiv <2 x i64> %x, <i64 192, i64 192><br>
>> ; CHECK-NEXT: ret <2 x i64><br>
>> }<br>
>><br>
>> @@ -264,3 +264,13 @@ define i32 @test30(i32 %a) {<br>
>> ; CHECK-LABEL: @test30(<br>
>> ; CHECK-NEXT: ret i32 %a<br>
>> }<br>
>> +<br>
>> +define <2 x i32> @test31(<2 x i32> %x) nounwind {<br>
>> + %shr = lshr <2 x i32> %x, <i32 31, i32 31><br>
>> + %div = udiv <2 x i32> %shr, <i32 2147483647, i32 2147483647><br>
>> + ret <2 x i32> %div<br>
>> +; CHECK-LABEL: @test31(<br>
>> +; CHECK-NEXT: %[[shr:.*]] = lshr <2 x i32> %x, <i32 31, i32 31><br>
>> +; CHECK-NEXT: udiv <2 x i32> %[[shr]], <i32 2147483647, i32 2147483647><br>
>> +; CHECK-NEXT: ret <2 x i32><br>
>> +}<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
</div></div></blockquote></div><br></div></div>