diff options
author | Roman Lebedev <lebedev.ri@gmail.com> | 2020-12-01 15:00:15 +0300 |
---|---|---|
committer | Roman Lebedev <lebedev.ri@gmail.com> | 2020-12-01 15:13:07 +0300 |
commit | 8e29e20e0d84719e1ad24f28be458db1d9c858db (patch) | |
tree | ccb3bc148b2b1faaa0c78176c6cca7aba3469bd3 | |
parent | [NFC][InstCombine] Add PR48343 miscompiled testcase (diff) | |
download | llvm-project-8e29e20e0d84719e1ad24f28be458db1d9c858db.tar.gz llvm-project-8e29e20e0d84719e1ad24f28be458db1d9c858db.tar.bz2 llvm-project-8e29e20e0d84719e1ad24f28be458db1d9c858db.zip |
[InstCombine] Evaluate new shift amount for sext(ashr(shl(trunc()))) fold in wide type (PR48343)
It is not correct to compute that new shift amount in it's narrow type
and only then extend it into the wide type:
----------------------------------------
Optimization: PR48343 good
Precondition: (width(%X) == width(%r))
%o0 = trunc %X
%o1 = shl %o0, %Y
%o2 = ashr %o1, %Y
%r = sext %o2
=>
%n0 = sext %Y
%n1 = sub width(%o0), %n0
%n2 = sub width(%X), %n1
%n3 = shl %X, %n2
%r = ashr %n3, %n2
Done: 2016
Optimization is correct!
----------------------------------------
Optimization: PR48343 bad
Precondition: (width(%X) == width(%r))
%o0 = trunc %X
%o1 = shl %o0, %Y
%o2 = ashr %o1, %Y
%r = sext %o2
=>
%n0 = sub width(%o0), %Y
%n1 = sub width(%X), %n0
%n2 = sext %n1
%n3 = shl %X, %n2
%r = ashr %n3, %n2
Done: 1
ERROR: Domain of definedness of Target is smaller than Source's for i9 %r
Example:
%X i9 = 0x000 (0)
%Y i4 = 0x3 (3)
%o0 i4 = 0x0 (0)
%o1 i4 = 0x0 (0)
%o2 i4 = 0x0 (0)
%n0 i4 = 0x1 (1)
%n1 i4 = 0x8 (8, -8)
%n2 i9 = 0x1F8 (504, -8)
%n3 i9 = 0x000 (0)
Source value: 0x000 (0)
Target value: undef
I.e. we should be computing it in the wide type from the beginning.
Fixes https://bugs.llvm.org/show_bug.cgi?id=48343
-rw-r--r-- | llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp | 25 | ||||
-rw-r--r-- | llvm/test/Transforms/InstCombine/sext.ll | 8 |
2 files changed, 17 insertions, 16 deletions
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp index 9bfaa3156edf..59dae932ae49 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp @@ -1509,25 +1509,26 @@ Instruction *InstCombinerImpl::visitSExt(SExtInst &CI) { // for a truncate. If the source and dest are the same type, eliminate the // trunc and extend and just do shifts. For example, turn: // %a = trunc i32 %i to i8 - // %b = shl i8 %a, 6 - // %c = ashr i8 %b, 6 + // %b = shl i8 %a, C + // %c = ashr i8 %b, C // %d = sext i8 %c to i32 // into: - // %a = shl i32 %i, 30 - // %d = ashr i32 %a, 30 + // %a = shl i32 %i, 32-(8-C) + // %d = ashr i32 %a, 32-(8-C) Value *A = nullptr; // TODO: Eventually this could be subsumed by EvaluateInDifferentType. Constant *BA = nullptr, *CA = nullptr; if (match(Src, m_AShr(m_Shl(m_Trunc(m_Value(A)), m_Constant(BA)), m_Constant(CA))) && - BA == CA && A->getType() == CI.getType()) { - unsigned MidSize = Src->getType()->getScalarSizeInBits(); - unsigned SrcDstSize = CI.getType()->getScalarSizeInBits(); - Constant *SizeDiff = ConstantInt::get(CA->getType(), SrcDstSize - MidSize); - Constant *ShAmt = ConstantExpr::getAdd(CA, SizeDiff); - Constant *ShAmtExt = ConstantExpr::getSExt(ShAmt, CI.getType()); - A = Builder.CreateShl(A, ShAmtExt, CI.getName()); - return BinaryOperator::CreateAShr(A, ShAmtExt); + BA == CA && A->getType() == DestTy) { + Constant *WideCurrShAmt = ConstantExpr::getSExt(CA, DestTy); + Constant *NumLowbitsLeft = ConstantExpr::getSub( + ConstantInt::get(DestTy, SrcTy->getScalarSizeInBits()), WideCurrShAmt); + Constant *NewShAmt = ConstantExpr::getSub( + ConstantInt::get(DestTy, DestTy->getScalarSizeInBits()), + NumLowbitsLeft); + A = Builder.CreateShl(A, NewShAmt, CI.getName()); + return BinaryOperator::CreateAShr(A, NewShAmt); } return nullptr; diff --git a/llvm/test/Transforms/InstCombine/sext.ll b/llvm/test/Transforms/InstCombine/sext.ll index b2feea73b62c..cf2c44f23810 100644 --- a/llvm/test/Transforms/InstCombine/sext.ll +++ b/llvm/test/Transforms/InstCombine/sext.ll @@ -166,8 +166,8 @@ define <2 x i32> @test10_vec_nonuniform(<2 x i32> %i) { define <2 x i32> @test10_vec_undef(<2 x i32> %i) { ; CHECK-LABEL: @test10_vec_undef( -; CHECK-NEXT: [[D1:%.*]] = shl <2 x i32> [[I:%.*]], <i32 30, i32 0> -; CHECK-NEXT: [[D:%.*]] = ashr <2 x i32> [[D1]], <i32 30, i32 0> +; CHECK-NEXT: [[D1:%.*]] = shl <2 x i32> [[I:%.*]], <i32 30, i32 24> +; CHECK-NEXT: [[D:%.*]] = ashr <2 x i32> [[D1]], <i32 30, i32 24> ; CHECK-NEXT: ret <2 x i32> [[D]] ; %A = trunc <2 x i32> %i to <2 x i8> @@ -281,8 +281,8 @@ define i32 @test18(i16 %x) { define i10 @test19(i10 %i) { ; CHECK-LABEL: @test19( -; CHECK-NEXT: [[D1:%.*]] = shl i10 [[I:%.*]], 1 -; CHECK-NEXT: [[D:%.*]] = ashr exact i10 [[D1]], 1 +; CHECK-NEXT: [[D1:%.*]] = shl i10 [[I:%.*]], 9 +; CHECK-NEXT: [[D:%.*]] = ashr exact i10 [[D1]], 9 ; CHECK-NEXT: ret i10 [[D]] ; %a = trunc i10 %i to i3 |