[APInt] Add rvalue reference support to and, or, xor operations to allow their memory allocation to be reused when possible

[craig.topper]

Summary

This extends an earlier change that did similar for add and sub operations.

With this first patch we lose the fastpath for the single word case as operator&= and friends don't support it. This can be added there if we think that's important.

I had to change some functions in the APInt class since the operator overloads were moved out of the class and can't be used inside the class now. The getBitsSet change collides with another outstanding patch to implement it with setBits. But I didn't want to make this patch dependent on that series.

I've also removed the Or, And, Xor functions which were rarely or never used. I already commited two changes to remove the only uses of Or that existed.

[dblaikie]

Are the cleanup changes (removing unused operations, etc) tied to the main change here, or could they be done separately?

Also, it looks like the main change could be tested by using getRawData to demonstrate that the moves rather than copies happened, maybe?

(that said, the move ctor looks questionable - it copies a possibly inactive member of a union & relies on that to copy the other member of the union)

[craig.topper]

Removing the And, Or, Xor functions could be removed separately.

The change to getBitsSet and intersects were necessary because of the operator functions being moved out of line. But they could be done as a pre-patch.

I hadn't looked at the move constructor. I was basing this on r276470 and assuming it was ok. Should I change it to use memcpy like the move assignment operator?

[craig.topper]

Add unit tests. Rebase for the removal of And/Or/Xor methods in r296993.

[dblaikie]

