From: Sanjay Patel Date: Thu, 12 Oct 2017 17:31:46 +0000 (+0000) Subject: [ValueTracking] return zero when there's conflict in known bits of a shift (PR34838) X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=dc813ccd3fcfc0c664b3e355b96c56aae1e29317;p=llvm [ValueTracking] return zero when there's conflict in known bits of a shift (PR34838) Poison allows us to return a better result than undef. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@315595 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index d9e164500c3..afdef2822ac 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -810,19 +810,20 @@ static void computeKnownBitsFromShiftOperator( computeKnownBits(I->getOperand(0), Known, Depth + 1, Q); Known.Zero = KZF(Known.Zero, ShiftAmt); Known.One = KOF(Known.One, ShiftAmt); - // If there is conflict between Known.Zero and Known.One, this must be an - // overflowing left shift, so the shift result is undefined. Clear Known - // bits so that other code could propagate this undef. - if ((Known.Zero & Known.One) != 0) - Known.resetAll(); + // If the known bits conflict, this must be an overflowing left shift, so + // the shift result is poison. We can return anything we want. Choose 0 for + // the best folding opportunity. + if (Known.hasConflict()) + Known.setAllZero(); return; } computeKnownBits(I->getOperand(1), Known, Depth + 1, Q); - // If the shift amount could be greater than or equal to the bit-width of the LHS, the - // value could be undef, so we don't know anything about it. + // If the shift amount could be greater than or equal to the bit-width of the + // LHS, the value could be poison, but bail out because the check below is + // expensive. TODO: Should we just carry on? if ((~Known.Zero).uge(BitWidth)) { Known.resetAll(); return; @@ -878,13 +879,10 @@ static void computeKnownBitsFromShiftOperator( Known.One &= KOF(Known2.One, ShiftAmt); } - // If there are no compatible shift amounts, then we've proven that the shift - // amount must be >= the BitWidth, and the result is undefined. We could - // return anything we'd like, but we need to make sure the sets of known bits - // stay disjoint (it should be better for some other code to actually - // propagate the undef than to pick a value here using known bits). - if (Known.Zero.intersects(Known.One)) - Known.resetAll(); + // If the known bits conflict, the result is poison. Return a 0 and hope the + // caller can further optimize that. + if (Known.hasConflict()) + Known.setAllZero(); } static void computeKnownBitsFromOperator(const Operator *I, KnownBits &Known, diff --git a/test/Analysis/ValueTracking/known-signbit-shift.ll b/test/Analysis/ValueTracking/known-signbit-shift.ll index bf984cb7474..7e9f1c2e70c 100644 --- a/test/Analysis/ValueTracking/known-signbit-shift.ll +++ b/test/Analysis/ValueTracking/known-signbit-shift.ll @@ -27,28 +27,22 @@ define i1 @test_shift_negative(i32 %a, i32 %b) { } ; If sign bit is a known zero, it cannot be a known one. -; This test should not crash opt. +; This test should not crash opt. The shift produces poison. define i32 @test_no_sign_bit_conflict1(i1 %b) { ; CHECK-LABEL: @test_no_sign_bit_conflict1( -; CHECK-NEXT: entry: -; CHECK-NEXT: [[SEL:%.*]] = select i1 %b, i32 -2147221504, i32 -2147483648 -; CHECK-NEXT: ret i32 [[SEL]] +; CHECK-NEXT: ret i32 0 ; -entry: %sel = select i1 %b, i32 8193, i32 8192 %mul = shl nsw i32 %sel, 18 ret i32 %mul } ; If sign bit is a known one, it cannot be a known zero. -; This test should not crash opt. +; This test should not crash opt. The shift produces poison. define i32 @test_no_sign_bit_conflict2(i1 %b) { ; CHECK-LABEL: @test_no_sign_bit_conflict2( -; CHECK-NEXT: entry: -; CHECK-NEXT: [[SEL:%.*]] = select i1 %b, i32 2147221504, i32 2146959360 -; CHECK-NEXT: ret i32 [[SEL]] +; CHECK-NEXT: ret i32 0 ; -entry: %sel = select i1 %b, i32 -8193, i32 -8194 %mul = shl nsw i32 %sel, 18 ret i32 %mul diff --git a/test/Transforms/InstSimplify/icmp-constant.ll b/test/Transforms/InstSimplify/icmp-constant.ll index 453d4befb92..2e58799f970 100644 --- a/test/Transforms/InstSimplify/icmp-constant.ll +++ b/test/Transforms/InstSimplify/icmp-constant.ll @@ -576,12 +576,7 @@ define <2 x i1> @add_nsw_pos_const5_splat_vec(<2 x i32> %x) { define i1 @ne_shl_by_constant_produces_poison(i8 %x) { ; CHECK-LABEL: @ne_shl_by_constant_produces_poison( -; CHECK-NEXT: [[ZX:%.*]] = zext i8 %x to i16 -; CHECK-NEXT: [[XOR:%.*]] = xor i16 [[ZX]], 32767 -; CHECK-NEXT: [[SUB:%.*]] = sub nsw i16 [[ZX]], [[XOR]] -; CHECK-NEXT: [[POISON:%.*]] = shl nsw i16 [[SUB]], 2 -; CHECK-NEXT: [[CMP:%.*]] = icmp ne i16 [[POISON]], 1 -; CHECK-NEXT: ret i1 [[CMP]] +; CHECK-NEXT: ret i1 true ; %zx = zext i8 %x to i16 ; zx = 0x00xx %xor = xor i16 %zx, 32767 ; xor = 0x7fyy @@ -593,11 +588,7 @@ define i1 @ne_shl_by_constant_produces_poison(i8 %x) { define i1 @eq_shl_by_constant_produces_poison(i8 %x) { ; CHECK-LABEL: @eq_shl_by_constant_produces_poison( -; CHECK-NEXT: [[CLEAR_HIGH_BIT:%.*]] = and i8 %x, 127 -; CHECK-NEXT: [[SET_NEXT_HIGH_BITS:%.*]] = or i8 [[CLEAR_HIGH_BIT]], 112 -; CHECK-NEXT: [[POISON:%.*]] = shl nsw i8 [[SET_NEXT_HIGH_BITS]], 3 -; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[POISON]], 15 -; CHECK-NEXT: ret i1 [[CMP]] +; CHECK-NEXT: ret i1 false ; %clear_high_bit = and i8 %x, 127 ; 0x7f %set_next_high_bits = or i8 %clear_high_bit, 112 ; 0x70 @@ -612,13 +603,7 @@ define i1 @eq_shl_by_constant_produces_poison(i8 %x) { define i1 @eq_shl_by_variable_produces_poison(i8 %x) { ; CHECK-LABEL: @eq_shl_by_variable_produces_poison( -; CHECK-NEXT: [[CLEAR_HIGH_BIT:%.*]] = and i8 %x, 127 -; CHECK-NEXT: [[SET_NEXT_HIGH_BITS:%.*]] = or i8 [[CLEAR_HIGH_BIT]], 112 -; CHECK-NEXT: [[NOTUNDEF_SHIFTAMT:%.*]] = and i8 %x, 3 -; CHECK-NEXT: [[NONZERO_SHIFTAMT:%.*]] = or i8 [[NOTUNDEF_SHIFTAMT]], 1 -; CHECK-NEXT: [[POISON:%.*]] = shl nsw i8 [[SET_NEXT_HIGH_BITS]], [[NONZERO_SHIFTAMT]] -; CHECK-NEXT: [[CMP:%.*]] = icmp eq i8 [[POISON]], 15 -; CHECK-NEXT: ret i1 [[CMP]] +; CHECK-NEXT: ret i1 false ; %clear_high_bit = and i8 %x, 127 ; 0x7f %set_next_high_bits = or i8 %clear_high_bit, 112 ; 0x70