<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Are you talking about a performance regression or a correctness problem?<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Oct 14, 2014, at 5:48 AM, Timur Iskhodzhanov <<a href="mailto:timurrrr@google.com" class="">timurrrr@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">This has regressed on our internal build, I'll send you the details off the list.</div><div class="gmail_extra"><br class=""><div class="gmail_quote">2014-10-14 1:48 GMT+04:00 David Majnemer <span dir="ltr" class=""><<a href="mailto:david.majnemer@gmail.com" target="_blank" class="">david.majnemer@gmail.com</a>></span>:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: majnemer<br class="">
Date: Mon Oct 13 16:48:30 2014<br class="">
New Revision: 219634<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=219634&view=rev" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=219634&view=rev</a><br class="">
Log:<br class="">
InstCombine: Don't miscompile (x lshr C1) udiv C2<br class="">
<br class="">
We have a transform that changes:<br class="">
(x lshr C1) udiv C2<br class="">
into:<br class="">
x udiv (C2 << C1)<br class="">
<br class="">
However, it is unsafe to do so if C2 << C1 discards any of C2's bits.<br class="">
<br class="">
This fixes PR21255.<br class="">
<br class="">
Modified:<br class="">
llvm/trunk/include/llvm/ADT/APInt.h<br class="">
llvm/trunk/lib/Support/APInt.cpp<br class="">
llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp<br class="">
llvm/trunk/test/Transforms/InstCombine/div.ll<br class="">
<br class="">
Modified: llvm/trunk/include/llvm/ADT/APInt.h<br class="">
URL: <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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=219634&r1=219633&r2=219634&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/include/llvm/ADT/APInt.h (original)<br class="">
+++ llvm/trunk/include/llvm/ADT/APInt.h Mon Oct 13 16:48:30 2014<br class="">
@@ -945,7 +945,8 @@ public:<br class="">
APInt sdiv_ov(const APInt &RHS, bool &Overflow) const;<br class="">
APInt smul_ov(const APInt &RHS, bool &Overflow) const;<br class="">
APInt umul_ov(const APInt &RHS, bool &Overflow) const;<br class="">
- APInt sshl_ov(unsigned Amt, bool &Overflow) const;<br class="">
+ APInt sshl_ov(const APInt &Amt, bool &Overflow) const;<br class="">
+ APInt ushl_ov(const APInt &Amt, bool &Overflow) const;<br class="">
<br class="">
/// \brief Array-indexing support.<br class="">
///<br class="">
<br class="">
Modified: llvm/trunk/lib/Support/APInt.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=219634&r1=219633&r2=219634&view=diff" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/APInt.cpp?rev=219634&r1=219633&r2=219634&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Support/APInt.cpp (original)<br class="">
+++ llvm/trunk/lib/Support/APInt.cpp Mon Oct 13 16:48:30 2014<br class="">
@@ -2064,19 +2064,29 @@ APInt APInt::umul_ov(const APInt &RHS, b<br class="">
return Res;<br class="">
}<br class="">
<br class="">
-APInt APInt::sshl_ov(unsigned ShAmt, bool &Overflow) const {<br class="">
- Overflow = ShAmt >= getBitWidth();<br class="">
+APInt APInt::sshl_ov(const APInt &ShAmt, bool &Overflow) const {<br class="">
+ Overflow = ShAmt.uge(getBitWidth());<br class="">
if (Overflow)<br class="">
- ShAmt = getBitWidth()-1;<br class="">
+ return APInt(BitWidth, 0);<br class="">
<br class="">
if (isNonNegative()) // Don't allow sign change.<br class="">
- Overflow = ShAmt >= countLeadingZeros();<br class="">
+ Overflow = ShAmt.uge(countLeadingZeros());<br class="">
else<br class="">
- Overflow = ShAmt >= countLeadingOnes();<br class="">
+ Overflow = ShAmt.uge(countLeadingOnes());<br class="">
<br class="">
return *this << ShAmt;<br class="">
}<br class="">
<br class="">
+APInt APInt::ushl_ov(const APInt &ShAmt, bool &Overflow) const {<br class="">
+ Overflow = ShAmt.uge(getBitWidth());<br class="">
+ if (Overflow)<br class="">
+ return APInt(BitWidth, 0);<br class="">
+<br class="">
+ Overflow = ShAmt.ugt(countLeadingZeros());<br class="">
+<br class="">
+ return *this << ShAmt;<br class="">
+}<br class="">
+<br class="">
<br class="">
<br class="">
<br class="">
<br class="">
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp<br class="">
URL: <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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp?rev=219634&r1=219633&r2=219634&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (original)<br class="">
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp Mon Oct 13 16:48:30 2014<br class="">
@@ -965,11 +965,17 @@ Instruction *InstCombiner::visitUDiv(Bin<br class="">
return Common;<br class="">
<br class="">
// (x lshr C1) udiv C2 --> x udiv (C2 << C1)<br class="">
- if (Constant *C2 = dyn_cast<Constant>(Op1)) {<br class="">
+ {<br class="">
Value *X;<br class="">
- Constant *C1;<br class="">
- if (match(Op0, m_LShr(m_Value(X), m_Constant(C1))))<br class="">
- return BinaryOperator::CreateUDiv(X, ConstantExpr::getShl(C2, C1));<br class="">
+ const APInt *C1, *C2;<br class="">
+ if (match(Op0, m_LShr(m_Value(X), m_APInt(C1))) &&<br class="">
+ match(Op1, m_APInt(C2))) {<br class="">
+ bool Overflow;<br class="">
+ APInt C2ShlC1 = C2->ushl_ov(*C1, Overflow);<br class="">
+ if (!Overflow)<br class="">
+ return BinaryOperator::CreateUDiv(<br class="">
+ X, ConstantInt::get(X->getType(), C2ShlC1));<br class="">
+ }<br class="">
}<br class="">
<br class="">
// (zext A) udiv (zext B) --> zext (A udiv B)<br class="">
<br class="">
Modified: llvm/trunk/test/Transforms/InstCombine/div.ll<br class="">
URL: <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" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/div.ll?rev=219634&r1=219633&r2=219634&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/Transforms/InstCombine/div.ll (original)<br class="">
+++ llvm/trunk/test/Transforms/InstCombine/div.ll Mon Oct 13 16:48:30 2014<br class="">
@@ -132,11 +132,11 @@ define i32 @test15(i32 %a, i32 %b) nounw<br class="">
}<br class="">
<br class="">
define <2 x i64> @test16(<2 x i64> %x) nounwind {<br class="">
- %shr = lshr <2 x i64> %x, <i64 3, i64 5><br class="">
- %div = udiv <2 x i64> %shr, <i64 4, i64 6><br class="">
+ %shr = lshr <2 x i64> %x, <i64 5, i64 5><br class="">
+ %div = udiv <2 x i64> %shr, <i64 6, i64 6><br class="">
ret <2 x i64> %div<br class="">
; CHECK-LABEL: @test16(<br class="">
-; CHECK-NEXT: udiv <2 x i64> %x, <i64 32, i64 192><br class="">
+; CHECK-NEXT: udiv <2 x i64> %x, <i64 192, i64 192><br class="">
; CHECK-NEXT: ret <2 x i64><br class="">
}<br class="">
<br class="">
@@ -264,3 +264,13 @@ define i32 @test30(i32 %a) {<br class="">
; CHECK-LABEL: @test30(<br class="">
; CHECK-NEXT: ret i32 %a<br class="">
}<br class="">
+<br class="">
+define <2 x i32> @test31(<2 x i32> %x) nounwind {<br class="">
+ %shr = lshr <2 x i32> %x, <i32 31, i32 31><br class="">
+ %div = udiv <2 x i32> %shr, <i32 2147483647, i32 2147483647><br class="">
+ ret <2 x i32> %div<br class="">
+; CHECK-LABEL: @test31(<br class="">
+; CHECK-NEXT: %[[shr:.*]] = lshr <2 x i32> %x, <i32 31, i32 31><br class="">
+; CHECK-NEXT: udiv <2 x i32> %[[shr]], <i32 2147483647, i32 2147483647><br class="">
+; CHECK-NEXT: ret <2 x i32><br class="">
+}<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div><br class=""></div>
_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></div></body></html>