[APInt] Fix isAllOnes and extractBits for zero width values.

[lattner]

Summary

isAllOnes() should return true for zero bit values because
there are no zeros in it.

Thanks to Jay Foad for pointing this out.

[foad]

This makes me happy!

llvm/unittests/ADT/APIntTest.cpp

2994

'true' would be better than '1U', but isn't there an EXPECT_TRUE for this?

[lattner]

use EXPECT_TRUE as Jay points out

[lattner]

Jay happiness++

[joerg]

Did this get an audit for existing users? It can be argued in both ways. E.g. "".isdigit() in Python is False, because there is no digit contained and that's pretty much the same use case.

[foad]

In D111241#3058363, @joerg wrote:

Did this get an audit for existing users? It can be argued in both ways. E.g. "".isdigit() in Python is False, because there is no digit contained and that's pretty much the same use case.

AFAIK there are no in-tree users to audit: zero-width APInts were only introduced very recently, and I don't think they're used in-tree except for unit tests.

[lattner]

Right, they are only being used in the CIRCT incubator project afaik, and it is fine.

llvm/include/llvm/ADT/APInt.h

public:
   /// \returns true if this APInt is positive.
   bool isStrictlyPositive() const { return isNonNegative() && !isZero(); }
 
   /// Determine if this APInt Value is non-positive (<= 0).
   ///
   /// \returns true if this APInt is non-positive.
   bool isNonPositive() const { return !isStrictlyPositive(); }
 
-  /// Determine if all bits are set.
+  /// Determine if all bits are set.  This is true for zero-width values.
   bool isAllOnes() const {
-    if (isSingleWord()) {
-      // Calculate the shift amount, handling the zero-bit wide case without UB.
-      unsigned ShiftAmt =
-          (APINT_BITS_PER_WORD - BitWidth) % APINT_BITS_PER_WORD;
-      return U.VAL == WORDTYPE_MAX >> ShiftAmt;
-    }
+    if (BitWidth == 0)
+      return true;
+    if (isSingleWord())
+      return U.VAL == WORDTYPE_MAX >> (APINT_BITS_PER_WORD - BitWidth);
     return countTrailingOnesSlowCase() == BitWidth;
   }
 
   /// NOTE: This is soft-deprecated.  Please use `isAllOnes()` instead.
   bool isAllOnesValue() const { return isAllOnes(); }
 
   /// Determine if this value is zero, i.e. all bits are clear.
   bool isZero() const {

llvm/lib/Support/APInt.cpp

void APInt::insertBits(uint64_t subBits, unsigned bitPosition, unsigned numBits) {
   U.pVal[loWord] &= ~(maskBits << loBit);
   U.pVal[loWord] |= subBits << loBit;
 
   U.pVal[hiWord] &= ~(maskBits >> (wordBits - loBit));
   U.pVal[hiWord] |= subBits >> (wordBits - loBit);
 }
 
 APInt APInt::extractBits(unsigned numBits, unsigned bitPosition) const {
-  assert(numBits > 0 && "Can't extract zero bits");
   assert(bitPosition < BitWidth && (numBits + bitPosition) <= BitWidth &&
          "Illegal bit extraction");
 
   if (isSingleWord())
     return APInt(numBits, U.VAL >> bitPosition);
 
   unsigned loBit = whichBit(bitPosition);
   unsigned loWord = whichWord(bitPosition);

llvm/unittests/ADT/APIntTest.cpp

TEST(APIntTest, ZeroWidth) {
   EXPECT_EQ(0U, ZW2.getBitWidth());
   // Assignment
   ZW = ZW2;
   EXPECT_EQ(0U, ZW.getBitWidth());
 
   // Methods like getLowBitsSet work with zero bits.
   EXPECT_EQ(0U, APInt::getLowBitsSet(0, 0).getBitWidth());
   EXPECT_EQ(0U, APInt::getSplat(0, ZW).getBitWidth());
+  EXPECT_EQ(0U, APInt(4, 10).extractBits(0, 2).getBitWidth());
 
   // Logical operators.
   ZW |= ZW2;
   ZW &= ZW2;
   ZW ^= ZW2;
   ZW |= 42; // These ignore high bits of the literal.
   ZW &= 42;
   ZW ^= 42;
TEST(APIntTest, ZeroWidth) {
   EXPECT_EQ(0U, ZW.countLeadingZeros());
   EXPECT_EQ(0U, ZW.countLeadingOnes());
   EXPECT_EQ(0U, ZW.countPopulation());
   EXPECT_EQ(0U, ZW.reverseBits().getBitWidth());
   EXPECT_EQ(0U, ZW.getHiBits(0).getBitWidth());
   EXPECT_EQ(0U, ZW.getLoBits(0).getBitWidth());
   EXPECT_EQ(0, ZW.zext(4));
   EXPECT_EQ(0U, APInt(4, 3).trunc(0).getBitWidth());
+  EXPECT_TRUE(ZW.isAllOnes());
 
   SmallString<42> STR;
   ZW.toStringUnsigned(STR);
   EXPECT_EQ("0", STR);
 
   // Move ctor (keep at the end of the method since moves are destructive).
   APInt MZW1(std::move(ZW));
   EXPECT_EQ(0U, MZW1.getBitWidth());