[APInt] Reorder fields to avoid a hole in the middle of the class

[craig.topper]

Summary

APInt is currently implemented with an unsigned BitWidth field first and then a uint_64/pointer union. Due to the 64-bit size of the union there is a hole after the bitwidth.

Putting the union first allows the class to be packed. Making it 12 bytes instead of 16 bytes. An APSInt goes from 20 bytes to 16 bytes.

This shows a 4k reduction on the size of the opt binary on my local x86-64 build. So this enables some other improvement to the code as well.

[davide]

Nice.

[davide]

I wonder if there's an easy way of testing this so that it doesn't break in the future? Maybe an unit test checking sizeof(). Thoughts?

[hans]

In D32001#725651, @davide wrote:

I wonder if there's an easy way of testing this so that it doesn't break in the future? Maybe an unit test checking sizeof(). Thoughts?

Clang (and GCC too I think) has a -Wpadded parning. Maybe we could #pragma enable it around this class? We have a lot of classes that are supposed to be carefully packed though.

[davide]

In D32001#726060, @hans wrote:

In D32001#725651, @davide wrote:

I wonder if there's an easy way of testing this so that it doesn't break in the future? Maybe an unit test checking sizeof(). Thoughts?

Clang (and GCC too I think) has a -Wpadded parning. Maybe we could #pragma enable it around this class? We have a lot of classes that are supposed to be carefully packed though.

Maybe, I still prefer a static_assert<> but I don't have a strong preference.

[craig.topper]

Turns out MSVC has different packing than gcc/clang. So APSInt doesn't use the padding from APInt for storage. So the static_assert failed on windows.

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

public:
   enum : unsigned {
     /// Byte size of a word.
     APINT_WORD_SIZE = sizeof(WordType),
     /// Bits in a word.
     APINT_BITS_PER_WORD = APINT_WORD_SIZE * CHAR_BIT
   };
 
 private:
-  unsigned BitWidth; ///< The number of bits in this APInt.
-
   /// This union is used to store the integer value. When the
   /// integer bit-width <= 64, it uses VAL, otherwise it uses pVal.
   union {
     uint64_t VAL;   ///< Used to store the <= 64 bits integer value.
     uint64_t *pVal; ///< Used to store the >64 bits integer value.
   };
 
+  unsigned BitWidth; ///< The number of bits in this APInt.
+
   friend struct DenseMapAPIntKeyInfo;
 
   /// \brief Fast internal constructor
   ///
   /// This constructor is used only internally for speed of construction of
   /// temporaries. It is unsafe for general use so it is not public.
-  APInt(uint64_t *val, unsigned bits) : BitWidth(bits), pVal(val) {}
+  APInt(uint64_t *val, unsigned bits) : pVal(val), BitWidth(bits) {}
 
   /// \brief Determine if this APInt just has one word to store value.
   ///
   /// \returns true if the number of bits <= 64, false otherwise.
   bool isSingleWord() const { return BitWidth <= APINT_BITS_PER_WORD; }
 
   /// \brief Determine which word a bit is in.
   ///
public:
   APInt(const APInt &that) : BitWidth(that.BitWidth) {
     if (isSingleWord())
       VAL = that.VAL;
     else
       initSlowCase(that);
   }
 
   /// \brief Move Constructor.
-  APInt(APInt &&that) : BitWidth(that.BitWidth), VAL(that.VAL) {
+  APInt(APInt &&that) : VAL(that.VAL), BitWidth(that.BitWidth) {
     that.BitWidth = 0;
   }
 
   /// \brief Destructor.
   ~APInt() {
     if (needsCleanup())
       delete[] pVal;
   }
 
   /// \brief Default constructor that creates an uninteresting APInt
   /// representing a 1-bit zero value.
   ///
   /// This is useful for object deserialization (pair this with the static
   ///  method Read).
-  explicit APInt() : BitWidth(1), VAL(0) {}
+  explicit APInt() : VAL(0), BitWidth(1) {}
 
   /// \brief Returns whether this instance allocated memory.
   bool needsCleanup() const { return !isSingleWord(); }
 
   /// Used to insert APInt objects, or objects that contain APInt objects, into
   ///  FoldingSets.
   void Profile(FoldingSetNodeID &id) const;
 

llvm/trunk/lib/Support/APInt.cpp

 }
 
 APInt::APInt(unsigned numBits, unsigned numWords, const uint64_t bigVal[])
   : BitWidth(numBits) {
   initFromArray(makeArrayRef(bigVal, numWords));
 }
 
 APInt::APInt(unsigned numbits, StringRef Str, uint8_t radix)
-  : BitWidth(numbits), VAL(0) {
+  : VAL(0), BitWidth(numbits) {
   assert(BitWidth && "Bitwidth too small");
   fromString(numbits, Str, radix);
 }
 
 APInt& APInt::AssignSlowCase(const APInt& RHS) {
   // Don't do anything for X = X
   if (this == &RHS)
     return *this;