Looks reasonable - I would (vaguely/generally - haven't thought about it very hard) wonder if the test cases could be simpler. I'd expect only one constant, maybe - (b1100 - or any other 4 bit sequence with 2 x 1s and 2 x 0s, to test the usual combination of ones and zeros on each side in each operation?). & hardcode the expected result with a literal instead of another APInt? Not sure. Anyway - if that seems like the tidiest/simplest testing for it, that's OK.

llvm/trunk/include/llvm/ADT/APInt.h

class LLVM_NODISCARD APInt {
   void initFromArray(ArrayRef<uint64_t> array);
 
   /// out-of-line slow case for inline copy constructor
   void initSlowCase(const APInt &that);
 
   /// out-of-line slow case for shl
   APInt shlSlowCase(unsigned shiftAmt) const;
 
-  /// out-of-line slow case for operator&
-  APInt AndSlowCase(const APInt &RHS) const;
-
-  /// out-of-line slow case for operator|
-  APInt OrSlowCase(const APInt &RHS) const;
-
-  /// out-of-line slow case for operator^
-  APInt XorSlowCase(const APInt &RHS) const;
-
   /// out-of-line slow case for operator=
   APInt &AssignSlowCase(const APInt &RHS);
 
   /// out-of-line slow case for operator==
   bool EqualSlowCase(const APInt &RHS) const;
 
   /// out-of-line slow case for operator==
   bool EqualSlowCase(uint64_t Val) const;
APInt &operator<<=(unsigned shiftAmt) {
     *this = shl(shiftAmt);
     return *this;
   }
 
   /// @}
   /// \name Binary Operators
   /// @{
 
-  /// \brief Bitwise AND operator.
-  ///
-  /// Performs a bitwise AND operation on *this and RHS.
-  ///
-  /// \returns An APInt value representing the bitwise AND of *this and RHS.
-  APInt operator&(const APInt &RHS) const {
-    assert(BitWidth == RHS.BitWidth && "Bit widths must be the same");
-    if (isSingleWord())
-      return APInt(getBitWidth(), VAL & RHS.VAL);
-    return AndSlowCase(RHS);
-  }
-
-  /// \brief Bitwise OR operator.
-  ///
-  /// Performs a bitwise OR operation on *this and RHS.
-  ///
-  /// \returns An APInt value representing the bitwise OR of *this and RHS.
-  APInt operator|(const APInt &RHS) const {
-    assert(BitWidth == RHS.BitWidth && "Bit widths must be the same");
-    if (isSingleWord())
-      return APInt(getBitWidth(), VAL | RHS.VAL);
-    return OrSlowCase(RHS);
-  }
-
-  /// \brief Bitwise XOR operator.
-  ///
-  /// Performs a bitwise XOR operation on *this and RHS.
-  ///
-  /// \returns An APInt value representing the bitwise XOR of *this and RHS.
-  APInt operator^(const APInt &RHS) const {
-    assert(BitWidth == RHS.BitWidth && "Bit widths must be the same");
-    if (isSingleWord())
-      return APInt(BitWidth, VAL ^ RHS.VAL);
-    return XorSlowCase(RHS);
-  }
-
   /// \brief Multiplication operator.
   ///
   /// Multiplies this APInt by RHS and returns the result.
   APInt operator*(const APInt &RHS) const;
 
   /// \brief Left logical shift operator.
   ///
   /// Shifts this APInt left by \p Bits and returns the result.
public:
   /// Regards both *this as a signed quantity and compares it with RHS for
   /// the validity of the greater-or-equal relationship.
   ///
   /// \returns true if *this >= RHS when considered signed.
   bool sge(int64_t RHS) const { return !slt(RHS); }
 
   /// This operation tests if there are any pairs of corresponding bits
   /// between this APInt and RHS that are both set.
-  bool intersects(const APInt &RHS) const { return (*this & RHS) != 0; }
+  bool intersects(const APInt &RHS) const {
+    APInt temp(*this);
+    temp &= RHS;
+    return temp != 0;
+  }
 
   /// @}
   /// \name Resizing Operators
   /// @{
 
   /// \brief Truncate to new width.
   ///
   /// Truncate the APInt to a specified width. It is an error to specify a width

 /// \brief Unary bitwise complement operator.
 ///
 /// \returns an APInt that is the bitwise complement of \p v.
 inline APInt operator~(APInt v) {
   v.flipAllBits();
   return v;
 }
 
+inline APInt operator&(APInt a, const APInt &b) {
+  a &= b;
+  return a;
+}
+
+inline APInt operator&(const APInt &a, APInt &&b) {
+  b &= a;
+  return std::move(b);
+}
+
 inline APInt operator&(APInt a, uint64_t RHS) {
   a &= RHS;
   return a;
 }
 
 inline APInt operator&(uint64_t LHS, APInt b) {
   b &= LHS;
   return b;
 }
 
+inline APInt operator|(APInt a, const APInt &b) {
+  a |= b;
+  return a;
+}
+
+inline APInt operator|(const APInt &a, APInt &&b) {
+  b |= a;
+  return std::move(b);
+}
+
 inline APInt operator|(APInt a, uint64_t RHS) {
   a |= RHS;
   return a;
 }
 
 inline APInt operator|(uint64_t LHS, APInt b) {
   b |= LHS;
   return b;
 }
 
+inline APInt operator^(APInt a, const APInt &b) {
+  a ^= b;
+  return a;
+}
+
+inline APInt operator^(const APInt &a, APInt &&b) {
+  b ^= a;
+  return std::move(b);
+}
+
 inline APInt operator^(APInt a, uint64_t RHS) {
   a ^= RHS;
   return a;
 }
 
 inline APInt operator^(uint64_t LHS, APInt b) {
   b ^= LHS;
   return b;

llvm/trunk/lib/Support/APInt.cpp

if (isSingleWord()) {
     return *this;
   }
   unsigned numWords = getNumWords();
   for (unsigned i = 0; i < numWords; ++i)
     pVal[i] ^= RHS.pVal[i];
   return *this;
 }
 
-APInt APInt::AndSlowCase(const APInt& RHS) const {
-  unsigned numWords = getNumWords();
-  uint64_t* val = getMemory(numWords);
-  for (unsigned i = 0; i < numWords; ++i)
-    val[i] = pVal[i] & RHS.pVal[i];
-  return APInt(val, getBitWidth());
-}
-
-APInt APInt::OrSlowCase(const APInt& RHS) const {
-  unsigned numWords = getNumWords();
-  uint64_t *val = getMemory(numWords);
-  for (unsigned i = 0; i < numWords; ++i)
-    val[i] = pVal[i] | RHS.pVal[i];
-  return APInt(val, getBitWidth());
-}
-
-APInt APInt::XorSlowCase(const APInt& RHS) const {
-  unsigned numWords = getNumWords();
-  uint64_t *val = getMemory(numWords);
-  for (unsigned i = 0; i < numWords; ++i)
-    val[i] = pVal[i] ^ RHS.pVal[i];
-
-  return APInt(val, getBitWidth());
-}
-
 APInt APInt::operator*(const APInt& RHS) const {
   assert(BitWidth == RHS.BitWidth && "Bit widths must be the same");
   if (isSingleWord())
     return APInt(BitWidth, VAL * RHS.VAL);
   APInt Result(*this);
   Result *= RHS;
   return Result;
 }

llvm/trunk/unittests/ADT/APIntTest.cpp

const uint64_t *RawDataR = nullptr;
     EXPECT_EQ(SubRK.getRawData(), RawDataL);
 
     APInt SubKR = 2 - getRValue("1FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", RawDataR);
     EXPECT_EQ(SubKR, Three);
     EXPECT_EQ(SubKR.getRawData(), RawDataR);
   }
 }
 
