[APInt] Fix bugs in isShiftedMask to match behavior of the similar function in MathEx...

This removes a parameter from the routine that was responsible for a lot of the issue. It was a bit count that had to be set to the BitWidth of the APInt and would get passed to getLowBitsSet. This guaranteed the call to getLowBitsSet would create an all ones value. This was then compared to (V | (V-1)). So the only shifted masks we detected had to have the MSB set.

The one in tree user is a transform in InstCombine that never fires due to earlier transforms covering the case better. I've submitted a patch to remove it completely, but for now I've just adapted it to the new interface for isShiftedMask.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@299273 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/ADT/APInt.h b/include/llvm/ADT/APInt.h
index 699f429fd3c63956ecdfd8bc158d77812a2cd280..8adbfdfeecbdab03ef45327690ddc29379696af4 100644 (file)
--- a/include/llvm/ADT/APInt.h
+++ b/include/llvm/ADT/APInt.h
@@ -1934,8 +1934,8 @@ inline bool isMask(const APInt &Value) {
 
 /// \brief Return true if the argument APInt value contains a sequence of ones
 /// with the remainder zero.
-inline bool isShiftedMask(unsigned numBits, const APInt &APIVal) {
-  return isMask(numBits, (APIVal - APInt(numBits, 1)) | APIVal);
+inline bool isShiftedMask(const APInt &APIVal) {
+  return (APIVal != 0) && isMask((APIVal - 1) | APIVal);
 }
 
 /// \brief Compute GCD of two APInt values.

diff --git a/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp b/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
index 13f3f31fdf91dc3abee5c810174117529ffd7308..2c6a455483d4969ea62d0cfa94b7826c008269c4 100644 (file)
--- a/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
+++ b/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
@@ -309,7 +309,7 @@ Value *InstCombiner::insertRangeTest(Value *V, const APInt &Lo, const APInt &Hi,
 static bool isRunOfOnes(ConstantInt *Val, uint32_t &MB, uint32_t &ME) {
   const APInt& V = Val->getValue();
   uint32_t BitWidth = Val->getType()->getBitWidth();
-  if (!APIntOps::isShiftedMask(BitWidth, V)) return false;
+  if (!APIntOps::isShiftedMask(V)) return false;
 
   // look for the first zero bit after the run of ones
   MB = BitWidth - ((V - 1) ^ V).countLeadingZeros();

diff --git a/unittests/ADT/APIntTest.cpp b/unittests/ADT/APIntTest.cpp
index 51c6347cbe8005d257fabfa98075b31ad6b1595d..bf7d8d833399f9f2df806bde00d274a3294ad055 100644 (file)
--- a/unittests/ADT/APIntTest.cpp
+++ b/unittests/ADT/APIntTest.cpp
@@ -1576,26 +1576,26 @@ TEST(APIntTest, isMask) {
 }
 
 TEST(APIntTest, isShiftedMask) {
-  EXPECT_FALSE(APIntOps::isShiftedMask(32, APInt(32, 0x01010101)));
-  EXPECT_TRUE(APIntOps::isShiftedMask(32, APInt(32, 0xf0000000)));
-  EXPECT_TRUE(APIntOps::isShiftedMask(32, APInt(32, 0xffff0000)));
-  EXPECT_FALSE(APIntOps::isShiftedMask(32, APInt(32, 0xff << 1))); // BUG
+  EXPECT_FALSE(APIntOps::isShiftedMask(APInt(32, 0x01010101)));
+  EXPECT_TRUE(APIntOps::isShiftedMask(APInt(32, 0xf0000000)));
+  EXPECT_TRUE(APIntOps::isShiftedMask(APInt(32, 0xffff0000)));
+  EXPECT_TRUE(APIntOps::isShiftedMask(APInt(32, 0xff << 1)));
 
   for (int N : { 1, 2, 3, 4, 7, 8, 16, 32, 64, 127, 128, 129, 256 }) {
-    EXPECT_TRUE(APIntOps::isShiftedMask(N, APInt(N, 0))); // BUG
+    EXPECT_FALSE(APIntOps::isShiftedMask(APInt(N, 0)));
 
     APInt One(N, 1);
     for (int I = 1; I < N; ++I) {
       APInt MaskVal = One.shl(I) - 1;
-      EXPECT_FALSE(APIntOps::isShiftedMask(N, MaskVal)); // BUG
+      EXPECT_TRUE(APIntOps::isShiftedMask(MaskVal));
     }
     for (int I = 1; I < N - 1; ++I) {
       APInt MaskVal = One.shl(I);
-      EXPECT_FALSE(APIntOps::isShiftedMask(N, MaskVal)); // BUG
+      EXPECT_TRUE(APIntOps::isShiftedMask(MaskVal));
     }
     for (int I = 1; I < N; ++I) {
       APInt MaskVal = APInt::getHighBitsSet(N, I);
-      EXPECT_TRUE(APIntOps::isShiftedMask(N, MaskVal));
+      EXPECT_TRUE(APIntOps::isShiftedMask(MaskVal));
     }
   }
 }