[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.
This commit is contained in:
Sanjay Patel 2021-09-29 11:38:48 -04:00
parent d3e2067c7c
commit ea56dcb730
2 changed files with 13 additions and 9 deletions

View File

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

View File

@ -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]]
;