+TEST(APIntTest, rvalue_bitwise) {
+  // Test all combinations of lvalue/rvalue lhs/rhs of and/or/xor
+
+  // Lamdba to return an APInt by value, but also provide the raw value of the
+  // allocated data.
+  auto getRValue = [](const char *HexString, uint64_t const *&RawData) {
+    APInt V(129, HexString, 16);
+    RawData = V.getRawData();
+    return V;
+  };
+
+  APInt Ten(129, "A", 16);
+  APInt Twelve(129, "C", 16);
+
+  const uint64_t *RawDataL = nullptr;
+  const uint64_t *RawDataR = nullptr;
+
+  {
+    // 12 & 10 = 8
+    APInt AndLL = Ten & Twelve;
+    EXPECT_EQ(AndLL, 0x8);
+
+    APInt AndLR = Ten & getRValue("C", RawDataR);
+    EXPECT_EQ(AndLR, 0x8);
+    EXPECT_EQ(AndLR.getRawData(), RawDataR);
+
+    APInt AndRL = getRValue("A", RawDataL) & Twelve;
+    EXPECT_EQ(AndRL, 0x8);
+    EXPECT_EQ(AndRL.getRawData(), RawDataL);
+
+    APInt AndRR = getRValue("A", RawDataL) & getRValue("C", RawDataR);
+    EXPECT_EQ(AndRR, 0x8);
+    EXPECT_EQ(AndRR.getRawData(), RawDataR);
+
+    // LValue's and constants
+    APInt AndLK = Ten & 0xc;
+    EXPECT_EQ(AndLK, 0x8);
+
+    APInt AndKL = 0xa & Twelve;
+    EXPECT_EQ(AndKL, 0x8);
+
+    // RValue's and constants
+    APInt AndRK = getRValue("A", RawDataL) & 0xc;
+    EXPECT_EQ(AndRK, 0x8);
+    EXPECT_EQ(AndRK.getRawData(), RawDataL);
+
+    APInt AndKR = 0xa & getRValue("C", RawDataR);
+    EXPECT_EQ(AndKR, 0x8);
+    EXPECT_EQ(AndKR.getRawData(), RawDataR);
+  }
+
+  {
+    // 12 | 10 = 14
+    APInt OrLL = Ten | Twelve;
+    EXPECT_EQ(OrLL, 0xe);
+
+    APInt OrLR = Ten | getRValue("C", RawDataR);
+    EXPECT_EQ(OrLR, 0xe);
+    EXPECT_EQ(OrLR.getRawData(), RawDataR);
+
+    APInt OrRL = getRValue("A", RawDataL) | Twelve;
+    EXPECT_EQ(OrRL, 0xe);
+    EXPECT_EQ(OrRL.getRawData(), RawDataL);
+
+    APInt OrRR = getRValue("A", RawDataL) | getRValue("C", RawDataR);
+    EXPECT_EQ(OrRR, 0xe);
+    EXPECT_EQ(OrRR.getRawData(), RawDataR);
+
+    // LValue's and constants
+    APInt OrLK = Ten | 0xc;
+    EXPECT_EQ(OrLK, 0xe);
+
+    APInt OrKL = 0xa | Twelve;
+    EXPECT_EQ(OrKL, 0xe);
+
+    // RValue's and constants
+    APInt OrRK = getRValue("A", RawDataL) | 0xc;
+    EXPECT_EQ(OrRK, 0xe);
+    EXPECT_EQ(OrRK.getRawData(), RawDataL);
+
+    APInt OrKR = 0xa | getRValue("C", RawDataR);
+    EXPECT_EQ(OrKR, 0xe);
+    EXPECT_EQ(OrKR.getRawData(), RawDataR);
+  }
+
+  {
+    // 12 ^ 10 = 6
+    APInt XorLL = Ten ^ Twelve;
+    EXPECT_EQ(XorLL, 0x6);
+
+    APInt XorLR = Ten ^ getRValue("C", RawDataR);
+    EXPECT_EQ(XorLR, 0x6);
+    EXPECT_EQ(XorLR.getRawData(), RawDataR);
+
+    APInt XorRL = getRValue("A", RawDataL) ^ Twelve;
+    EXPECT_EQ(XorRL, 0x6);
+    EXPECT_EQ(XorRL.getRawData(), RawDataL);
+
+    APInt XorRR = getRValue("A", RawDataL) ^ getRValue("C", RawDataR);
+    EXPECT_EQ(XorRR, 0x6);
+    EXPECT_EQ(XorRR.getRawData(), RawDataR);
+
+    // LValue's and constants
+    APInt XorLK = Ten ^ 0xc;
+    EXPECT_EQ(XorLK, 0x6);
+
+    APInt XorKL = 0xa ^ Twelve;
+    EXPECT_EQ(XorKL, 0x6);
+
+    // RValue's and constants
+    APInt XorRK = getRValue("A", RawDataL) ^ 0xc;
+    EXPECT_EQ(XorRK, 0x6);
+    EXPECT_EQ(XorRK.getRawData(), RawDataL);
+
+    APInt XorKR = 0xa ^ getRValue("C", RawDataR);
+    EXPECT_EQ(XorKR, 0x6);
+    EXPECT_EQ(XorKR.getRawData(), RawDataR);
+  }
+}
+
 TEST(APIntTest, rvalue_invert) {
   // Lamdba to return an APInt by value, but also provide the raw value of the
   // allocated data.
   auto getRValue = [](const char *HexString, uint64_t const *&RawData) {
     APInt V(129, HexString, 16);
     RawData = V.getRawData();
     return V;
   };