[Alignment][NFC] migrate DataLayout internal struct to llvm::Align

[gchatelet]

Summary

This is patch is part of a series to introduce an Alignment type.
See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html
See this patch for the introduction of the type: https://reviews.llvm.org/D64790

With this patch the sizeof(PointerAlignElem) struct goes from 20B to 16B, while sizeof(LayoutAlignElem) stays at 8B (the struct is padded)

[gchatelet]

• Format

[courbet]

llvm/lib/IR/DataLayout.cpp

54

const

369–376

I would move this back to DataLayout::setAlignment, or ask for a review to the original authors. They might have had other reasons beyond storage for guarding against this size.

[gchatelet]

llvm/lib/IR/DataLayout.cpp

369–376

@majnemer it looks like the checks are only because of implementation details (from 1b9fc3a186aed6ec120688e4d957466f72db3ed5)
Would it make sense to remove them as well as the tests?

[gchatelet]

• Address comments

[gchatelet]

A gentle ping @majnemer

[gchatelet]

• Adding asserts to setAlignment

[gchatelet]

llvm/lib/IR/DataLayout.cpp

369–376

@majnemer I'll submit the code as-is, if you have any concerns please let me know and I can fix it in subsequent patches. Thx!

llvm/include/llvm/IR/DataLayout.h

 /// vector, float) and type bit width.
 ///
 /// \note The unusual order of elements in the structure attempts to reduce
 /// padding and make the structure slightly more cache friendly.
 struct LayoutAlignElem {
   /// Alignment type from \c AlignTypeEnum
   unsigned AlignType : 8;
   unsigned TypeBitWidth : 24;
-  unsigned ABIAlign : 16;
-  unsigned PrefAlign : 16;
+  llvm::Align ABIAlign;
+  llvm::Align PrefAlign;
 
-  static LayoutAlignElem get(AlignTypeEnum align_type, unsigned abi_align,
-                             unsigned pref_align, uint32_t bit_width);
+  static LayoutAlignElem get(AlignTypeEnum align_type, llvm::Align abi_align,
+                             llvm::Align pref_align, uint32_t bit_width);
 
   bool operator==(const LayoutAlignElem &rhs) const;
 };
 
 /// Layout pointer alignment element.
 ///
 /// Stores the alignment data associated with a given pointer and address space.
 ///
 /// \note The unusual order of elements in the structure attempts to reduce
 /// padding and make the structure slightly more cache friendly.
 struct PointerAlignElem {
-  unsigned ABIAlign;
-  unsigned PrefAlign;
+  llvm::Align ABIAlign;
+  llvm::Align PrefAlign;
   uint32_t TypeByteWidth;
   uint32_t AddressSpace;
   uint32_t IndexWidth;
 
   /// Initializer
-  static PointerAlignElem get(uint32_t AddressSpace, unsigned ABIAlign,
-                              unsigned PrefAlign, uint32_t TypeByteWidth,
+  static PointerAlignElem get(uint32_t AddressSpace, llvm::Align ABIAlign,
+                              llvm::Align PrefAlign, uint32_t TypeByteWidth,
                               uint32_t IndexWidth);
 
   bool operator==(const PointerAlignElem &rhs) const;
 };
 
 /// A parsed version of the target data layout string in and methods for
 /// querying it.
 ///
private:
 
   // The StructType -> StructLayout map.
   mutable void *LayoutMap = nullptr;
 
   /// Pointers in these address spaces are non-integral, and don't have a
   /// well-defined bitwise representation.
   SmallVector<unsigned, 8> NonIntegralAddressSpaces;
 
-  void setAlignment(AlignTypeEnum align_type, unsigned abi_align,
-                    unsigned pref_align, uint32_t bit_width);
+  void setAlignment(AlignTypeEnum align_type, llvm::Align abi_align,
+                    llvm::Align pref_align, uint32_t bit_width);
   unsigned getAlignmentInfo(AlignTypeEnum align_type, uint32_t bit_width,
                             bool ABIAlign, Type *Ty) const;
-  void setPointerAlignment(uint32_t AddrSpace, unsigned ABIAlign,
-                           unsigned PrefAlign, uint32_t TypeByteWidth,
+  void setPointerAlignment(uint32_t AddrSpace, llvm::Align ABIAlign,
+                           llvm::Align PrefAlign, uint32_t TypeByteWidth,
                            uint32_t IndexWidth);
 
   /// Internal helper method that returns requested alignment for type.
   unsigned getAlignment(Type *Ty, bool abi_or_pref) const;
 
   /// Parses a target data specification string. Assert if the string is
   /// malformed.
   void parseSpecifier(StringRef LayoutDescription);

 inline LLVMTargetDataRef wrap(const DataLayout *P) {
   return reinterpret_cast<LLVMTargetDataRef>(const_cast<DataLayout *>(P));
 }
 
 /// Used to lazily calculate structure layout information for a target machine,
 /// based on the DataLayout structure.
 class StructLayout {
   uint64_t StructSize;
-  unsigned StructAlignment;
+  llvm::Align StructAlignment;
   unsigned IsPadded : 1;
   unsigned NumElements : 31;
   uint64_t MemberOffsets[1]; // variable sized array!
 
 public:
   uint64_t getSizeInBytes() const { return StructSize; }
 
   uint64_t getSizeInBits() const { return 8 * StructSize; }
 
-  unsigned getAlignment() const { return StructAlignment; }
+  unsigned getAlignment() const { return StructAlignment.value(); }
 
   /// Returns whether the struct has padding or not between its fields.
   /// NB: Padding in nested element is not taken into account.
   bool hasPadding() const { return IsPadded; }
 
   /// Given a valid byte offset into the structure, returns the structure
   /// index that contains it.
   unsigned getElementContainingOffset(uint64_t Offset) const;

llvm/lib/IR/DataLayout.cpp

 using namespace llvm;
 
 //===----------------------------------------------------------------------===//
 // Support for StructLayout
 //===----------------------------------------------------------------------===//
 
 StructLayout::StructLayout(StructType *ST, const DataLayout &DL) {
   assert(!ST->isOpaque() && "Cannot get layout of opaque structs");
-  StructAlignment = 0;
   StructSize = 0;
   IsPadded = false;
   NumElements = ST->getNumElements();
 
   // Loop over each of the elements, placing them in memory.
   for (unsigned i = 0, e = NumElements; i != e; ++i) {
     Type *Ty = ST->getElementType(i);
-    unsigned TyAlign = ST->isPacked() ? 1 : DL.getABITypeAlignment(Ty);
+    const llvm::Align TyAlign(ST->isPacked() ? 1 : DL.getABITypeAlignment(Ty));
 
     // Add padding if necessary to align the data element properly.
-    if ((StructSize & (TyAlign-1)) != 0) {
+    if (!isAligned(TyAlign, StructSize)) {
       IsPadded = true;
       StructSize = alignTo(StructSize, TyAlign);
     }
 
     // Keep track of maximum alignment constraint.
     StructAlignment = std::max(TyAlign, StructAlignment);
 
     MemberOffsets[i] = StructSize;
     StructSize += DL.getTypeAllocSize(Ty); // Consume space for this data item
   }
 
-  // Empty structures have alignment of 1 byte.
-  if (StructAlignment == 0) StructAlignment = 1;
-
   // Add padding to the end of the struct so that it could be put in an array
   // and all array elements would be aligned correctly.
-  if ((StructSize & (StructAlignment-1)) != 0) {
+  if (!isAligned(StructAlignment, StructSize)) {
     IsPadded = true;
     StructSize = alignTo(StructSize, StructAlignment);
   }
 }
 
 /// getElementContainingOffset - Given a valid offset into the structure,
 /// return the structure index that contains it.
 unsigned StructLayout::getElementContainingOffset(uint64_t Offset) const {
unsigned StructLayout::getElementContainingOffset(uint64_t Offset) const {
   // offset, implying that this element is non-empty.
   return SI-&MemberOffsets[0];
 }
 
 //===----------------------------------------------------------------------===//
 // LayoutAlignElem, LayoutAlign support
 //===----------------------------------------------------------------------===//
 
-LayoutAlignElem
-LayoutAlignElem::get(AlignTypeEnum align_type, unsigned abi_align,
-                     unsigned pref_align, uint32_t bit_width) {
+LayoutAlignElem LayoutAlignElem::get(AlignTypeEnum align_type,
+                                     llvm::Align abi_align,
+                                     llvm::Align pref_align,
+                                     uint32_t bit_width) {
   assert(abi_align <= pref_align && "Preferred alignment worse than ABI!");
   LayoutAlignElem retval;
   retval.AlignType = align_type;
   retval.ABIAlign = abi_align;
   retval.PrefAlign = pref_align;
   retval.TypeBitWidth = bit_width;
   return retval;
 }
 
 bool
 LayoutAlignElem::operator==(const LayoutAlignElem &rhs) const {
   return (AlignType == rhs.AlignType
           && ABIAlign == rhs.ABIAlign
           && PrefAlign == rhs.PrefAlign
           && TypeBitWidth == rhs.TypeBitWidth);
 }
 
 //===----------------------------------------------------------------------===//
 // PointerAlignElem, PointerAlign support
 //===----------------------------------------------------------------------===//
 
-PointerAlignElem
-PointerAlignElem::get(uint32_t AddressSpace, unsigned ABIAlign,
-                      unsigned PrefAlign, uint32_t TypeByteWidth,
+PointerAlignElem PointerAlignElem::get(uint32_t AddressSpace,
+                                       llvm::Align ABIAlign,
+                                       llvm::Align PrefAlign,
+                                       uint32_t TypeByteWidth,
                                        uint32_t IndexWidth) {
   assert(ABIAlign <= PrefAlign && "Preferred alignment worse than ABI!");
   PointerAlignElem retval;
   retval.AddressSpace = AddressSpace;
   retval.ABIAlign = ABIAlign;
   retval.PrefAlign = PrefAlign;
   retval.TypeByteWidth = TypeByteWidth;
   retval.IndexWidth = IndexWidth;
   return retval;
const char *DataLayout::getManglingComponent(const Triple &T) {
   if (T.isOSBinFormatMachO())
     return "-m:o";
   if (T.isOSWindows() && T.isOSBinFormatCOFF())
     return T.getArch() == Triple::x86 ? "-m:x" : "-m:w";
   return "-m:e";
 }
 
 static const LayoutAlignElem DefaultAlignments[] = {
-  { INTEGER_ALIGN, 1, 1, 1 },    // i1
-  { INTEGER_ALIGN, 8, 1, 1 },    // i8
-  { INTEGER_ALIGN, 16, 2, 2 },   // i16
-  { INTEGER_ALIGN, 32, 4, 4 },   // i32
-  { INTEGER_ALIGN, 64, 4, 8 },   // i64
-  { FLOAT_ALIGN, 16, 2, 2 },     // half
-  { FLOAT_ALIGN, 32, 4, 4 },     // float
-  { FLOAT_ALIGN, 64, 8, 8 },     // double
-  { FLOAT_ALIGN, 128, 16, 16 },  // ppcf128, quad, ...
-  { VECTOR_ALIGN, 64, 8, 8 },    // v2i32, v1i64, ...
-  { VECTOR_ALIGN, 128, 16, 16 }, // v16i8, v8i16, v4i32, ...
-  { AGGREGATE_ALIGN, 0, 0, 8 }   // struct
+    {INTEGER_ALIGN, 1, llvm::Align(1), llvm::Align(1)},   // i1
+    {INTEGER_ALIGN, 8, llvm::Align(1), llvm::Align(1)},   // i8
+    {INTEGER_ALIGN, 16, llvm::Align(2), llvm::Align(2)},  // i16
+    {INTEGER_ALIGN, 32, llvm::Align(4), llvm::Align(4)},  // i32
+    {INTEGER_ALIGN, 64, llvm::Align(4), llvm::Align(8)},  // i64
+    {FLOAT_ALIGN, 16, llvm::Align(2), llvm::Align(2)},    // half
+    {FLOAT_ALIGN, 32, llvm::Align(4), llvm::Align(4)},    // float
+    {FLOAT_ALIGN, 64, llvm::Align(8), llvm::Align(8)},    // double
+    {FLOAT_ALIGN, 128, llvm::Align(16), llvm::Align(16)}, // ppcf128, quad, ...
+    {VECTOR_ALIGN, 64, llvm::Align(8), llvm::Align(8)},   // v2i32, v1i64, ...
+    {VECTOR_ALIGN, 128, llvm::Align(16),
+     llvm::Align(16)}, // v16i8, v8i16, v4i32, ...
+    {AGGREGATE_ALIGN, 0, llvm::Align(1), llvm::Align(8)} // struct
 };
 
 void DataLayout::reset(StringRef Desc) {
   clear();
 
   LayoutMap = nullptr;
   BigEndian = false;
   AllocaAddrSpace = 0;
   StackNaturalAlign.reset();
   ProgramAddrSpace = 0;
   FunctionPtrAlign.reset();
   TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
   ManglingMode = MM_None;
   NonIntegralAddressSpaces.clear();
 
   // Default alignments
   for (const LayoutAlignElem &E : DefaultAlignments) {
     setAlignment((AlignTypeEnum)E.AlignType, E.ABIAlign, E.PrefAlign,
                  E.TypeBitWidth);
   }
-  setPointerAlignment(0, 8, 8, 8, 8);
+  setPointerAlignment(0, llvm::Align(8), llvm::Align(8), 8, 8);
 
   parseSpecifier(Desc);
 }
 
 /// Checked version of split, to ensure mandatory subparts.
 static std::pair<StringRef, StringRef> split(StringRef Str, char Separator) {
   assert(!Str.empty() && "parse error, string can't be empty here");
   std::pair<StringRef, StringRef> Split = Str.split(Separator);
case 'p': {
         // Now read the index. It is the second optional parameter here.
         if (!Rest.empty()) {
           Split = split(Rest, ':');
           IndexSize = inBytes(getInt(Tok));
           if (!IndexSize)
             report_fatal_error("Invalid index size of 0 bytes");
         }
       }
-      setPointerAlignment(AddrSpace, PointerABIAlign, PointerPrefAlign,
-                          PointerMemSize, IndexSize);
+      setPointerAlignment(AddrSpace, assumeAligned(PointerABIAlign),
+                          assumeAligned(PointerPrefAlign), PointerMemSize,
+                          IndexSize);
       break;
     }
     case 'i':
     case 'v':
     case 'f':
     case 'a': {
       AlignTypeEnum AlignType;
       switch (Specifier) {
case 'a': {
         report_fatal_error(
             "Sized aggregate specification in datalayout string");
 
       // ABI alignment.
       if (Rest.empty())
         report_fatal_error(
             "Missing alignment specification in datalayout string");
       Split = split(Rest, ':');
-      unsigned ABIAlign = inBytes(getInt(Tok));
+      const unsigned ABIAlign = inBytes(getInt(Tok));
       if (AlignType != AGGREGATE_ALIGN && !ABIAlign)
         report_fatal_error(
             "ABI alignment specification must be >0 for non-aggregate types");
 
+      if (!isUInt<16>(ABIAlign))
+        report_fatal_error("Invalid ABI alignment, must be a 16bit integer");
+      if (ABIAlign != 0 && !isPowerOf2_64(ABIAlign))
+        report_fatal_error("Invalid ABI alignment, must be a power of 2");
+
       // Preferred alignment.
       unsigned PrefAlign = ABIAlign;
       if (!Rest.empty()) {
         Split = split(Rest, ':');
         PrefAlign = inBytes(getInt(Tok));
       }
 
-      setAlignment(AlignType, ABIAlign, PrefAlign, Size);
+      if (!isUInt<16>(PrefAlign))
+        report_fatal_error(
+            "Invalid preferred alignment, must be a 16bit integer");
+      if (PrefAlign != 0 && !isPowerOf2_64(PrefAlign))
+        report_fatal_error("Invalid preferred alignment, must be a power of 2");
+
+      setAlignment(AlignType, assumeAligned(ABIAlign), assumeAligned(PrefAlign),
+                   Size);
 
       break;
     }
     case 'n':  // Native integer types.
       while (true) {
         unsigned Width = getInt(Tok);
         if (Width == 0)
           report_fatal_error(

 DataLayout::findAlignmentLowerBound(AlignTypeEnum AlignType,
                                     uint32_t BitWidth) {
   auto Pair = std::make_pair((unsigned)AlignType, BitWidth);
   return partition_point(Alignments, [=](const LayoutAlignElem &E) {
     return std::make_pair(E.AlignType, E.TypeBitWidth) < Pair;
   });
 }
 
-void
-DataLayout::setAlignment(AlignTypeEnum align_type, unsigned abi_align,
-                         unsigned pref_align, uint32_t bit_width) {
+void DataLayout::setAlignment(AlignTypeEnum align_type, llvm::Align abi_align,
+                              llvm::Align pref_align, uint32_t bit_width) {
+  // AlignmentsTy::ABIAlign and AlignmentsTy::PrefAlign were once stored as
+  // uint16_t, it is unclear if there are requirements for alignment to be less
+  // than 2^16 other than storage. In the meantime we leave the restriction as
+  // an assert. See D67400 for context.
+  assert(Log2(abi_align) < 16 && Log2(pref_align) < 16 && "Alignment too big");
   if (!isUInt<24>(bit_width))
     report_fatal_error("Invalid bit width, must be a 24bit integer");
-  if (!isUInt<16>(abi_align))
-    report_fatal_error("Invalid ABI alignment, must be a 16bit integer");
-  if (!isUInt<16>(pref_align))
-    report_fatal_error("Invalid preferred alignment, must be a 16bit integer");
-  if (abi_align != 0 && !isPowerOf2_64(abi_align))
-    report_fatal_error("Invalid ABI alignment, must be a power of 2");
-  if (pref_align != 0 && !isPowerOf2_64(pref_align))
-    report_fatal_error("Invalid preferred alignment, must be a power of 2");
-
   if (pref_align < abi_align)
     report_fatal_error(
         "Preferred alignment cannot be less than the ABI alignment");
 
   AlignmentsTy::iterator I = findAlignmentLowerBound(align_type, bit_width);
   if (I != Alignments.end() &&
       I->AlignType == (unsigned)align_type && I->TypeBitWidth == bit_width) {
     // Update the abi, preferred alignments.

 DataLayout::PointersTy::iterator
 DataLayout::findPointerLowerBound(uint32_t AddressSpace) {
   return std::lower_bound(Pointers.begin(), Pointers.end(), AddressSpace,
                           [](const PointerAlignElem &A, uint32_t AddressSpace) {
     return A.AddressSpace < AddressSpace;
   });
 }
 
-void DataLayout::setPointerAlignment(uint32_t AddrSpace, unsigned ABIAlign,
-                                     unsigned PrefAlign, uint32_t TypeByteWidth,
+void DataLayout::setPointerAlignment(uint32_t AddrSpace, llvm::Align ABIAlign,
+                                     llvm::Align PrefAlign,
+                                     uint32_t TypeByteWidth,
                                      uint32_t IndexWidth) {
   if (PrefAlign < ABIAlign)
     report_fatal_error(
         "Preferred alignment cannot be less than the ABI alignment");
 
   PointersTy::iterator I = findPointerLowerBound(AddrSpace);
   if (I == Pointers.end() || I->AddressSpace != AddrSpace) {
     Pointers.insert(I, PointerAlignElem::get(AddrSpace, ABIAlign, PrefAlign,
unsigned DataLayout::getAlignmentInfo(AlignTypeEnum AlignType,
                                       uint32_t BitWidth, bool ABIInfo,
                                       Type *Ty) const {
   AlignmentsTy::const_iterator I = findAlignmentLowerBound(AlignType, BitWidth);
   // See if we found an exact match. Of if we are looking for an integer type,
   // but don't have an exact match take the next largest integer. This is where
   // the lower_bound will point to when it fails an exact match.
   if (I != Alignments.end() && I->AlignType == (unsigned)AlignType &&
       (I->TypeBitWidth == BitWidth || AlignType == INTEGER_ALIGN))
-    return ABIInfo ? I->ABIAlign : I->PrefAlign;
+    return (ABIInfo ? I->ABIAlign : I->PrefAlign).value();
 
   if (AlignType == INTEGER_ALIGN) {
     // If we didn't have a larger value try the largest value we have.
     if (I != Alignments.begin()) {
       --I; // Go to the previous entry and see if its an integer.
       if (I->AlignType == INTEGER_ALIGN)
-        return ABIInfo ? I->ABIAlign : I->PrefAlign;
+        return (ABIInfo ? I->ABIAlign : I->PrefAlign).value();
     }
   } else if (AlignType == VECTOR_ALIGN) {
     // By default, use natural alignment for vector types. This is consistent
     // with what clang and llvm-gcc do.
     unsigned Align = getTypeAllocSize(cast<VectorType>(Ty)->getElementType());
     Align *= cast<VectorType>(Ty)->getNumElements();
     Align = PowerOf2Ceil(Align);
     return Align;

 }
 
 unsigned DataLayout::getPointerABIAlignment(unsigned AS) const {
   PointersTy::const_iterator I = findPointerLowerBound(AS);
   if (I == Pointers.end() || I->AddressSpace != AS) {
     I = findPointerLowerBound(0);
     assert(I->AddressSpace == 0);
   }
-  return I->ABIAlign;
+  return I->ABIAlign.value();
 }
 
 unsigned DataLayout::getPointerPrefAlignment(unsigned AS) const {
   PointersTy::const_iterator I = findPointerLowerBound(AS);
   if (I == Pointers.end() || I->AddressSpace != AS) {
     I = findPointerLowerBound(0);
     assert(I->AddressSpace == 0);
   }
-  return I->PrefAlign;
+  return I->PrefAlign.value();
 }
 
 unsigned DataLayout::getPointerSize(unsigned AS) const {
   PointersTy::const_iterator I = findPointerLowerBound(AS);
   if (I == Pointers.end() || I->AddressSpace != AS) {
     I = findPointerLowerBound(0);
     assert(I->AddressSpace == 0);
   }