[llvm] 9a85643 - [KnownBits] Combine abs() implementations

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2020-11-13T22:23:50+01:00
New Revision: 9a85643cd357e412cff69067bb5c4840e228c2ab

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

LOG: [KnownBits] Combine abs() implementations

ValueTracking was using a more powerful abs() implementation. Roll
it into KnownBits::abs(). Also add an exhaustive test for abs(),
in both the poisoning and non-poisoning variants.

Added: 
    

Modified: 
    llvm/include/llvm/Support/KnownBits.h
    llvm/lib/Analysis/ValueTracking.cpp
    llvm/lib/Support/KnownBits.cpp
    llvm/unittests/Support/KnownBitsTest.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/KnownBits.h b/llvm/include/llvm/Support/KnownBits.h
index b43fde0085db..4c4cd3525137 100644
--- a/llvm/include/llvm/Support/KnownBits.h
+++ b/llvm/include/llvm/Support/KnownBits.h
@@ -326,7 +326,7 @@ struct KnownBits {
   KnownBits &operator^=(const KnownBits &RHS);
 
   /// Compute known bits for the absolute value.
-  KnownBits abs() const;
+  KnownBits abs(bool IntMinIsPoison = false) const;
 
   KnownBits byteSwap() {
     return KnownBits(Zero.byteSwap(), One.byteSwap());

diff  --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index e6d3727a2390..a3c139dbbfa4 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1511,28 +1511,12 @@ static void computeKnownBitsFromOperator(const Operator *I,
     if (const IntrinsicInst *II = dyn_cast<IntrinsicInst>(I)) {
       switch (II->getIntrinsicID()) {
       default: break;
-      case Intrinsic::abs:
+      case Intrinsic::abs: {
         computeKnownBits(I->getOperand(0), Known2, Depth + 1, Q);
-
-        // If the source's MSB is zero then we know the rest of the bits.
-        if (Known2.isNonNegative()) {
-          Known.Zero |= Known2.Zero;
-          Known.One |= Known2.One;
-          break;
-        }
-
-        // Absolute value preserves trailing zero count.
-        Known.Zero.setLowBits(Known2.Zero.countTrailingOnes());
-
-        // If this call is undefined for INT_MIN, the result is positive. We
-        // also know it can't be INT_MIN if there is a set bit that isn't the
-        // sign bit.
-        Known2.One.clearSignBit();
-        if (match(II->getArgOperand(1), m_One()) || Known2.One.getBoolValue())
-          Known.Zero.setSignBit();
-        // FIXME: Handle known negative input?
-        // FIXME: Calculate the negated Known bits and combine them?
+        bool IntMinIsPoison = match(II->getArgOperand(1), m_One());
+        Known = Known2.abs(IntMinIsPoison);
         break;
+      }
       case Intrinsic::bitreverse:
         computeKnownBits(I->getOperand(0), DemandedElts, Known2, Depth + 1, Q);
         Known.Zero |= Known2.Zero.reverseBits();

diff  --git a/llvm/lib/Support/KnownBits.cpp b/llvm/lib/Support/KnownBits.cpp
index 7b94e7b95949..748abf8458d3 100644
--- a/llvm/lib/Support/KnownBits.cpp
+++ b/llvm/lib/Support/KnownBits.cpp
@@ -233,21 +233,23 @@ KnownBits KnownBits::ashr(const KnownBits &LHS, const KnownBits &RHS) {
   return Known;
 }
 
-KnownBits KnownBits::abs() const {
+KnownBits KnownBits::abs(bool IntMinIsPoison) const {
   // If the source's MSB is zero then we know the rest of the bits already.
   if (isNonNegative())
     return *this;
 
-  // Assume we know nothing.
+  // Absolute value preserves trailing zero count.
   KnownBits KnownAbs(getBitWidth());
+  KnownAbs.Zero.setLowBits(countMinTrailingZeros());
 
-  // We only know that the absolute values's MSB will be zero iff there is
-  // a set bit that isn't the sign bit (otherwise it could be INT_MIN).
-  APInt Val = One;
-  Val.clearSignBit();
-  if (!Val.isNullValue())
+  // We only know that the absolute values's MSB will be zero if INT_MIN is
+  // poison, or there is a set bit that isn't the sign bit (otherwise it could
+  // be INT_MIN).
+  if (IntMinIsPoison || (!One.isNullValue() && !One.isMinSignedValue()))
     KnownAbs.Zero.setSignBit();
 
+  // FIXME: Handle known negative input?
+  // FIXME: Calculate the negated Known bits and combine them?
   return KnownAbs;
 }
 

diff  --git a/llvm/unittests/Support/KnownBitsTest.cpp b/llvm/unittests/Support/KnownBitsTest.cpp
index 709a3fcb40c7..c5eb96efc85e 100644
--- a/llvm/unittests/Support/KnownBitsTest.cpp
+++ b/llvm/unittests/Support/KnownBitsTest.cpp
@@ -251,6 +251,36 @@ TEST(KnownBitsTest, BinaryExhaustive) {
   });
 }
 
+TEST(KnownBitsTest, UnaryExhaustive) {
+  unsigned Bits = 4;
+  ForeachKnownBits(Bits, [&](const KnownBits &Known) {
+    KnownBits KnownAbs(Bits);
+    KnownAbs.Zero.setAllBits();
+    KnownAbs.One.setAllBits();
+    KnownBits KnownAbsPoison(KnownAbs);
+
+    ForeachNumInKnownBits(Known, [&](const APInt &N) {
+      APInt Res = N.abs();
+      KnownAbs.One &= Res;
+      KnownAbs.Zero &= ~Res;
+
+      if (!N.isMinSignedValue()) {
+        KnownAbsPoison.One &= Res;
+        KnownAbsPoison.Zero &= ~Res;
+      }
+    });
+
+    // abs() is conservatively correct, but not guaranteed to be precise.
+    KnownBits ComputedAbs = Known.abs();
+    EXPECT_TRUE(ComputedAbs.Zero.isSubsetOf(KnownAbs.Zero));
+    EXPECT_TRUE(ComputedAbs.One.isSubsetOf(KnownAbs.One));
+
+    KnownBits ComputedAbsPoison = Known.abs(true);
+    EXPECT_TRUE(ComputedAbsPoison.Zero.isSubsetOf(KnownAbsPoison.Zero));
+    EXPECT_TRUE(ComputedAbsPoison.One.isSubsetOf(KnownAbsPoison.One));
+  });
+}
+
 TEST(KnownBitsTest, GetMinMaxVal) {
   unsigned Bits = 4;
   ForeachKnownBits(Bits, [&](const KnownBits &Known) {