[llvm] 3205d1a - [InstCombine] Handle known shl nsw sign bit in SimplifyDemanded

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2020-03-20T18:16:05+01:00
New Revision: 3205d1a860367f7ab77d331f03086eeed194e44c

URL: https://github.com/llvm/llvm-project/commit/3205d1a860367f7ab77d331f03086eeed194e44c
DIFF: https://github.com/llvm/llvm-project/commit/3205d1a860367f7ab77d331f03086eeed194e44c.diff

LOG: [InstCombine] Handle known shl nsw sign bit in SimplifyDemanded

Ideally SimplifyDemanded should compute the same known bits as
computeKnownBits(). This patch addresses one discrepancy, where
ValueTracking is more powerful: If we have a shl nsw shift, we
know that the sign bit of the input and output must be the same.
If this results in a conflict, the result is poison.

This is implemented in
https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/lib/Analysis/ValueTracking.cpp#L1175-L1179
and
https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/lib/Analysis/ValueTracking.cpp#L904-L908.

This implements the same basic logic in SimplifyDemanded. It's
slightly stronger, because I return undef instead of zero for the
poison case (which is not an option inside ValueTracking).

As mentioned in https://reviews.llvm.org/D75801#inline-698484,
we could detect poison in more cases, this just establishes parity
with the existing logic.

Differential Revision: https://reviews.llvm.org/D76489

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
    llvm/test/Transforms/InstCombine/known-signbit-shift.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
index aa5d33ce8ac8..e697b99d6cf8 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
@@ -553,11 +553,25 @@ Value *InstCombiner::SimplifyDemandedUseBits(Value *V, APInt DemandedMask,
       if (SimplifyDemandedBits(I, 0, DemandedMaskIn, Known, Depth + 1))
         return I;
       assert(!Known.hasConflict() && "Bits known to be one AND zero?");
+
+      bool SignBitZero = Known.Zero.isSignBitSet();
+      bool SignBitOne = Known.One.isSignBitSet();
       Known.Zero <<= ShiftAmt;
       Known.One  <<= ShiftAmt;
       // low bits known zero.
       if (ShiftAmt)
         Known.Zero.setLowBits(ShiftAmt);
+
+      // If this shift has "nsw" keyword, then the result is either a poison
+      // value or has the same sign bit as the first operand.
+      if (IOp->hasNoSignedWrap()) {
+        if (SignBitZero)
+          Known.Zero.setSignBit();
+        else if (SignBitOne)
+          Known.One.setSignBit();
+        if (Known.hasConflict())
+          return UndefValue::get(I->getType());
+      }
     } else {
       computeKnownBits(I, Known, Depth, CxtI);
     }

diff  --git a/llvm/test/Transforms/InstCombine/known-signbit-shift.ll b/llvm/test/Transforms/InstCombine/known-signbit-shift.ll
index d619e5358114..0a4e7660ef1f 100644
--- a/llvm/test/Transforms/InstCombine/known-signbit-shift.ll
+++ b/llvm/test/Transforms/InstCombine/known-signbit-shift.ll
@@ -31,8 +31,7 @@ define i1 @test_shift_negative(i32 %a, i32 %b) {
 ; This test should not crash opt. The shift produces poison.
 define i32 @test_no_sign_bit_conflict1(i1 %b) {
 ; EXPENSIVE-OFF-LABEL: @test_no_sign_bit_conflict1(
-; EXPENSIVE-OFF-NEXT:    [[SEL:%.*]] = select i1 [[B:%.*]], i32 -2147221504, i32 -2147483648
-; EXPENSIVE-OFF-NEXT:    ret i32 [[SEL]]
+; EXPENSIVE-OFF-NEXT:    ret i32 undef
 ;
 ; EXPENSIVE-ON-LABEL: @test_no_sign_bit_conflict1(
 ; EXPENSIVE-ON-NEXT:    ret i32 0
@@ -46,8 +45,7 @@ define i32 @test_no_sign_bit_conflict1(i1 %b) {
 ; This test should not crash opt. The shift produces poison.
 define i32 @test_no_sign_bit_conflict2(i1 %b) {
 ; EXPENSIVE-OFF-LABEL: @test_no_sign_bit_conflict2(
-; EXPENSIVE-OFF-NEXT:    [[SEL:%.*]] = select i1 [[B:%.*]], i32 2147221504, i32 2146959360
-; EXPENSIVE-OFF-NEXT:    ret i32 [[SEL]]
+; EXPENSIVE-OFF-NEXT:    ret i32 undef
 ;
 ; EXPENSIVE-ON-LABEL: @test_no_sign_bit_conflict2(
 ; EXPENSIVE-ON-NEXT:    ret i32 0