[llvm] ea56dcb - [InstCombine] fix miscompile from dropRedundantMaskingOfLeftShiftInput()
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 29 08:45:28 PDT 2021
Author: Sanjay Patel
Date: 2021-09-29T11:43:18-04:00
New Revision: ea56dcb73012f65e6ffb1c320c97c6fbf45480e0
URL: https://github.com/llvm/llvm-project/commit/ea56dcb73012f65e6ffb1c320c97c6fbf45480e0
DIFF: https://github.com/llvm/llvm-project/commit/ea56dcb73012f65e6ffb1c320c97c6fbf45480e0.diff
LOG: [InstCombine] fix miscompile from dropRedundantMaskingOfLeftShiftInput()
The test is from https://llvm.org/PR51351.
There are 2 related logic bugs from over-generalizing "lshr" to "any shr",
but I'm not sure how to expose the difference for "MaskC" because instsimplify
already folds ashr of -1.
I'll extend instsimplify to catch the MaskD pattern as a follow-up, but this
patch should be enough to avoid the miscompile.
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-d.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index 8d4113835dc77..cf746a0323f52 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -172,8 +172,8 @@ Value *InstCombinerImpl::reassociateShiftAmtsOfTwoSameDirectionShifts(
// There are many variants to this pattern:
// a) (x & ((1 << MaskShAmt) - 1)) << ShiftShAmt
// b) (x & (~(-1 << MaskShAmt))) << ShiftShAmt
-// c) (x & (-1 >> MaskShAmt)) << ShiftShAmt
-// d) (x & ((-1 << MaskShAmt) >> MaskShAmt)) << ShiftShAmt
+// c) (x & (-1 l>> MaskShAmt)) << ShiftShAmt
+// d) (x & ((-1 << MaskShAmt) l>> MaskShAmt)) << ShiftShAmt
// e) ((x << MaskShAmt) l>> MaskShAmt) << ShiftShAmt
// f) ((x << MaskShAmt) a>> MaskShAmt) << ShiftShAmt
// All these patterns can be simplified to just:
@@ -213,11 +213,11 @@ dropRedundantMaskingOfLeftShiftInput(BinaryOperator *OuterShift,
auto MaskA = m_Add(m_Shl(m_One(), m_Value(MaskShAmt)), m_AllOnes());
// (~(-1 << maskNbits))
auto MaskB = m_Xor(m_Shl(m_AllOnes(), m_Value(MaskShAmt)), m_AllOnes());
- // (-1 >> MaskShAmt)
- auto MaskC = m_Shr(m_AllOnes(), m_Value(MaskShAmt));
- // ((-1 << MaskShAmt) >> MaskShAmt)
+ // (-1 l>> MaskShAmt)
+ auto MaskC = m_LShr(m_AllOnes(), m_Value(MaskShAmt));
+ // ((-1 << MaskShAmt) l>> MaskShAmt)
auto MaskD =
- m_Shr(m_Shl(m_AllOnes(), m_Value(MaskShAmt)), m_Deferred(MaskShAmt));
+ m_LShr(m_Shl(m_AllOnes(), m_Value(MaskShAmt)), m_Deferred(MaskShAmt));
Value *X;
Constant *NewMask;
diff --git a/llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-d.ll b/llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-d.ll
index c3039dc84fc52..efc2323c9dd29 100644
--- a/llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-d.ll
+++ b/llvm/test/Transforms/InstCombine/partally-redundant-left-shift-input-masking-after-truncation-variant-d.ll
@@ -246,13 +246,17 @@ define i32 @n6_extrause2(i64 %x, i32 %nbits) {
ret i32 %t6
}
-; FIXME:
+; TODO: shl+ashr of -1 should be reducecd.
; This is a miscompile if it ends by masking off the high bit of the result.
define i32 @PR51351(i64 %x, i32 %nbits) {
; CHECK-LABEL: @PR51351(
-; CHECK-NEXT: [[T3:%.*]] = add i32 [[NBITS:%.*]], -33
-; CHECK-NEXT: [[T5:%.*]] = trunc i64 [[X:%.*]] to i32
+; CHECK-NEXT: [[T0:%.*]] = zext i32 [[NBITS:%.*]] to i64
+; CHECK-NEXT: [[T1:%.*]] = shl i64 -1, [[T0]]
+; CHECK-NEXT: [[T2:%.*]] = ashr i64 [[T1]], [[T0]]
+; CHECK-NEXT: [[T3:%.*]] = add i32 [[NBITS]], -33
+; CHECK-NEXT: [[T4:%.*]] = and i64 [[T2]], [[X:%.*]]
+; CHECK-NEXT: [[T5:%.*]] = trunc i64 [[T4]] to i32
; CHECK-NEXT: [[T6:%.*]] = shl i32 [[T5]], [[T3]]
; CHECK-NEXT: ret i32 [[T6]]
;
More information about the llvm-commits
mailing list