[llvm] [DataLayout] Refactor parsing of "p" specification (PR #104583)

Sergei Barannikov via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 16 12:43:44 PDT 2024


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

>From cc9c3e5ae49efa95d63ee5934272f583a7b03fee Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 16 Aug 2024 09:54:32 +0300
Subject: [PATCH 1/3] [DataLayout] Refactor parsing of "p" specification

Split off of #104545 to reduce patch size.
Similar to #104546, this introduces `parseSize` and `parseAlignment`,
which are improved versions of `getInt` tailored for specific needs.

I'm not a GTest professional, so the tests are very straightforward.
---
 llvm/include/llvm/IR/DataLayout.h    |  12 +-
 llvm/lib/IR/DataLayout.cpp           | 180 +++++++++++++---------
 llvm/test/Assembler/getInt.ll        |   3 -
 llvm/unittests/IR/DataLayoutTest.cpp | 217 +++++++++++++++++++++++----
 4 files changed, 302 insertions(+), 110 deletions(-)
 delete mode 100644 llvm/test/Assembler/getInt.ll

diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 228b723ee663ff..85dae10883c67c 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -154,10 +154,9 @@ class DataLayout {
   /// 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.
-  Error setPointerSpec(uint32_t AddrSpace, uint32_t BitWidth, Align ABIAlign,
-                       Align PrefAlign, uint32_t IndexBitWidth);
+  /// Sets or updates the specification for pointer in the given address space.
+  void 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;
@@ -165,8 +164,11 @@ class DataLayout {
   /// Internal helper method that returns requested alignment for type.
   Align getAlignment(Type *Ty, bool abi_or_pref) const;
 
+  /// Attempts to parse a pointer specification ('p').
+  Error parsePointerSpec(StringRef Spec);
+
   /// Attempts to parse a single specification.
-  Error parseSpecification(StringRef Specification);
+  Error parseSpecification(StringRef Spec);
 
   /// Attempts to parse a data layout string.
   Error parseLayoutString(StringRef LayoutString);
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 7043fa410192a3..68e71214b1c9ae 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -288,6 +288,54 @@ static Error parseAddrSpace(StringRef Str, unsigned &AddrSpace) {
   return Error::success();
 }
 
+/// Attempts to parse a size component of a specification.
+static Error parseSize(StringRef Str, unsigned &BitWidth,
+                       StringRef Name = "size") {
+  if (Str.empty())
+    return createStringError(Name + " component cannot be empty");
+
+  if (!to_integer(Str, BitWidth, 10) || BitWidth == 0 || !isUInt<24>(BitWidth))
+    return createStringError(Name + " must be a non-zero 24-bit integer");
+
+  return Error::success();
+}
+
+/// Attempts to parse an alignment component of a specification.
+///
+/// On success, returns the value converted to byte amount in \p Alignment.
+/// If the value is zero and \p AllowZero is true, \p Alignment is set to one.
+///
+/// Return an error in a number of cases:
+/// - \p Str is empty or contains characters other than decimal digits;
+/// - the value is zero and \p AllowZero is false;
+/// - the value is too large;
+/// - the value is not a multiple of the byte width;
+/// - the value converted to byte amount is not not a power of two.
+static Error parseAlignment(StringRef Str, Align &Alignment, StringRef Name,
+                            bool AllowZero = false) {
+  if (Str.empty())
+    return createStringError(Name + " alignment component cannot be empty");
+
+  unsigned Value;
+  if (!to_integer(Str, Value, 10) || !isUInt<16>(Value))
+    return createStringError(Name + " alignment must be a 16-bit integer");
+
+  if (Value == 0) {
+    if (!AllowZero)
+      return createStringError(Name + " alignment must be non-zero");
+    Alignment = Align(1);
+    return Error::success();
+  }
+
+  constexpr unsigned ByteWidth = 8;
+  if (Value % ByteWidth || !isPowerOf2_32(Value / ByteWidth))
+    return createStringError(
+        Name + " alignment must be a power of two times the byte width");
+
+  Alignment = Align(Value / ByteWidth);
+  return Error::success();
+}
+
 /// Checked version of split, to ensure mandatory subparts.
 static Error split(StringRef Str, char Separator,
                    std::pair<StringRef, StringRef> &Split) {
@@ -328,6 +376,56 @@ static Error getAddrSpace(StringRef R, unsigned &AddrSpace) {
   return Error::success();
 }
 
+Error DataLayout::parsePointerSpec(StringRef Spec) {
+  // p[<n>]:<size>:<abi>[:<pref>[:<idx>]]
+  SmallVector<StringRef, 5> Components;
+  assert(Spec.front() == 'p');
+  Spec.drop_front().split(Components, ':');
+
+  if (Components.size() < 3 || Components.size() > 5)
+    return createSpecFormatError("p[<n>]:<size>:<abi>[:<pref>[:<idx>]]");
+
+  // Address space. Optional, defaults to 0.
+  unsigned AddrSpace = 0;
+  if (!Components[0].empty())
+    if (Error Err = parseAddrSpace(Components[0], AddrSpace))
+      return Err;
+
+  // Size. Required, cannot be zero.
+  unsigned BitWidth;
+  if (Error Err = parseSize(Components[1], BitWidth, "pointer size"))
+    return Err;
+
+  // ABI alignment. Required, cannot be zero.
+  Align ABIAlign;
+  if (Error Err = parseAlignment(Components[2], ABIAlign, "ABI"))
+    return Err;
+
+  // Preferred alignment. Optional, defaults to the ABI alignment.
+  // Cannot be zero.
+  Align PrefAlign = ABIAlign;
+  if (Components.size() > 3)
+    if (Error Err = parseAlignment(Components[3], PrefAlign, "preferred"))
+      return Err;
+
+  if (PrefAlign < ABIAlign)
+    return createStringError(
+        "preferred alignment cannot be less than the ABI alignment");
+
+  // Index size. Optional, defaults to pointer size. Cannot be zero.
+  unsigned IndexBitWidth = BitWidth;
+  if (Components.size() > 4)
+    if (Error Err = parseSize(Components[4], IndexBitWidth, "index size"))
+      return Err;
+
+  if (IndexBitWidth > BitWidth)
+    return createStringError(
+        "index size cannot be larger than the pointer size");
+
+  setPointerSpec(AddrSpace, BitWidth, ABIAlign, PrefAlign, IndexBitWidth);
+  return Error::success();
+}
+
 Error DataLayout::parseSpecification(StringRef Spec) {
   // The "ni" specifier is the only two-character specifier. Handle it first.
   if (Spec.starts_with("ni")) {
@@ -349,6 +447,12 @@ Error DataLayout::parseSpecification(StringRef Spec) {
     return Error::success();
   }
 
+  // The rest of the specifiers are single-character.
+  assert(!Spec.empty() && "Empty specification is handled by the caller");
+
+  if (Spec.front() == 'p')
+    return parsePointerSpec(Spec);
+
   // Split at ':'.
   std::pair<StringRef, StringRef> Split;
   if (Error Err = ::split(Spec, ':', Split))
@@ -372,69 +476,6 @@ Error DataLayout::parseSpecification(StringRef Spec) {
   case 'e':
     BigEndian = false;
     break;
-  case 'p': {
-    // Address space.
-    unsigned AddrSpace = 0;
-    if (!Tok.empty())
-      if (Error Err = getInt(Tok, AddrSpace))
-        return Err;
-    if (!isUInt<24>(AddrSpace))
-      return reportError("Invalid address space, must be a 24-bit integer");
-
-    // Size.
-    if (Rest.empty())
-      return reportError(
-          "Missing size specification for pointer in datalayout string");
-    if (Error Err = ::split(Rest, ':', Split))
-      return Err;
-    unsigned PointerMemSize;
-    if (Error Err = getInt(Tok, PointerMemSize))
-      return Err;
-    if (!PointerMemSize)
-      return reportError("Invalid pointer size of 0 bytes");
-
-    // ABI alignment.
-    if (Rest.empty())
-      return reportError(
-          "Missing alignment specification for pointer in datalayout string");
-    if (Error Err = ::split(Rest, ':', Split))
-      return Err;
-    unsigned PointerABIAlign;
-    if (Error Err = getIntInBytes(Tok, PointerABIAlign))
-      return Err;
-    if (!isPowerOf2_64(PointerABIAlign))
-      return reportError("Pointer ABI alignment must be a power of 2");
-
-    // Size of index used in GEP for address calculation.
-    // The parameter is optional. By default it is equal to size of pointer.
-    unsigned IndexSize = PointerMemSize;
-
-    // Preferred alignment.
-    unsigned PointerPrefAlign = PointerABIAlign;
-    if (!Rest.empty()) {
-      if (Error Err = ::split(Rest, ':', Split))
-        return Err;
-      if (Error Err = getIntInBytes(Tok, PointerPrefAlign))
-        return Err;
-      if (!isPowerOf2_64(PointerPrefAlign))
-        return reportError("Pointer preferred alignment must be a power of 2");
-
-      // Now read the index. It is the second optional parameter here.
-      if (!Rest.empty()) {
-        if (Error Err = ::split(Rest, ':', Split))
-          return Err;
-        if (Error Err = getInt(Tok, IndexSize))
-          return Err;
-        if (!IndexSize)
-          return reportError("Invalid index size of 0 bytes");
-      }
-    }
-    if (Error Err = setPointerSpec(AddrSpace, PointerMemSize,
-                                   assumeAligned(PointerABIAlign),
-                                   assumeAligned(PointerPrefAlign), IndexSize))
-      return Err;
-    break;
-  }
   case 'i':
   case 'v':
   case 'f':
@@ -680,15 +721,9 @@ DataLayout::getPointerSpec(uint32_t AddrSpace) const {
   return PointerSpecs[0];
 }
 
-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 > BitWidth)
-    return reportError("Index width cannot be larger than pointer width");
-
+void DataLayout::setPointerSpec(uint32_t AddrSpace, uint32_t BitWidth,
+                                Align ABIAlign, Align PrefAlign,
+                                uint32_t IndexBitWidth) {
   auto I = lower_bound(PointerSpecs, AddrSpace, LessPointerAddrSpace());
   if (I == PointerSpecs.end() || I->AddrSpace != AddrSpace) {
     PointerSpecs.insert(I, PointerSpec{AddrSpace, BitWidth, ABIAlign, PrefAlign,
@@ -699,7 +734,6 @@ Error DataLayout::setPointerSpec(uint32_t AddrSpace, uint32_t BitWidth,
     I->PrefAlign = PrefAlign;
     I->IndexBitWidth = IndexBitWidth;
   }
-  return Error::success();
 }
 
 Align DataLayout::getIntegerAlignment(uint32_t BitWidth,
diff --git a/llvm/test/Assembler/getInt.ll b/llvm/test/Assembler/getInt.ll
deleted file mode 100644
index 8e2537ae6cf1d8..00000000000000
--- a/llvm/test/Assembler/getInt.ll
+++ /dev/null
@@ -1,3 +0,0 @@
-; RUN: not opt < %s 2>&1 | grep 'not a number, or does not fit in an unsigned int'
-
-target datalayout = "p:4294967296:64:64"
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index a40ac5b83a83e4..0806bd572c0237 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -30,26 +30,12 @@ TEST(DataLayoutTest, ParseErrors) {
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("n0"),
       FailedWithMessage("Zero width native integer type in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("p16777216:64:64:64"),
-      FailedWithMessage("Invalid address space, must be a 24-bit integer"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("a1:64"),
       FailedWithMessage("Sized aggregate specification in datalayout string"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("a:"),
       FailedWithMessage("Trailing separator in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("p:48:52"),
-      FailedWithMessage("number of bits must be a byte width multiple"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("e-p"),
-      FailedWithMessage(
-          "Missing size specification for pointer in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("e-p:64"),
-      FailedWithMessage(
-          "Missing alignment specification for pointer in datalayout string"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("m"),
       FailedWithMessage("Expected mangling specifier in datalayout string"));
@@ -79,21 +65,6 @@ TEST(DataLayoutTest, ParseErrors) {
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("i16777216:16:16"),
       FailedWithMessage("Invalid bit width, must be a 24-bit integer"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("p:32:32:16"),
-      FailedWithMessage(
-          "Preferred alignment cannot be less than the ABI alignment"));
-  EXPECT_THAT_EXPECTED(DataLayout::parse("p:0:32:32"),
-                       FailedWithMessage("Invalid pointer size of 0 bytes"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("p:64:24:64"),
-      FailedWithMessage("Pointer ABI alignment must be a power of 2"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("p:64:64:24"),
-      FailedWithMessage("Pointer preferred alignment must be a power of 2"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("p:64:64:64:128"),
-      FailedWithMessage("Index width cannot be larger than pointer width"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("v128:0:128"),
       FailedWithMessage(
@@ -134,6 +105,98 @@ TEST(DataLayout, LayoutStringFormat) {
         FailedWithMessage("empty specification is not allowed"));
 }
 
+TEST(DataLayout, ParsePointerSpec) {
+  for (StringRef Str :
+       {"p:16:8", "p:16:16:64", "p:32:64:64:32", "p0:32:64", "p42:64:32:32",
+        "p16777215:32:32:64:8", "p16777215:16777215:32768:32768:16777215"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
+  for (StringRef Str :
+       {"p", "p0", "p:32", "p0:32", "p:32:32:32:32:32", "p0:32:32:32:32:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("malformed specification, must be of the form "
+                          "\"p[<n>]:<size>:<abi>[:<pref>[:<idx>]]\""));
+
+  // address space
+  for (StringRef Str : {"p0x0:32:32", "px:32:32:32", "p16777216:32:32:32:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("address space must be a 24-bit integer"));
+
+  // pointer size
+  for (StringRef Str : {"p::32", "p0::32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("pointer size component cannot be empty"));
+
+  for (StringRef Str : {"p:0:32", "p0:0x1:32:32", "p42:16777216:32:32:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("pointer size must be a non-zero 24-bit integer"));
+
+  // ABI alignment
+  for (StringRef Str : {"p:32:", "p0:32::32", "p42:32::32:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("ABI alignment component cannot be empty"));
+
+  for (StringRef Str : {"p:32:x", "p0:32:0x20:32", "p42:32:65536:32:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("ABI alignment must be a 16-bit integer"));
+
+  for (StringRef Str : {"p:32:0", "p0:32:0:32", "p42:32:0:32:32"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str),
+                         FailedWithMessage("ABI alignment must be non-zero"));
+
+  for (StringRef Str : {"p:32:4", "p42:32:24:32", "p0:32:65535:32:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage(
+            "ABI alignment must be a power of two times the byte width"));
+
+  // preferred alignment
+  for (StringRef Str : {"p:32:32:", "p0:32:32:", "p42:32:32::32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("preferred alignment component cannot be empty"));
+
+  for (StringRef Str : {"p:32:32:x", "p0:32:32:0x20", "p42:32:32:65536:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("preferred alignment must be a 16-bit integer"));
+
+  for (StringRef Str : {"p:32:32:0", "p0:32:32:0", "p42:32:32:0:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("preferred alignment must be non-zero"));
+
+  for (StringRef Str : {"p:32:32:4", "p42:32:32:24", "p0:32:32:65535:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage(
+            "preferred alignment must be a power of two times the byte width"));
+  // TODO: Check PrefAlign >= ABIAlign.
+
+  // index size
+  for (StringRef Str : {"p:32:32:32:", "p0:32:32:32:"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("index size component cannot be empty"));
+
+  for (StringRef Str :
+       {"p:32:32:32:0", "p0:32:32:32:0x20", "p42:32:32:32:16777216"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("index size must be a non-zero 24-bit integer"));
+
+  for (StringRef Str : {"p:16:16:16:17", "p0:32:64:64:64", "p42:16:64:64:32"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("index size cannot be larger than the pointer size"));
+}
+
 TEST(DataLayout, ParseNonIntegralAddrSpace) {
   for (StringRef Str : {"ni:1", "ni:16777215", "ni:1:16777215"})
     EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
@@ -160,6 +223,102 @@ TEST(DataLayout, ParseNonIntegralAddrSpace) {
         FailedWithMessage("address space 0 cannot be non-integral"));
 }
 
+TEST(DataLayout, GetPointerSizeInBits) {
+  std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
+      {"", 64, 64, 64},
+      {"p:16:32", 16, 16, 16},
+      {"p0:32:64", 32, 32, 32},
+      {"p1:16:32", 64, 16, 64},
+      {"p1:31:32-p2:15:16:16:14", 64, 31, 15},
+  };
+  for (auto [Layout, V0, V1, V2] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getPointerSizeInBits(0), V0) << Layout;
+    EXPECT_EQ(DL.getPointerSizeInBits(1), V1) << Layout;
+    EXPECT_EQ(DL.getPointerSizeInBits(2), V2) << Layout;
+  }
+}
+
+TEST(DataLayout, GetPointerSize) {
+  std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
+      {"", 8, 8, 8},
+      {"p:16:32", 2, 2, 2},
+      {"p0:32:64", 4, 4, 4},
+      {"p1:17:32", 8, 3, 8},
+      {"p1:31:64-p2:23:8:16:9", 8, 4, 3},
+  };
+  for (auto [Layout, V0, V1, V2] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getPointerSize(0), V0) << Layout;
+    EXPECT_EQ(DL.getPointerSize(1), V1) << Layout;
+    EXPECT_EQ(DL.getPointerSize(2), V2) << Layout;
+  }
+}
+
+TEST(DataLayout, GetIndexSizeInBits) {
+  std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
+      {"", 64, 64, 64},
+      {"p:16:32", 16, 16, 16},
+      {"p0:32:64", 32, 32, 32},
+      {"p1:16:32:32:10", 64, 10, 64},
+      {"p1:31:32:64:20-p2:17:16:16:15", 64, 20, 15},
+  };
+  for (auto [Layout, V0, V1, V2] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getIndexSizeInBits(0), V0) << Layout;
+    EXPECT_EQ(DL.getIndexSizeInBits(1), V1) << Layout;
+    EXPECT_EQ(DL.getIndexSizeInBits(2), V2) << Layout;
+  }
+}
+
+TEST(DataLayout, GetIndexSize) {
+  std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
+      {"", 8, 8, 8},
+      {"p:16:32", 2, 2, 2},
+      {"p0:27:64", 4, 4, 4},
+      {"p1:19:32:64:5", 8, 1, 8},
+      {"p1:33:32:64:23-p2:21:8:16:13", 8, 3, 2},
+  };
+  for (auto [Layout, V0, V1, V2] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getIndexSize(0), V0) << Layout;
+    EXPECT_EQ(DL.getIndexSize(1), V1) << Layout;
+    EXPECT_EQ(DL.getIndexSize(2), V2) << Layout;
+  }
+}
+
+TEST(DataLayout, GetPointerABIAlignment) {
+  std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
+      {"", 8, 8, 8},
+      {"p:16:32", 4, 4, 4},
+      {"p0:16:32:64", 4, 4, 4},
+      {"p1:32:16:64", 8, 2, 8},
+      {"p1:33:16:32:15-p2:23:8:16:9", 8, 2, 1},
+  };
+  for (auto [Layout, V0, V1, V2] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getPointerABIAlignment(0).value(), V0) << Layout;
+    EXPECT_EQ(DL.getPointerABIAlignment(1).value(), V1) << Layout;
+    EXPECT_EQ(DL.getPointerABIAlignment(2).value(), V2) << Layout;
+  }
+}
+
+TEST(DataLayout, GetPointerPrefAlignment) {
+  std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
+      {"", 8, 8, 8},
+      {"p:16:32", 4, 4, 4},
+      {"p0:8:16:32", 4, 4, 4},
+      {"p1:32:8:16", 8, 2, 8},
+      {"p1:33:8:16:31-p2:23:8:32:17", 8, 2, 4},
+  };
+  for (auto [Layout, V0, V1, V2] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getPointerPrefAlignment(0).value(), V0) << Layout;
+    EXPECT_EQ(DL.getPointerPrefAlignment(1).value(), V1) << Layout;
+    EXPECT_EQ(DL.getPointerPrefAlignment(2).value(), V2) << Layout;
+  }
+}
+
 TEST(DataLayout, IsNonIntegralAddressSpace) {
   DataLayout Default;
   EXPECT_THAT(Default.getNonIntegralAddressSpaces(), ::testing::SizeIs(0));

>From 33487f6e61453045318ba95d09502c8e7818dbd1 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 16 Aug 2024 21:32:53 +0300
Subject: [PATCH 2/3] Add missing test

---
 llvm/unittests/IR/DataLayoutTest.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 0806bd572c0237..4b236b2d165f9c 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -172,12 +172,17 @@ TEST(DataLayout, ParsePointerSpec) {
         DataLayout::parse(Str),
         FailedWithMessage("preferred alignment must be non-zero"));
 
-  for (StringRef Str : {"p:32:32:4", "p42:32:32:24", "p0:32:32:65535:32"})
+  for (StringRef Str : {"p:32:32:4", "p0:32:32:24", "p42:32:32:65535:32"})
     EXPECT_THAT_EXPECTED(
         DataLayout::parse(Str),
         FailedWithMessage(
             "preferred alignment must be a power of two times the byte width"));
-  // TODO: Check PrefAlign >= ABIAlign.
+
+  for (StringRef Str : {"p:64:64:32", "p0:16:32:16:16"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage(
+            "preferred alignment cannot be less than the ABI alignment"));
 
   // index size
   for (StringRef Str : {"p:32:32:32:", "p0:32:32:32:"})

>From 157f5c656d3dc45a7139773766d179ad0af7ea39 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 16 Aug 2024 22:41:31 +0300
Subject: [PATCH 3/3] Minor tweak to make diff with the draft smaller

---
 llvm/lib/IR/DataLayout.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 68e71214b1c9ae..425a9024c897d4 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -449,8 +449,9 @@ Error DataLayout::parseSpecification(StringRef Spec) {
 
   // The rest of the specifiers are single-character.
   assert(!Spec.empty() && "Empty specification is handled by the caller");
+  char Specifier = Spec.front();
 
-  if (Spec.front() == 'p')
+  if (Specifier == 'p')
     return parsePointerSpec(Spec);
 
   // Split at ':'.



More information about the llvm-commits mailing list