[llvm] r219634 - InstCombine: Don't miscompile (x lshr C1) udiv C2

Timur Iskhodzhanov timurrrr at google.com
Wed Oct 15 09:13:41 PDT 2014


There was a segfault that was probably fixed by r219639.  David?

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
>
>



More information about the llvm-commits mailing list