[llvm] d65ddca - [ValueTracking] ComputeKnownBits - minimum leading/trailing zero bits in LSHR/SHL (PR44526)

[Simon Pilgrim via llvm-commits]

Author: Simon Pilgrim
Date: 2021-02-24T12:15:45Z
New Revision: d65ddca83ff85c7345fe9a0f5a15750f01e38420

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

LOG: [ValueTracking] ComputeKnownBits - minimum leading/trailing zero bits in LSHR/SHL (PR44526)

Followup to D72573 - as detailed in https://blog.regehr.org/archives/1709 we don't make use of the known leading/trailing zeros for shifted values in cases where we don't know the shift amount value.

Stop ValueTracking returning zero for poison shift patterns and use the KnownBits shift helpers directly.

Extend KnownBits::shl to combine all possible shifted combinations if both min/max shift amount values are in range.

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

Added: 
    

Modified: 
    llvm/lib/Analysis/ValueTracking.cpp
    llvm/lib/Support/KnownBits.cpp
    llvm/test/Transforms/InstSimplify/icmp-constant.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 520c7f98b9be..9e5c8b49767e 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -994,30 +994,26 @@ static void computeKnownBitsFromShiftOperator(
   bool ShiftAmtIsConstant = Known.isConstant();
   bool MaxShiftAmtIsOutOfRange = Known.getMaxValue().uge(BitWidth);
 
-  if (ShiftAmtIsConstant) {
-    Known = KF(Known2, Known);
-
-    // 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();
+  // Use the KF callback to get an initial knownbits approximation.
+  Known = KF(Known2, Known);
 
+  // If the known bits conflict, this must be an overflowing left shift, so
+  // the shift result is poison.
+  if (Known.hasConflict()) {
+    Known.resetAll();
     return;
   }
 
+  // Constant shift amount - we're not going to improve on this.
+  if (ShiftAmtIsConstant)
+    return;
+
   // 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 (MaxShiftAmtIsOutOfRange) {
-    Known.resetAll();
+  if (MaxShiftAmtIsOutOfRange)
     return;
-  }
-
-  // It would be more-clearly correct to use the two temporaries for this
-  // calculation. Reusing the APInts here to prevent unnecessary allocations.
-  Known.resetAll();
 
   // If we know the shifter operand is nonzero, we can sometimes infer more
   // known bits. However this is expensive to compute, so be lazy about it and
@@ -1057,10 +1053,9 @@ static void computeKnownBitsFromShiftOperator(
         Known, KF(Known2, KnownBits::makeConstant(APInt(32, ShiftAmt))));
   }
 
-  // If the known bits conflict, the result is poison. Return a 0 and hope the
-  // caller can further optimize that.
+  // If the known bits conflict, the result is poison.
   if (Known.hasConflict())
-    Known.setAllZero();
+    Known.resetAll();
 }
 
 static void computeKnownBitsFromOperator(const Operator *I,
@@ -1232,10 +1227,6 @@ static void computeKnownBitsFromOperator(const Operator *I,
     };
     computeKnownBitsFromShiftOperator(I, DemandedElts, Known, Known2, Depth, Q,
                                       KF);
-    // Trailing zeros of a right-shifted constant never decrease.
-    const APInt *C;
-    if (match(I->getOperand(0), m_APInt(C)))
-      Known.Zero.setLowBits(C->countTrailingZeros());
     break;
   }
   case Instruction::LShr: {
@@ -1244,10 +1235,6 @@ static void computeKnownBitsFromOperator(const Operator *I,
     };
     computeKnownBitsFromShiftOperator(I, DemandedElts, Known, Known2, Depth, Q,
                                       KF);
-    // Leading zeros of a left-shifted constant never decrease.
-    const APInt *C;
-    if (match(I->getOperand(0), m_APInt(C)))
-      Known.Zero.setHighBits(C->countLeadingZeros());
     break;
   }
   case Instruction::AShr: {

diff  --git a/llvm/lib/Support/KnownBits.cpp b/llvm/lib/Support/KnownBits.cpp
index d7265d03d27d..0f87f2209a1c 100644
--- a/llvm/lib/Support/KnownBits.cpp
+++ b/llvm/lib/Support/KnownBits.cpp
@@ -187,6 +187,25 @@ KnownBits KnownBits::shl(const KnownBits &LHS, const KnownBits &RHS) {
     MinTrailingZeros = std::min(MinTrailingZeros, BitWidth);
   }
 
+  // If the maximum shift is in range, then find the common bits from all
+  // possible shifts.
+  APInt MaxShiftAmount = RHS.getMaxValue();
+  if (MaxShiftAmount.ult(BitWidth)) {
+    assert(MinShiftAmount.ult(MaxShiftAmount) && "Illegal shift range");
+    Known.Zero.setAllBits();
+    Known.One.setAllBits();
+    for (uint64_t ShiftAmt = MinShiftAmount.getZExtValue(),
+                  MaxShiftAmt = MaxShiftAmount.getZExtValue();
+         ShiftAmt <= MaxShiftAmt; ++ShiftAmt) {
+      KnownBits SpecificShift;
+      SpecificShift.Zero = LHS.Zero << ShiftAmt;
+      SpecificShift.One = LHS.One << ShiftAmt;
+      Known = KnownBits::commonBits(Known, SpecificShift);
+      if (Known.isUnknown())
+        break;
+    }
+  }
+
   Known.Zero.setLowBits(MinTrailingZeros);
   return Known;
 }

diff  --git a/llvm/test/Transforms/InstSimplify/icmp-constant.ll b/llvm/test/Transforms/InstSimplify/icmp-constant.ll
index f8d5bdc89ceb..9db63e64b914 100644
--- a/llvm/test/Transforms/InstSimplify/icmp-constant.ll
+++ b/llvm/test/Transforms/InstSimplify/icmp-constant.ll
@@ -799,7 +799,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:    ret i1 false
+; CHECK-NEXT:    ret i1 poison
 ;
   %clear_high_bit = and i8 %x, 127                 ; 0x7f
   %set_next_high_bits = or i8 %clear_high_bit, 112 ; 0x70