[llvm] r299273 - [APInt] Fix bugs in isShiftedMask to match behavior of the similar function in MathExtras.h

[Craig Topper via llvm-commits]

Author: ctopper
Date: Fri Mar 31 17:23:42 2017
New Revision: 299273

URL: http://llvm.org/viewvc/llvm-project?rev=299273&view=rev
Log:
[APInt] Fix bugs in isShiftedMask to match behavior of the similar function in MathExtras.h

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.


Modified:
    llvm/trunk/include/llvm/ADT/APInt.h
    llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
    llvm/trunk/unittests/ADT/APIntTest.cpp

Modified: llvm/trunk/include/llvm/ADT/APInt.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=299273&r1=299272&r2=299273&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/APInt.h (original)
+++ llvm/trunk/include/llvm/ADT/APInt.h Fri Mar 31 17:23:42 2017
@@ -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.

Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp?rev=299273&r1=299272&r2=299273&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp Fri Mar 31 17:23:42 2017
@@ -309,7 +309,7 @@ Value *InstCombiner::insertRangeTest(Val
 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();

Modified: llvm/trunk/unittests/ADT/APIntTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APIntTest.cpp?rev=299273&r1=299272&r2=299273&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/APIntTest.cpp (original)
+++ llvm/trunk/unittests/ADT/APIntTest.cpp Fri Mar 31 17:23:42 2017
@@ -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));
     }
   }
 }