[llvm] [DataLayout] Move '*AlignElem' structs and enum inside DataLayout (NFC) (PR #103723)

Sergei Barannikov via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 14 17:32:25 PDT 2024


https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/103723

>From 7df4e253f334c611b93d942e00ce85c9d18182a4 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Tue, 13 Aug 2024 22:09:36 +0300
Subject: [PATCH 1/3] [DataLayout] Inline static constructors (NFC)

The removed asserts check invariants already checked by the caller.
---
 llvm/include/llvm/IR/DataLayout.h |  8 --------
 llvm/lib/IR/DataLayout.cpp        | 31 +++----------------------------
 2 files changed, 3 insertions(+), 36 deletions(-)

diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 795cd05ea5b5e2..dd97c732884585 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -70,9 +70,6 @@ struct LayoutAlignElem {
   Align ABIAlign;
   Align PrefAlign;
 
-  static LayoutAlignElem get(Align ABIAlign, Align PrefAlign,
-                             uint32_t BitWidth);
-
   bool operator==(const LayoutAlignElem &rhs) const;
 };
 
@@ -86,11 +83,6 @@ struct PointerAlignElem {
   Align PrefAlign;
   uint32_t IndexBitWidth;
 
-  /// Initializer
-  static PointerAlignElem getInBits(uint32_t AddressSpace, Align ABIAlign,
-                                    Align PrefAlign, uint32_t TypeBitWidth,
-                                    uint32_t IndexBitWidth);
-
   bool operator==(const PointerAlignElem &rhs) const;
 };
 
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 530979c75063b4..7ac528031652d8 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -142,16 +142,6 @@ class StructLayoutMap {
 // LayoutAlignElem, LayoutAlign support
 //===----------------------------------------------------------------------===//
 
-LayoutAlignElem LayoutAlignElem::get(Align ABIAlign, Align PrefAlign,
-                                     uint32_t BitWidth) {
-  assert(ABIAlign <= PrefAlign && "Preferred alignment worse than ABI!");
-  LayoutAlignElem retval;
-  retval.ABIAlign = ABIAlign;
-  retval.PrefAlign = PrefAlign;
-  retval.TypeBitWidth = BitWidth;
-  return retval;
-}
-
 bool LayoutAlignElem::operator==(const LayoutAlignElem &rhs) const {
   return ABIAlign == rhs.ABIAlign && PrefAlign == rhs.PrefAlign &&
          TypeBitWidth == rhs.TypeBitWidth;
@@ -161,20 +151,6 @@ bool LayoutAlignElem::operator==(const LayoutAlignElem &rhs) const {
 // PointerAlignElem, PointerAlign support
 //===----------------------------------------------------------------------===//
 
-PointerAlignElem PointerAlignElem::getInBits(uint32_t AddressSpace,
-                                             Align ABIAlign, Align PrefAlign,
-                                             uint32_t TypeBitWidth,
-                                             uint32_t IndexBitWidth) {
-  assert(ABIAlign <= PrefAlign && "Preferred alignment worse than ABI!");
-  PointerAlignElem retval;
-  retval.AddressSpace = AddressSpace;
-  retval.ABIAlign = ABIAlign;
-  retval.PrefAlign = PrefAlign;
-  retval.TypeBitWidth = TypeBitWidth;
-  retval.IndexBitWidth = IndexBitWidth;
-  return retval;
-}
-
 bool
 PointerAlignElem::operator==(const PointerAlignElem &rhs) const {
   return (ABIAlign == rhs.ABIAlign && AddressSpace == rhs.AddressSpace &&
@@ -654,7 +630,7 @@ Error DataLayout::setAlignment(AlignTypeEnum AlignType, Align ABIAlign,
     I->PrefAlign = PrefAlign;
   } else {
     // Insert before I to keep the vector sorted.
-    Alignments->insert(I, LayoutAlignElem::get(ABIAlign, PrefAlign, BitWidth));
+    Alignments->insert(I, LayoutAlignElem{BitWidth, ABIAlign, PrefAlign});
   }
   return Error::success();
 }
@@ -689,9 +665,8 @@ Error DataLayout::setPointerAlignmentInBits(uint32_t AddrSpace, Align ABIAlign,
     return A.AddressSpace < AddressSpace;
   });
   if (I == Pointers.end() || I->AddressSpace != AddrSpace) {
-    Pointers.insert(I,
-                    PointerAlignElem::getInBits(AddrSpace, ABIAlign, PrefAlign,
-                                                TypeBitWidth, IndexBitWidth));
+    Pointers.insert(I, PointerAlignElem{AddrSpace, TypeBitWidth, ABIAlign,
+                                        PrefAlign, IndexBitWidth});
   } else {
     I->ABIAlign = ABIAlign;
     I->PrefAlign = PrefAlign;

>From 61bc6b7a2c3815ddbc887a945823cd5e1c742b35 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Wed, 14 Aug 2024 06:40:59 +0300
Subject: [PATCH 2/3] [DataLayout] Move '*AlignElem' structs and enum inside
 DataLayout (NFC)

---
 llvm/include/llvm/IR/DataLayout.h |  96 +++++++-------
 llvm/lib/IR/DataLayout.cpp        | 207 +++++++++++++++---------------
 2 files changed, 151 insertions(+), 152 deletions(-)

diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index dd97c732884585..4bf1c61c892ae9 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -49,43 +49,11 @@ class StructLayout;
 class Triple;
 class Value;
 
-/// Enum used to categorize the alignment types stored by LayoutAlignElem
-enum AlignTypeEnum {
-  INTEGER_ALIGN = 'i',
-  VECTOR_ALIGN = 'v',
-  FLOAT_ALIGN = 'f',
-  AGGREGATE_ALIGN = 'a'
-};
-
 // FIXME: Currently the DataLayout string carries a "preferred alignment"
 // for types. As the DataLayout is module/global, this should likely be
 // sunk down to an FTTI element that is queried rather than a global
 // preference.
 
-/// Layout alignment element.
-///
-/// Stores the alignment data associated with a given type bit width.
-struct LayoutAlignElem {
-  uint32_t TypeBitWidth;
-  Align ABIAlign;
-  Align PrefAlign;
-
-  bool operator==(const LayoutAlignElem &rhs) const;
-};
-
-/// Layout pointer alignment element.
-///
-/// Stores the alignment data associated with a given pointer and address space.
-struct PointerAlignElem {
-  uint32_t AddressSpace;
-  uint32_t TypeBitWidth;
-  Align ABIAlign;
-  Align PrefAlign;
-  uint32_t IndexBitWidth;
-
-  bool operator==(const PointerAlignElem &rhs) const;
-};
-
 /// A parsed version of the target data layout string in and methods for
 /// querying it.
 ///
@@ -94,6 +62,26 @@ struct PointerAlignElem {
 /// target being codegen'd to.
 class DataLayout {
 public:
+  /// Primitive type specification.
+  struct PrimitiveSpec {
+    uint32_t BitWidth;
+    Align ABIAlign;
+    Align PrefAlign;
+
+    bool operator==(const PrimitiveSpec &Other) const;
+  };
+
+  /// Pointer type specification.
+  struct PointerSpec {
+    uint32_t AddrSpace;
+    uint32_t BitWidth;
+    Align ABIAlign;
+    Align PrefAlign;
+    uint32_t IndexBitWidth;
+
+    bool operator==(const PointerSpec &Other) const;
+  };
+
   enum class FunctionPtrAlignType {
     /// The function pointer alignment is independent of the function alignment.
     Independent,
@@ -127,19 +115,26 @@ class DataLayout {
   // FIXME: `unsigned char` truncates the value parsed by `parseSpecifier`.
   SmallVector<unsigned char, 8> LegalIntWidths;
 
+  // Primitive type specifier.
+  enum class PrimitiveSpecifier {
+    Integer = 'i',
+    Float = 'f',
+    Vector = 'v',
+    // TODO: Aggregates are not primitives. This should be separated.
+    Aggregate = 'a'
+  };
+
   // Primitive type specifications. Sorted and uniqued by type bit width.
-  SmallVector<LayoutAlignElem, 6> IntAlignments;
-  SmallVector<LayoutAlignElem, 4> FloatAlignments;
-  SmallVector<LayoutAlignElem, 10> VectorAlignments;
+  SmallVector<PrimitiveSpec, 6> IntSpecs;
+  SmallVector<PrimitiveSpec, 4> FloatSpecs;
+  SmallVector<PrimitiveSpec, 10> VectorSpecs;
 
   // Pointer type specifications. Sorted and uniqued by address space number.
-  SmallVector<PointerAlignElem, 8> Pointers;
+  SmallVector<PointerSpec, 8> PointerSpecs;
 
   /// The string representation used to create this DataLayout
   std::string StringRepresentation;
 
-  const PointerAlignElem &getPointerAlignElem(uint32_t AddressSpace) const;
-
   // Struct type ABI and preferred alignments. The default spec is "a:8:64".
   Align StructABIAlignment = Align::Constant<1>();
   Align StructPrefAlignment = Align::Constant<8>();
@@ -151,16 +146,19 @@ class DataLayout {
   /// well-defined bitwise representation.
   SmallVector<unsigned, 8> NonIntegralAddressSpaces;
 
-  /// Attempts to set the alignment of the given type. Returns an error
-  /// description on failure.
-  Error setAlignment(AlignTypeEnum AlignType, Align ABIAlign, Align PrefAlign,
-                     uint32_t BitWidth);
+  // Attempts to set the specification for the given type.
+  // Returns an error description on failure.
+  Error setPrimitiveSpec(PrimitiveSpecifier Specifier, uint32_t BitWidth,
+                         Align ABIAlign, Align PrefAlign);
+
+  // Searches for a pointer specification that matches the given address space.
+  // Returns the default address space specification if not found.
+  const PointerSpec &getPointerSpec(uint32_t Spec) const;
 
-  /// Attempts to set the alignment of a pointer in the given address space.
-  /// Returns an error description on failure.
-  Error setPointerAlignmentInBits(uint32_t AddrSpace, Align ABIAlign,
-                                  Align PrefAlign, uint32_t TypeBitWidth,
-                                  uint32_t IndexBitWidth);
+  // Attempts to set the specification for pointer in the given address space.
+  // Returns an error description on failure.
+  Error setPointerSpec(uint32_t AddrSpace, uint32_t BitWidth, Align ABIAlign,
+                       Align PrefAlign, uint32_t IndexBitWidth);
 
   /// Internal helper to get alignment for integer of given bitwidth.
   Align getIntegerAlignment(uint32_t BitWidth, bool abi_or_pref) const;
@@ -367,7 +365,7 @@ class DataLayout {
   /// FIXME: The defaults need to be removed once all of
   /// the backends/clients are updated.
   unsigned getPointerSizeInBits(unsigned AS = 0) const {
-    return getPointerAlignElem(AS).TypeBitWidth;
+    return getPointerSpec(AS).BitWidth;
   }
 
   /// Returns the maximum index size over all address spaces.
@@ -377,7 +375,7 @@ class DataLayout {
 
   /// Size in bits of index used for address calculation in getelementptr.
   unsigned getIndexSizeInBits(unsigned AS) const {
-    return getPointerAlignElem(AS).IndexBitWidth;
+    return getPointerSpec(AS).IndexBitWidth;
   }
 
   /// Layout pointer size, in bits, based on the type.  If this function is
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 7ac528031652d8..552da2de2d2e08 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -139,29 +139,20 @@ class StructLayoutMap {
 } // end anonymous namespace
 
 //===----------------------------------------------------------------------===//
-// LayoutAlignElem, LayoutAlign support
+//                       DataLayout Class Implementation
 //===----------------------------------------------------------------------===//
 
-bool LayoutAlignElem::operator==(const LayoutAlignElem &rhs) const {
-  return ABIAlign == rhs.ABIAlign && PrefAlign == rhs.PrefAlign &&
-         TypeBitWidth == rhs.TypeBitWidth;
+bool DataLayout::PrimitiveSpec::operator==(const PrimitiveSpec &Other) const {
+  return BitWidth == Other.BitWidth && ABIAlign == Other.ABIAlign &&
+         PrefAlign == Other.PrefAlign;
 }
 
-//===----------------------------------------------------------------------===//
-// PointerAlignElem, PointerAlign support
-//===----------------------------------------------------------------------===//
-
-bool
-PointerAlignElem::operator==(const PointerAlignElem &rhs) const {
-  return (ABIAlign == rhs.ABIAlign && AddressSpace == rhs.AddressSpace &&
-          PrefAlign == rhs.PrefAlign && TypeBitWidth == rhs.TypeBitWidth &&
-          IndexBitWidth == rhs.IndexBitWidth);
+bool DataLayout::PointerSpec::operator==(const PointerSpec &Other) const {
+  return AddrSpace == Other.AddrSpace && BitWidth == Other.BitWidth &&
+         ABIAlign == Other.ABIAlign && PrefAlign == Other.PrefAlign &&
+         IndexBitWidth == Other.IndexBitWidth;
 }
 
-//===----------------------------------------------------------------------===//
-//                       DataLayout Class Implementation
-//===----------------------------------------------------------------------===//
-
 const char *DataLayout::getManglingComponent(const Triple &T) {
   if (T.isOSBinFormatGOFF())
     return "-m:l";
@@ -176,34 +167,34 @@ const char *DataLayout::getManglingComponent(const Triple &T) {
 
 // Default primitive type specifications.
 // NOTE: These arrays must be sorted by type bit width.
-constexpr LayoutAlignElem DefaultIntSpecs[] = {
+constexpr DataLayout::PrimitiveSpec DefaultIntSpecs[] = {
     {1, Align::Constant<1>(), Align::Constant<1>()},  // i1:8:8
     {8, Align::Constant<1>(), Align::Constant<1>()},  // i8:8:8
     {16, Align::Constant<2>(), Align::Constant<2>()}, // i16:16:16
     {32, Align::Constant<4>(), Align::Constant<4>()}, // i32:32:32
     {64, Align::Constant<4>(), Align::Constant<8>()}, // i64:32:64
 };
-constexpr LayoutAlignElem DefaultFloatSpecs[] = {
+constexpr DataLayout::PrimitiveSpec DefaultFloatSpecs[] = {
     {16, Align::Constant<2>(), Align::Constant<2>()},    // f16:16:16
     {32, Align::Constant<4>(), Align::Constant<4>()},    // f32:32:32
     {64, Align::Constant<8>(), Align::Constant<8>()},    // f64:64:64
     {128, Align::Constant<16>(), Align::Constant<16>()}, // f128:128:128
 };
-constexpr LayoutAlignElem DefaultVectorSpecs[] = {
+constexpr DataLayout::PrimitiveSpec DefaultVectorSpecs[] = {
     {64, Align::Constant<8>(), Align::Constant<8>()},    // v64:64:64
     {128, Align::Constant<16>(), Align::Constant<16>()}, // v128:128:128
 };
 
 // Default pointer type specifications.
-constexpr PointerAlignElem DefaultPointerSpecs[] = {
+constexpr DataLayout::PointerSpec DefaultPointerSpecs[] = {
     {0, 64, Align::Constant<8>(), Align::Constant<8>(), 64} // p0:64:64:64:64
 };
 
 DataLayout::DataLayout()
-    : IntAlignments(ArrayRef(DefaultIntSpecs)),
-      FloatAlignments(ArrayRef(DefaultFloatSpecs)),
-      VectorAlignments(ArrayRef(DefaultVectorSpecs)),
-      Pointers(ArrayRef(DefaultPointerSpecs)) {}
+    : IntSpecs(ArrayRef(DefaultIntSpecs)),
+      FloatSpecs(ArrayRef(DefaultFloatSpecs)),
+      VectorSpecs(ArrayRef(DefaultVectorSpecs)),
+      PointerSpecs(ArrayRef(DefaultPointerSpecs)) {}
 
 DataLayout::DataLayout(StringRef LayoutString) : DataLayout() {
   if (Error Err = parseSpecifier(LayoutString))
@@ -223,10 +214,10 @@ DataLayout &DataLayout::operator=(const DataLayout &Other) {
   TheFunctionPtrAlignType = Other.TheFunctionPtrAlignType;
   ManglingMode = Other.ManglingMode;
   LegalIntWidths = Other.LegalIntWidths;
-  IntAlignments = Other.IntAlignments;
-  FloatAlignments = Other.FloatAlignments;
-  VectorAlignments = Other.VectorAlignments;
-  Pointers = Other.Pointers;
+  IntSpecs = Other.IntSpecs;
+  FloatSpecs = Other.FloatSpecs;
+  VectorSpecs = Other.VectorSpecs;
+  PointerSpecs = Other.PointerSpecs;
   StructABIAlignment = Other.StructABIAlignment;
   StructPrefAlignment = Other.StructPrefAlignment;
   NonIntegralAddressSpaces = Other.NonIntegralAddressSpaces;
@@ -244,11 +235,9 @@ bool DataLayout::operator==(const DataLayout &Other) const {
          FunctionPtrAlign == Other.FunctionPtrAlign &&
          TheFunctionPtrAlignType == Other.TheFunctionPtrAlignType &&
          ManglingMode == Other.ManglingMode &&
-         LegalIntWidths == Other.LegalIntWidths &&
-         IntAlignments == Other.IntAlignments &&
-         FloatAlignments == Other.FloatAlignments &&
-         VectorAlignments == Other.VectorAlignments &&
-         Pointers == Other.Pointers &&
+         LegalIntWidths == Other.LegalIntWidths && IntSpecs == Other.IntSpecs &&
+         FloatSpecs == Other.FloatSpecs && VectorSpecs == Other.VectorSpecs &&
+         PointerSpecs == Other.PointerSpecs &&
          StructABIAlignment == Other.StructABIAlignment &&
          StructPrefAlignment == Other.StructPrefAlignment;
 }
@@ -409,9 +398,9 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
             return reportError("Invalid index size of 0 bytes");
         }
       }
-      if (Error Err = setPointerAlignmentInBits(
-              AddrSpace, assumeAligned(PointerABIAlign),
-              assumeAligned(PointerPrefAlign), PointerMemSize, IndexSize))
+      if (Error Err = setPointerSpec(
+              AddrSpace, PointerMemSize, assumeAligned(PointerABIAlign),
+              assumeAligned(PointerPrefAlign), IndexSize))
         return Err;
       break;
     }
@@ -419,13 +408,22 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
     case 'v':
     case 'f':
     case 'a': {
-      AlignTypeEnum AlignType;
+      PrimitiveSpecifier PrimSpecifier;
       switch (Specifier) {
-      default: llvm_unreachable("Unexpected specifier!");
-      case 'i': AlignType = INTEGER_ALIGN; break;
-      case 'v': AlignType = VECTOR_ALIGN; break;
-      case 'f': AlignType = FLOAT_ALIGN; break;
-      case 'a': AlignType = AGGREGATE_ALIGN; break;
+      default:
+        llvm_unreachable("Unexpected specifier!");
+      case 'i':
+        PrimSpecifier = PrimitiveSpecifier::Integer;
+        break;
+      case 'v':
+        PrimSpecifier = PrimitiveSpecifier::Vector;
+        break;
+      case 'f':
+        PrimSpecifier = PrimitiveSpecifier::Float;
+        break;
+      case 'a':
+        PrimSpecifier = PrimitiveSpecifier::Aggregate;
+        break;
       }
 
       // Bit size.
@@ -434,7 +432,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
         if (Error Err = getInt(Tok, Size))
           return Err;
 
-      if (AlignType == AGGREGATE_ALIGN && Size != 0)
+      if (PrimSpecifier == PrimitiveSpecifier::Aggregate && Size != 0)
         return reportError(
             "Sized aggregate specification in datalayout string");
 
@@ -447,7 +445,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
       unsigned ABIAlign;
       if (Error Err = getIntInBytes(Tok, ABIAlign))
         return Err;
-      if (AlignType != AGGREGATE_ALIGN && !ABIAlign)
+      if (PrimSpecifier != PrimitiveSpecifier::Aggregate && !ABIAlign)
         return reportError(
             "ABI alignment specification must be >0 for non-aggregate types");
 
@@ -455,7 +453,8 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
         return reportError("Invalid ABI alignment, must be a 16bit integer");
       if (ABIAlign != 0 && !isPowerOf2_64(ABIAlign))
         return reportError("Invalid ABI alignment, must be a power of 2");
-      if (AlignType == INTEGER_ALIGN && Size == 8 && ABIAlign != 1)
+      if (PrimSpecifier == PrimitiveSpecifier::Integer && Size == 8 &&
+          ABIAlign != 1)
         return reportError(
             "Invalid ABI alignment, i8 must be naturally aligned");
 
@@ -474,8 +473,9 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
       if (PrefAlign != 0 && !isPowerOf2_64(PrefAlign))
         return reportError("Invalid preferred alignment, must be a power of 2");
 
-      if (Error Err = setAlignment(AlignType, assumeAligned(ABIAlign),
-                                   assumeAligned(PrefAlign), Size))
+      if (Error Err =
+              setPrimitiveSpec(PrimSpecifier, Size, assumeAligned(ABIAlign),
+                               assumeAligned(PrefAlign)))
         return Err;
 
       break;
@@ -583,16 +583,18 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
   return Error::success();
 }
 
-static SmallVectorImpl<LayoutAlignElem>::const_iterator
-findAlignmentLowerBound(const SmallVectorImpl<LayoutAlignElem> &Alignments,
-                        uint32_t BitWidth) {
-  return partition_point(Alignments, [BitWidth](const LayoutAlignElem &E) {
-    return E.TypeBitWidth < BitWidth;
+static SmallVectorImpl<DataLayout::PrimitiveSpec>::const_iterator
+findPrimitiveSpecLowerBound(
+    const SmallVectorImpl<DataLayout::PrimitiveSpec> &Specs,
+    uint32_t BitWidth) {
+  return partition_point(Specs, [BitWidth](const DataLayout::PrimitiveSpec &E) {
+    return E.BitWidth < BitWidth;
   });
 }
 
-Error DataLayout::setAlignment(AlignTypeEnum AlignType, Align ABIAlign,
-                               Align PrefAlign, uint32_t BitWidth) {
+Error DataLayout::setPrimitiveSpec(PrimitiveSpecifier Specifier,
+                                   uint32_t BitWidth, Align ABIAlign,
+                                   Align PrefAlign) {
   // 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
@@ -604,73 +606,72 @@ Error DataLayout::setAlignment(AlignTypeEnum AlignType, Align ABIAlign,
     return reportError(
         "Preferred alignment cannot be less than the ABI alignment");
 
-  SmallVectorImpl<LayoutAlignElem> *Alignments;
-  switch (AlignType) {
-  case AGGREGATE_ALIGN:
+  SmallVectorImpl<PrimitiveSpec> *Specs;
+  switch (Specifier) {
+  case PrimitiveSpecifier::Aggregate:
     StructABIAlignment = ABIAlign;
     StructPrefAlignment = PrefAlign;
     return Error::success();
-  case INTEGER_ALIGN:
-    Alignments = &IntAlignments;
+  case PrimitiveSpecifier::Integer:
+    Specs = &IntSpecs;
     break;
-  case FLOAT_ALIGN:
-    Alignments = &FloatAlignments;
+  case PrimitiveSpecifier::Float:
+    Specs = &FloatSpecs;
     break;
-  case VECTOR_ALIGN:
-    Alignments = &VectorAlignments;
+  case PrimitiveSpecifier::Vector:
+    Specs = &VectorSpecs;
     break;
   }
 
-  auto I = partition_point(*Alignments, [BitWidth](const LayoutAlignElem &E) {
-    return E.TypeBitWidth < BitWidth;
+  auto I = partition_point(*Specs, [BitWidth](const PrimitiveSpec &E) {
+    return E.BitWidth < BitWidth;
   });
-  if (I != Alignments->end() && I->TypeBitWidth == BitWidth) {
+  if (I != Specs->end() && I->BitWidth == BitWidth) {
     // Update the abi, preferred alignments.
     I->ABIAlign = ABIAlign;
     I->PrefAlign = PrefAlign;
   } else {
     // Insert before I to keep the vector sorted.
-    Alignments->insert(I, LayoutAlignElem{BitWidth, ABIAlign, PrefAlign});
+    Specs->insert(I, PrimitiveSpec{BitWidth, ABIAlign, PrefAlign});
   }
   return Error::success();
 }
 
-const PointerAlignElem &
-DataLayout::getPointerAlignElem(uint32_t AddressSpace) const {
-  if (AddressSpace != 0) {
-    auto I = lower_bound(Pointers, AddressSpace,
-                         [](const PointerAlignElem &A, uint32_t AddressSpace) {
-      return A.AddressSpace < AddressSpace;
-    });
-    if (I != Pointers.end() && I->AddressSpace == AddressSpace)
+const DataLayout::PointerSpec &
+DataLayout::getPointerSpec(uint32_t AddrSpace) const {
+  if (AddrSpace != 0) {
+    auto I = lower_bound(PointerSpecs, AddrSpace,
+                         [](const PointerSpec &Spec, uint32_t AddrSpace) {
+                           return Spec.AddrSpace < AddrSpace;
+                         });
+    if (I != PointerSpecs.end() && I->AddrSpace == AddrSpace)
       return *I;
   }
 
-  assert(Pointers[0].AddressSpace == 0);
-  return Pointers[0];
+  assert(PointerSpecs[0].AddrSpace == 0);
+  return PointerSpecs[0];
 }
 
-Error DataLayout::setPointerAlignmentInBits(uint32_t AddrSpace, Align ABIAlign,
-                                            Align PrefAlign,
-                                            uint32_t TypeBitWidth,
-                                            uint32_t IndexBitWidth) {
+Error DataLayout::setPointerSpec(uint32_t AddrSpace, uint32_t BitWidth,
+                                 Align ABIAlign, Align PrefAlign,
+                                 uint32_t IndexBitWidth) {
   if (PrefAlign < ABIAlign)
     return reportError(
         "Preferred alignment cannot be less than the ABI alignment");
-  if (IndexBitWidth > TypeBitWidth)
+  if (IndexBitWidth > BitWidth)
     return reportError("Index width cannot be larger than pointer width");
 
-  auto I = lower_bound(Pointers, AddrSpace,
-                       [](const PointerAlignElem &A, uint32_t AddressSpace) {
-    return A.AddressSpace < AddressSpace;
-  });
-  if (I == Pointers.end() || I->AddressSpace != AddrSpace) {
-    Pointers.insert(I, PointerAlignElem{AddrSpace, TypeBitWidth, ABIAlign,
-                                        PrefAlign, IndexBitWidth});
+  auto I = lower_bound(PointerSpecs, AddrSpace,
+                       [](const PointerSpec &A, uint32_t AddrSpace) {
+                         return A.AddrSpace < AddrSpace;
+                       });
+  if (I == PointerSpecs.end() || I->AddrSpace != AddrSpace) {
+    PointerSpecs.insert(I, PointerSpec{AddrSpace, BitWidth, ABIAlign, PrefAlign,
+                                       IndexBitWidth});
   } else {
+    I->BitWidth = BitWidth;
     I->ABIAlign = ABIAlign;
     I->PrefAlign = PrefAlign;
-    I->TypeBitWidth = TypeBitWidth;
     I->IndexBitWidth = IndexBitWidth;
   }
   return Error::success();
@@ -678,11 +679,11 @@ Error DataLayout::setPointerAlignmentInBits(uint32_t AddrSpace, Align ABIAlign,
 
 Align DataLayout::getIntegerAlignment(uint32_t BitWidth,
                                       bool abi_or_pref) const {
-  auto I = findAlignmentLowerBound(IntAlignments, BitWidth);
+  auto I = findPrimitiveSpecLowerBound(IntSpecs, BitWidth);
   // If we don't have an exact match, use alignment of next larger integer
   // type. If there is none, use alignment of largest integer type by going
   // back one element.
-  if (I == IntAlignments.end())
+  if (I == IntSpecs.end())
     --I;
   return abi_or_pref ? I->ABIAlign : I->PrefAlign;
 }
@@ -712,22 +713,22 @@ const StructLayout *DataLayout::getStructLayout(StructType *Ty) const {
 }
 
 Align DataLayout::getPointerABIAlignment(unsigned AS) const {
-  return getPointerAlignElem(AS).ABIAlign;
+  return getPointerSpec(AS).ABIAlign;
 }
 
 Align DataLayout::getPointerPrefAlignment(unsigned AS) const {
-  return getPointerAlignElem(AS).PrefAlign;
+  return getPointerSpec(AS).PrefAlign;
 }
 
 unsigned DataLayout::getPointerSize(unsigned AS) const {
-  return divideCeil(getPointerAlignElem(AS).TypeBitWidth, 8);
+  return divideCeil(getPointerSpec(AS).BitWidth, 8);
 }
 
 unsigned DataLayout::getMaxIndexSize() const {
   unsigned MaxIndexSize = 0;
-  for (auto &P : Pointers)
+  for (const PointerSpec &Spec : PointerSpecs)
     MaxIndexSize =
-        std::max(MaxIndexSize, (unsigned)divideCeil(P.TypeBitWidth, 8));
+        std::max(MaxIndexSize, (unsigned)divideCeil(Spec.BitWidth, 8));
 
   return MaxIndexSize;
 }
@@ -740,7 +741,7 @@ unsigned DataLayout::getPointerTypeSizeInBits(Type *Ty) const {
 }
 
 unsigned DataLayout::getIndexSize(unsigned AS) const {
-  return divideCeil(getPointerAlignElem(AS).IndexBitWidth, 8);
+  return divideCeil(getPointerSpec(AS).IndexBitWidth, 8);
 }
 
 unsigned DataLayout::getIndexTypeSizeInBits(Type *Ty) const {
@@ -794,8 +795,8 @@ Align DataLayout::getAlignment(Type *Ty, bool abi_or_pref) const {
   case Type::FP128TyID:
   case Type::X86_FP80TyID: {
     unsigned BitWidth = getTypeSizeInBits(Ty).getFixedValue();
-    auto I = findAlignmentLowerBound(FloatAlignments, BitWidth);
-    if (I != FloatAlignments.end() && I->TypeBitWidth == BitWidth)
+    auto I = findPrimitiveSpecLowerBound(FloatSpecs, BitWidth);
+    if (I != FloatSpecs.end() && I->BitWidth == BitWidth)
       return abi_or_pref ? I->ABIAlign : I->PrefAlign;
 
     // If we still couldn't find a reasonable default alignment, fall back
@@ -809,8 +810,8 @@ Align DataLayout::getAlignment(Type *Ty, bool abi_or_pref) const {
   case Type::FixedVectorTyID:
   case Type::ScalableVectorTyID: {
     unsigned BitWidth = getTypeSizeInBits(Ty).getKnownMinValue();
-    auto I = findAlignmentLowerBound(VectorAlignments, BitWidth);
-    if (I != VectorAlignments.end() && I->TypeBitWidth == BitWidth)
+    auto I = findPrimitiveSpecLowerBound(VectorSpecs, BitWidth);
+    if (I != VectorSpecs.end() && I->BitWidth == BitWidth)
       return abi_or_pref ? I->ABIAlign : I->PrefAlign;
 
     // By default, use natural alignment for vector types. This is consistent

>From 66818ea038f3b27c0a4a475486cecaf1b33fd957 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Wed, 14 Aug 2024 23:39:41 +0300
Subject: [PATCH 3/3] Address review comments

---
 llvm/include/llvm/IR/DataLayout.h | 27 ++++++++++----------
 llvm/lib/IR/DataLayout.cpp        | 41 ++++++++++++++-----------------
 2 files changed, 32 insertions(+), 36 deletions(-)

diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 4bf1c61c892ae9..1185939cd9c75b 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -115,27 +115,26 @@ class DataLayout {
   // FIXME: `unsigned char` truncates the value parsed by `parseSpecifier`.
   SmallVector<unsigned char, 8> LegalIntWidths;
 
-  // Primitive type specifier.
-  enum class PrimitiveSpecifier {
+  /// Type specifier used by some internal functions.
+  enum class TypeSpecifier {
     Integer = 'i',
     Float = 'f',
     Vector = 'v',
-    // TODO: Aggregates are not primitives. This should be separated.
     Aggregate = 'a'
   };
 
-  // Primitive type specifications. Sorted and uniqued by type bit width.
+  /// Primitive type specifications. Sorted and uniqued by type bit width.
   SmallVector<PrimitiveSpec, 6> IntSpecs;
   SmallVector<PrimitiveSpec, 4> FloatSpecs;
   SmallVector<PrimitiveSpec, 10> VectorSpecs;
 
-  // Pointer type specifications. Sorted and uniqued by address space number.
+  /// Pointer type specifications. Sorted and uniqued by address space number.
   SmallVector<PointerSpec, 8> PointerSpecs;
 
   /// The string representation used to create this DataLayout
   std::string StringRepresentation;
 
-  // Struct type ABI and preferred alignments. The default spec is "a:8:64".
+  /// Struct type ABI and preferred alignments. The default spec is "a:8:64".
   Align StructABIAlignment = Align::Constant<1>();
   Align StructPrefAlignment = Align::Constant<8>();
 
@@ -146,17 +145,17 @@ class DataLayout {
   /// well-defined bitwise representation.
   SmallVector<unsigned, 8> NonIntegralAddressSpaces;
 
-  // Attempts to set the specification for the given type.
-  // Returns an error description on failure.
-  Error setPrimitiveSpec(PrimitiveSpecifier Specifier, uint32_t BitWidth,
+  /// Attempts to set the specification for the given type.
+  /// Returns an error description on failure.
+  Error setPrimitiveSpec(TypeSpecifier Specifier, uint32_t BitWidth,
                          Align ABIAlign, Align PrefAlign);
 
-  // Searches for a pointer specification that matches the given address space.
-  // Returns the default address space specification if not found.
-  const PointerSpec &getPointerSpec(uint32_t Spec) const;
+  /// Searches for a pointer specification that matches the given address space.
+  /// Returns the default address space specification if not found.
+  const PointerSpec &getPointerSpec(uint32_t AddrSpace) const;
 
-  // Attempts to set the specification for pointer in the given address space.
-  // Returns an error description on failure.
+  /// Attempts to set the specification for pointer in the given address space.
+  /// Returns an error description on failure.
   Error setPointerSpec(uint32_t AddrSpace, uint32_t BitWidth, Align ABIAlign,
                        Align PrefAlign, uint32_t IndexBitWidth);
 
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 552da2de2d2e08..44cd1e69818953 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -326,10 +326,10 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
       continue;
     }
 
-    char Specifier = Tok.front();
+    char SpecifierChar = Tok.front();
     Tok = Tok.substr(1);
 
-    switch (Specifier) {
+    switch (SpecifierChar) {
     case 's':
       // Deprecated, but ignoring here to preserve loading older textual llvm
       // ASM file
@@ -408,21 +408,21 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
     case 'v':
     case 'f':
     case 'a': {
-      PrimitiveSpecifier PrimSpecifier;
-      switch (Specifier) {
+      TypeSpecifier Specifier;
+      switch (SpecifierChar) {
       default:
         llvm_unreachable("Unexpected specifier!");
       case 'i':
-        PrimSpecifier = PrimitiveSpecifier::Integer;
+        Specifier = TypeSpecifier::Integer;
         break;
       case 'v':
-        PrimSpecifier = PrimitiveSpecifier::Vector;
+        Specifier = TypeSpecifier::Vector;
         break;
       case 'f':
-        PrimSpecifier = PrimitiveSpecifier::Float;
+        Specifier = TypeSpecifier::Float;
         break;
       case 'a':
-        PrimSpecifier = PrimitiveSpecifier::Aggregate;
+        Specifier = TypeSpecifier::Aggregate;
         break;
       }
 
@@ -432,7 +432,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
         if (Error Err = getInt(Tok, Size))
           return Err;
 
-      if (PrimSpecifier == PrimitiveSpecifier::Aggregate && Size != 0)
+      if (Specifier == TypeSpecifier::Aggregate && Size != 0)
         return reportError(
             "Sized aggregate specification in datalayout string");
 
@@ -445,7 +445,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
       unsigned ABIAlign;
       if (Error Err = getIntInBytes(Tok, ABIAlign))
         return Err;
-      if (PrimSpecifier != PrimitiveSpecifier::Aggregate && !ABIAlign)
+      if (Specifier != TypeSpecifier::Aggregate && !ABIAlign)
         return reportError(
             "ABI alignment specification must be >0 for non-aggregate types");
 
@@ -453,8 +453,7 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
         return reportError("Invalid ABI alignment, must be a 16bit integer");
       if (ABIAlign != 0 && !isPowerOf2_64(ABIAlign))
         return reportError("Invalid ABI alignment, must be a power of 2");
-      if (PrimSpecifier == PrimitiveSpecifier::Integer && Size == 8 &&
-          ABIAlign != 1)
+      if (Specifier == TypeSpecifier::Integer && Size == 8 && ABIAlign != 1)
         return reportError(
             "Invalid ABI alignment, i8 must be naturally aligned");
 
@@ -473,9 +472,8 @@ Error DataLayout::parseSpecifier(StringRef Desc) {
       if (PrefAlign != 0 && !isPowerOf2_64(PrefAlign))
         return reportError("Invalid preferred alignment, must be a power of 2");
 
-      if (Error Err =
-              setPrimitiveSpec(PrimSpecifier, Size, assumeAligned(ABIAlign),
-                               assumeAligned(PrefAlign)))
+      if (Error Err = setPrimitiveSpec(Specifier, Size, assumeAligned(ABIAlign),
+                                       assumeAligned(PrefAlign)))
         return Err;
 
       break;
@@ -592,9 +590,8 @@ findPrimitiveSpecLowerBound(
   });
 }
 
-Error DataLayout::setPrimitiveSpec(PrimitiveSpecifier Specifier,
-                                   uint32_t BitWidth, Align ABIAlign,
-                                   Align PrefAlign) {
+Error DataLayout::setPrimitiveSpec(TypeSpecifier Specifier, uint32_t BitWidth,
+                                   Align ABIAlign, Align PrefAlign) {
   // 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
@@ -608,17 +605,17 @@ Error DataLayout::setPrimitiveSpec(PrimitiveSpecifier Specifier,
 
   SmallVectorImpl<PrimitiveSpec> *Specs;
   switch (Specifier) {
-  case PrimitiveSpecifier::Aggregate:
+  case TypeSpecifier::Aggregate:
     StructABIAlignment = ABIAlign;
     StructPrefAlignment = PrefAlign;
     return Error::success();
-  case PrimitiveSpecifier::Integer:
+  case TypeSpecifier::Integer:
     Specs = &IntSpecs;
     break;
-  case PrimitiveSpecifier::Float:
+  case TypeSpecifier::Float:
     Specs = &FloatSpecs;
     break;
-  case PrimitiveSpecifier::Vector:
+  case TypeSpecifier::Vector:
     Specs = &VectorSpecs;
     break;
   }



More information about the llvm-commits mailing list