aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRoman Lebedev <lebedev.ri@gmail.com>2020-12-01 15:00:15 +0300
committerRoman Lebedev <lebedev.ri@gmail.com>2020-12-01 15:13:07 +0300
commit8e29e20e0d84719e1ad24f28be458db1d9c858db (patch)
treeccb3bc148b2b1faaa0c78176c6cca7aba3469bd3
parent[NFC][InstCombine] Add PR48343 miscompiled testcase (diff)
downloadllvm-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.cpp25
-rw-r--r--llvm/test/Transforms/InstCombine/sext.ll8
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