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