[llvm] [DataLayout] Refactor `parseSpecification` (PR #104545)

Sergei Barannikov via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 16 13:57:07 PDT 2024


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

>From 13983f4f40c413bcaf371f95fb710b317de69782 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 16 Aug 2024 23:49:01 +0300
Subject: [PATCH 1/2] [DataLayout] Refactor parsing of "p" specification
 (#104583)

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 guru, so the tests are not ideal.
---
 llvm/include/llvm/IR/DataLayout.h    |  12 +-
 llvm/lib/IR/DataLayout.cpp           | 181 +++++++++++++---------
 llvm/test/Assembler/getInt.ll        |   3 -
 llvm/unittests/IR/DataLayoutTest.cpp | 222 +++++++++++++++++++++++----
 4 files changed, 308 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..425a9024c897d4 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,13 @@ 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");
+  char Specifier = Spec.front();
+
+  if (Specifier == 'p')
+    return parsePointerSpec(Spec);
+
   // Split at ':'.
   std::pair<StringRef, StringRef> Split;
   if (Error Err = ::split(Spec, ':', Split))
@@ -372,69 +477,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 +722,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 +735,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..4b236b2d165f9c 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,103 @@ 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", "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"));
+
+  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:"})
+    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 +228,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 e554f3dd06c655ca01c46eb9b9dde40937c418de Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Thu, 15 Aug 2024 18:30:16 +0300
Subject: [PATCH 2/2] [DataLayout] Refactor `parseSpecification`

---
 llvm/include/llvm/IR/DataLayout.h             |  23 +-
 llvm/lib/IR/DataLayout.cpp                    | 295 +++++++-----------
 .../Assembler/invalid-datalayout-override.ll  |   2 +-
 .../function-default-address-spaces.ll        |   2 +-
 .../test/Bitcode/invalid-functionptr-align.ll |   5 -
 ...ptr-align.ll.bc => invalid-stack-align.bc} | Bin
 llvm/test/Bitcode/invalid-stack-align.ll.bc   |   5 +
 llvm/test/Transforms/InstCombine/crash.ll     |   2 +-
 llvm/test/Transforms/InstCombine/phi.ll       |   2 +-
 llvm/unittests/IR/DataLayoutTest.cpp          |  77 ++---
 10 files changed, 168 insertions(+), 245 deletions(-)
 delete mode 100644 llvm/test/Bitcode/invalid-functionptr-align.ll
 rename llvm/test/Bitcode/{invalid-functionptr-align.ll.bc => invalid-stack-align.bc} (100%)
 create mode 100644 llvm/test/Bitcode/invalid-stack-align.ll.bc

diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 85dae10883c67c..2f06bda6c30a51 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -115,14 +115,6 @@ class DataLayout {
   // FIXME: `unsigned char` truncates the value parsed by `parseSpecifier`.
   SmallVector<unsigned char, 8> LegalIntWidths;
 
-  /// Type specifier used by some internal functions.
-  enum class TypeSpecifier {
-    Integer = 'i',
-    Float = 'f',
-    Vector = 'v',
-    Aggregate = 'a'
-  };
-
   /// Primitive type specifications. Sorted and uniqued by type bit width.
   SmallVector<PrimitiveSpec, 6> IntSpecs;
   SmallVector<PrimitiveSpec, 4> FloatSpecs;
@@ -145,10 +137,9 @@ 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(TypeSpecifier Specifier, uint32_t BitWidth,
-                         Align ABIAlign, Align PrefAlign);
+  /// Sets or updates the specification for the given primitive type.
+  void setPrimitiveSpec(char 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.
@@ -164,7 +155,13 @@ 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').
+  /// Attempts to parse primitive specification ('i', 'f', or 'v').
+  Error parsePrimitiveSpec(StringRef Spec);
+
+  /// Attempts to parse aggregate specification ('a').
+  Error parseAggregateSpec(StringRef Spec);
+
+  /// Attempts to parse pointer specification ('p').
   Error parsePointerSpec(StringRef Spec);
 
   /// Attempts to parse a single specification.
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 425a9024c897d4..1a9613b6d5078f 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -268,10 +268,6 @@ Expected<DataLayout> DataLayout::parse(StringRef LayoutString) {
   return Layout;
 }
 
-static Error reportError(const Twine &Message) {
-  return createStringError(inconvertibleErrorCode(), Message);
-}
-
 static Error createSpecFormatError(Twine Format) {
   return createStringError("malformed specification, must be of the form \"" +
                            Format + "\"");
@@ -336,43 +332,81 @@ static Error parseAlignment(StringRef Str, Align &Alignment, StringRef Name,
   return Error::success();
 }
 
-/// Checked version of split, to ensure mandatory subparts.
-static Error split(StringRef Str, char Separator,
-                   std::pair<StringRef, StringRef> &Split) {
-  assert(!Str.empty() && "parse error, string can't be empty here");
-  Split = Str.split(Separator);
-  if (Split.second.empty() && Split.first != Str)
-    return reportError("Trailing separator in datalayout string");
-  if (!Split.second.empty() && Split.first.empty())
-    return reportError("Expected token before separator in datalayout string");
-  return Error::success();
-}
+Error DataLayout::parsePrimitiveSpec(StringRef Spec) {
+  // [ifv]<size>:<abi>[:<pref>]
+  SmallVector<StringRef, 3> Components;
+  char Specifier = Spec.front();
+  assert(Specifier == 'i' || Specifier == 'f' || Specifier == 'v');
+  Spec.drop_front().split(Components, ':');
 
-/// Get an unsigned integer, including error checks.
-template <typename IntTy> static Error getInt(StringRef R, IntTy &Result) {
-  bool error = R.getAsInteger(10, Result); (void)error;
-  if (error)
-    return reportError("not a number, or does not fit in an unsigned int");
-  return Error::success();
-}
+  if (Components.size() < 2 || Components.size() > 3)
+    return createSpecFormatError(Twine(Specifier) + "<size>:<abi>[:<pref>]");
 
-/// Get an unsigned integer representing the number of bits and convert it into
-/// bytes. Error out of not a byte width multiple.
-template <typename IntTy>
-static Error getIntInBytes(StringRef R, IntTy &Result) {
-  if (Error Err = getInt<IntTy>(R, Result))
+  // Size. Required, cannot be zero.
+  unsigned BitWidth;
+  if (Error Err = parseSize(Components[0], BitWidth))
     return Err;
-  if (Result % 8)
-    return reportError("number of bits must be a byte width multiple");
-  Result /= 8;
+
+  // ABI alignment. Required, cannot be zero.
+  Align ABIAlign;
+  if (Error Err = parseAlignment(Components[1], ABIAlign, "ABI"))
+    return Err;
+
+  if (Specifier == 'i' && BitWidth == 8 && ABIAlign != 1)
+    return createStringError("i8 must be 8-bit aligned");
+
+  // Preferred alignment. Optional, defaults to the ABI alignment.
+  // Can be zero, meaning use ABI alignment.
+  Align PrefAlign = ABIAlign;
+  if (Components.size() > 2)
+    if (Error Err = parseAlignment(Components[2], PrefAlign, "preferred",
+                                   /*AllowZero=*/true))
+      return Err;
+
+  if (PrefAlign < ABIAlign)
+    return createStringError(
+        "preferred alignment cannot be less than the ABI alignment");
+
+  setPrimitiveSpec(Specifier, BitWidth, ABIAlign, PrefAlign);
   return Error::success();
 }
 
-static Error getAddrSpace(StringRef R, unsigned &AddrSpace) {
-  if (Error Err = getInt(R, AddrSpace))
+Error DataLayout::parseAggregateSpec(StringRef Spec) {
+  // a<size>:<abi>[:<pref>]
+  SmallVector<StringRef, 3> Components;
+  assert(Spec.front() == 'a');
+  Spec.drop_front().split(Components, ':');
+
+  if (Components.size() < 2 || Components.size() > 3)
+    return createSpecFormatError("a:<abi>[:<pref>]");
+
+  // According to LangRef, <size> component must be absent altogether.
+  // For backward compatibility, allow it to be specified, but require
+  // it to be literal "0".
+  if (!Components[0].empty() && Components[0] != "0")
+    return createStringError("size component must be empty");
+
+  // ABI alignment. Required.
+  // Can be zero, meaning use one byte alignment.
+  Align ABIAlign;
+  if (Error Err =
+          parseAlignment(Components[1], ABIAlign, "ABI", /*AllowZero=*/true))
     return Err;
-  if (!isUInt<24>(AddrSpace))
-    return reportError("Invalid address space, must be a 24-bit integer");
+
+  // Preferred alignment. Optional, defaults to the ABI alignment.
+  // Can be zero, meaning use ABI alignment.
+  Align PrefAlign = ABIAlign;
+  if (Components.size() > 2)
+    if (Error Err = parseAlignment(Components[2], PrefAlign, "preferred",
+                                   /*AllowZero=*/true))
+      return Err;
+
+  if (PrefAlign < ABIAlign)
+    return createStringError(
+        "preferred alignment cannot be less than the ABI alignment");
+
+  StructABIAlignment = ABIAlign;
+  StructPrefAlignment = PrefAlign;
   return Error::success();
 }
 
@@ -451,130 +485,51 @@ Error DataLayout::parseSpecification(StringRef Spec) {
   assert(!Spec.empty() && "Empty specification is handled by the caller");
   char Specifier = Spec.front();
 
-  if (Specifier == 'p')
-    return parsePointerSpec(Spec);
-
-  // Split at ':'.
-  std::pair<StringRef, StringRef> Split;
-  if (Error Err = ::split(Spec, ':', Split))
-    return Err;
+  if (Specifier == 'i' || Specifier == 'f' || Specifier == 'v')
+    return parsePrimitiveSpec(Spec);
 
-  // Aliases used below.
-  StringRef &Tok = Split.first;   // Current token.
-  StringRef &Rest = Split.second; // The rest of the string.
+  if (Specifier == 'a')
+    return parseAggregateSpec(Spec);
 
-  char SpecifierChar = Tok.front();
-  Tok = Tok.substr(1);
+  if (Specifier == 'p')
+    return parsePointerSpec(Spec);
 
-  switch (SpecifierChar) {
+  StringRef Rest = Spec.drop_front();
+  switch (Specifier) {
   case 's':
     // Deprecated, but ignoring here to preserve loading older textual llvm
     // ASM file
     break;
-  case 'E':
-    BigEndian = true;
-    break;
   case 'e':
-    BigEndian = false;
-    break;
-  case 'i':
-  case 'v':
-  case 'f':
-  case 'a': {
-    TypeSpecifier Specifier;
-    switch (SpecifierChar) {
-    default:
-      llvm_unreachable("Unexpected specifier!");
-    case 'i':
-      Specifier = TypeSpecifier::Integer;
-      break;
-    case 'v':
-      Specifier = TypeSpecifier::Vector;
-      break;
-    case 'f':
-      Specifier = TypeSpecifier::Float;
-      break;
-    case 'a':
-      Specifier = TypeSpecifier::Aggregate;
-      break;
-    }
-
-    // Bit size.
-    unsigned Size = 0;
-    if (!Tok.empty())
-      if (Error Err = getInt(Tok, Size))
-        return Err;
-
-    if (Specifier == TypeSpecifier::Aggregate && Size != 0)
-      return reportError("Sized aggregate specification in datalayout string");
-
-    // ABI alignment.
-    if (Rest.empty())
-      return reportError(
-          "Missing alignment specification in datalayout string");
-    if (Error Err = ::split(Rest, ':', Split))
-      return Err;
-    unsigned ABIAlign;
-    if (Error Err = getIntInBytes(Tok, ABIAlign))
-      return Err;
-    if (Specifier != TypeSpecifier::Aggregate && !ABIAlign)
-      return reportError(
-          "ABI alignment specification must be >0 for non-aggregate types");
-
-    if (!isUInt<16>(ABIAlign))
-      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 (Specifier == TypeSpecifier::Integer && Size == 8 && ABIAlign != 1)
-      return reportError("Invalid ABI alignment, i8 must be naturally aligned");
-
-    // Preferred alignment.
-    unsigned PrefAlign = ABIAlign;
-    if (!Rest.empty()) {
-      if (Error Err = ::split(Rest, ':', Split))
-        return Err;
-      if (Error Err = getIntInBytes(Tok, PrefAlign))
-        return Err;
-    }
-
-    if (!isUInt<16>(PrefAlign))
-      return reportError(
-          "Invalid preferred alignment, must be a 16bit integer");
-    if (PrefAlign != 0 && !isPowerOf2_64(PrefAlign))
-      return reportError("Invalid preferred alignment, must be a power of 2");
-
-    if (Error Err = setPrimitiveSpec(Specifier, Size, assumeAligned(ABIAlign),
-                                     assumeAligned(PrefAlign)))
-      return Err;
-
+  case 'E':
+    BigEndian = Specifier == 'E';
+    if (!Rest.empty())
+      return createSpecFormatError(Twine(Specifier));
     break;
-  }
   case 'n': // Native integer types.
-    while (true) {
-      unsigned Width;
-      if (Error Err = getInt(Tok, Width))
-        return Err;
-      if (Width == 0)
-        return reportError(
-            "Zero width native integer type in datalayout string");
-      LegalIntWidths.push_back(Width);
-      if (Rest.empty())
-        break;
-      if (Error Err = ::split(Rest, ':', Split))
+    // n<size>[:<size>]...
+    for (StringRef Str : split(Rest, ':')) {
+      unsigned BitWidth;
+      if (Error Err = parseSize(Str, BitWidth))
         return Err;
+      LegalIntWidths.push_back(BitWidth);
     }
     break;
   case 'S': { // Stack natural alignment.
-    uint64_t Alignment;
-    if (Error Err = getIntInBytes(Tok, Alignment))
+    // S<size>
+    Align Alignment;
+    if (Error Err = parseAlignment(Rest, Alignment, "stack natural",
+                                   /*AllowZero=*/true))
       return Err;
-    if (Alignment != 0 && !llvm::isPowerOf2_64(Alignment))
-      return reportError("Alignment is neither 0 nor a power of 2");
-    StackNaturalAlign = MaybeAlign(Alignment);
+    StackNaturalAlign = Alignment;
     break;
   }
   case 'F': {
-    switch (Tok.front()) {
+    // F<type><abi>
+    // FIXME: Bounds checking.
+    char Type = Rest.front();
+    Rest = Rest.drop_front();
+    switch (Type) {
     case 'i':
       TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
       break;
@@ -582,44 +537,38 @@ Error DataLayout::parseSpecification(StringRef Spec) {
       TheFunctionPtrAlignType = FunctionPtrAlignType::MultipleOfFunctionAlign;
       break;
     default:
-      return reportError("Unknown function pointer alignment type in "
-                         "datalayout string");
+      return createStringError("unknown function pointer alignment type '" +
+                               Twine(Type) + "'");
     }
-    Tok = Tok.substr(1);
-    uint64_t Alignment;
-    if (Error Err = getIntInBytes(Tok, Alignment))
+    Align Alignment;
+    if (Error Err = parseAlignment(Rest, Alignment, "ABI", /*AllowZero=*/true))
       return Err;
-    if (Alignment != 0 && !llvm::isPowerOf2_64(Alignment))
-      return reportError("Alignment is neither 0 nor a power of 2");
-    FunctionPtrAlign = MaybeAlign(Alignment);
+    FunctionPtrAlign = Alignment;
     break;
   }
   case 'P': { // Function address space.
-    if (Error Err = getAddrSpace(Tok, ProgramAddrSpace))
+    if (Error Err = parseAddrSpace(Rest, ProgramAddrSpace))
       return Err;
     break;
   }
   case 'A': { // Default stack/alloca address space.
-    if (Error Err = getAddrSpace(Tok, AllocaAddrSpace))
+    if (Error Err = parseAddrSpace(Rest, AllocaAddrSpace))
       return Err;
     break;
   }
   case 'G': { // Default address space for global variables.
-    if (Error Err = getAddrSpace(Tok, DefaultGlobalsAddrSpace))
+    if (Error Err = parseAddrSpace(Rest, DefaultGlobalsAddrSpace))
       return Err;
     break;
   }
   case 'm':
-    if (!Tok.empty())
-      return reportError("Unexpected trailing characters after mangling "
-                         "specifier in datalayout string");
-    if (Rest.empty())
-      return reportError("Expected mangling specifier in datalayout string");
+    if (!Rest.consume_front(":") || Rest.empty())
+      return createSpecFormatError("m:<mangling>");
     if (Rest.size() > 1)
-      return reportError("Unknown mangling specifier in datalayout string");
+      return createStringError("unknown mangling specifier");
     switch (Rest[0]) {
     default:
-      return reportError("Unknown mangling in datalayout string");
+      return createStringError("unknown mangling specifier");
     case 'e':
       ManglingMode = MM_ELF;
       break;
@@ -644,7 +593,7 @@ Error DataLayout::parseSpecification(StringRef Spec) {
     }
     break;
   default:
-    return reportError("Unknown specifier in datalayout string");
+    return createStringError("unknown specifier '" + Twine(Specifier) + "'");
   }
 
   return Error::success();
@@ -668,32 +617,19 @@ Error DataLayout::parseLayoutString(StringRef LayoutString) {
   return Error::success();
 }
 
-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
-  // an assert. See D67400 for context.
-  assert(Log2(ABIAlign) < 16 && Log2(PrefAlign) < 16 && "Alignment too big");
-  if (!isUInt<24>(BitWidth))
-    return reportError("Invalid bit width, must be a 24-bit integer");
-  if (PrefAlign < ABIAlign)
-    return reportError(
-        "Preferred alignment cannot be less than the ABI alignment");
-
+void DataLayout::setPrimitiveSpec(char Specifier, uint32_t BitWidth,
+                                  Align ABIAlign, Align PrefAlign) {
   SmallVectorImpl<PrimitiveSpec> *Specs;
   switch (Specifier) {
-  case TypeSpecifier::Aggregate:
-    StructABIAlignment = ABIAlign;
-    StructPrefAlignment = PrefAlign;
-    return Error::success();
-  case TypeSpecifier::Integer:
+  default:
+    llvm_unreachable("Unexpected specifier");
+  case 'i':
     Specs = &IntSpecs;
     break;
-  case TypeSpecifier::Float:
+  case 'f':
     Specs = &FloatSpecs;
     break;
-  case TypeSpecifier::Vector:
+  case 'v':
     Specs = &VectorSpecs;
     break;
   }
@@ -707,7 +643,6 @@ Error DataLayout::setPrimitiveSpec(TypeSpecifier Specifier, uint32_t BitWidth,
     // Insert before I to keep the vector sorted.
     Specs->insert(I, PrimitiveSpec{BitWidth, ABIAlign, PrefAlign});
   }
-  return Error::success();
 }
 
 const DataLayout::PointerSpec &
diff --git a/llvm/test/Assembler/invalid-datalayout-override.ll b/llvm/test/Assembler/invalid-datalayout-override.ll
index f62777c6c53287..28bbbfb99a180d 100644
--- a/llvm/test/Assembler/invalid-datalayout-override.ll
+++ b/llvm/test/Assembler/invalid-datalayout-override.ll
@@ -4,4 +4,4 @@
 ; RUN: llvm-as -data-layout "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" < %s
 
 target datalayout = "A16777216"
-; CHECK: Invalid address space, must be a 24-bit integer
+; CHECK: address space must be a 24-bit integer
diff --git a/llvm/test/Bitcode/function-default-address-spaces.ll b/llvm/test/Bitcode/function-default-address-spaces.ll
index 3aae5db863b88a..03b4d5c2e04b80 100644
--- a/llvm/test/Bitcode/function-default-address-spaces.ll
+++ b/llvm/test/Bitcode/function-default-address-spaces.ll
@@ -1,7 +1,7 @@
 ; RUN: llvm-as %s  -o - | llvm-dis - | FileCheck %s -check-prefixes CHECK,PROG-AS0
 ; RUN: llvm-as -data-layout "P200" %s  -o - | llvm-dis | FileCheck %s -check-prefixes CHECK,PROG-AS200
 ; RUN: not llvm-as -data-layout "P123456789" %s -o /dev/null 2>&1 | FileCheck %s -check-prefix BAD-DATALAYOUT
-; BAD-DATALAYOUT: error: Invalid address space, must be a 24-bit integer
+; BAD-DATALAYOUT: error: address space must be a 24-bit integer
 
 ; PROG-AS0-NOT: target datalayout
 ; PROG-AS200: target datalayout = "P200"
diff --git a/llvm/test/Bitcode/invalid-functionptr-align.ll b/llvm/test/Bitcode/invalid-functionptr-align.ll
deleted file mode 100644
index 8b446d371f6fde..00000000000000
--- a/llvm/test/Bitcode/invalid-functionptr-align.ll
+++ /dev/null
@@ -1,5 +0,0 @@
-; Bitcode with invalid function pointer alignment.
-
-; RUN: not llvm-dis %s.bc -o - 2>&1 | FileCheck %s
-
-CHECK: error: Alignment is neither 0 nor a power of 2
diff --git a/llvm/test/Bitcode/invalid-functionptr-align.ll.bc b/llvm/test/Bitcode/invalid-stack-align.bc
similarity index 100%
rename from llvm/test/Bitcode/invalid-functionptr-align.ll.bc
rename to llvm/test/Bitcode/invalid-stack-align.bc
diff --git a/llvm/test/Bitcode/invalid-stack-align.ll.bc b/llvm/test/Bitcode/invalid-stack-align.ll.bc
new file mode 100644
index 00000000000000..d499d707e3514c
--- /dev/null
+++ b/llvm/test/Bitcode/invalid-stack-align.ll.bc
@@ -0,0 +1,5 @@
+; Bitcode with invalid natural stack alignment.
+
+; RUN: not llvm-dis %s.bc -o - 2>&1 | FileCheck %s
+
+CHECK: error: <size> must be a power of two times the byte width
diff --git a/llvm/test/Transforms/InstCombine/crash.ll b/llvm/test/Transforms/InstCombine/crash.ll
index 5f86069ef73680..9b37d6943b9e52 100644
--- a/llvm/test/Transforms/InstCombine/crash.ll
+++ b/llvm/test/Transforms/InstCombine/crash.ll
@@ -1,5 +1,5 @@
 ; RUN: opt < %s -passes=instcombine -S
-target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128:n8:16:32"
+target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128-n8:16:32"
 target triple = "i386-apple-darwin10.0"
 
 define i32 @test0(i8 %tmp2) ssp {
diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll
index b12982dd27e404..2673b1d74bb6fb 100644
--- a/llvm/test/Transforms/InstCombine/phi.ll
+++ b/llvm/test/Transforms/InstCombine/phi.ll
@@ -1,7 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=instcombine -S | FileCheck %s
 
-target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128:n8:16:32:64"
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64"
 
 define i32 @test1(i32 %A, i1 %b) {
 ; CHECK-LABEL: @test1(
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 4b236b2d165f9c..cffeb050acea4f 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -21,78 +21,69 @@ namespace {
 
 // TODO: Split into multiple TESTs.
 TEST(DataLayoutTest, ParseErrors) {
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("^"),
-      FailedWithMessage("Unknown specifier in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("m:v"),
-      FailedWithMessage("Unknown mangling in datalayout string"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("^"),
+                       FailedWithMessage("unknown specifier '^'"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("m:v"),
+                       FailedWithMessage("unknown mangling specifier"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("n0"),
-      FailedWithMessage("Zero width native integer type in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("a1:64"),
-      FailedWithMessage("Sized aggregate specification in datalayout string"));
+      FailedWithMessage("size must be a non-zero 24-bit integer"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("a1:64"),
+                       FailedWithMessage("size component must be empty"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("a:"),
-      FailedWithMessage("Trailing separator in datalayout string"));
+      FailedWithMessage("ABI alignment component cannot be empty"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("m"),
-      FailedWithMessage("Expected mangling specifier in datalayout string"));
+      FailedWithMessage(
+          "malformed specification, must be of the form \"m:<mangling>\""));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("m."),
-      FailedWithMessage("Unexpected trailing characters after mangling "
-                        "specifier in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("f"),
       FailedWithMessage(
-          "Missing alignment specification in datalayout string"));
+          "malformed specification, must be of the form \"m:<mangling>\""));
   EXPECT_THAT_EXPECTED(
-      DataLayout::parse(":32"),
-      FailedWithMessage(
-          "Expected token before separator in datalayout string"));
+      DataLayout::parse("f"),
+      FailedWithMessage("malformed specification, must be of the form "
+                        "\"f<size>:<abi>[:<pref>]\""));
+  EXPECT_THAT_EXPECTED(DataLayout::parse(":32"),
+                       FailedWithMessage("unknown specifier ':'"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("i64:64:16"),
       FailedWithMessage(
-          "Preferred alignment cannot be less than the ABI alignment"));
+          "preferred alignment cannot be less than the ABI alignment"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("i64:16:16777216"),
-      FailedWithMessage(
-          "Invalid preferred alignment, must be a 16bit integer"));
+      FailedWithMessage("preferred alignment must be a 16-bit integer"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("i64:16777216:16777216"),
-      FailedWithMessage("Invalid ABI alignment, must be a 16bit integer"));
+      FailedWithMessage("ABI alignment must be a 16-bit integer"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("i16777216:16:16"),
-      FailedWithMessage("Invalid bit width, must be a 24-bit integer"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("v128:0:128"),
-      FailedWithMessage(
-          "ABI alignment specification must be >0 for non-aggregate types"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("i32:24:32"),
-      FailedWithMessage("Invalid ABI alignment, must be a power of 2"));
+      FailedWithMessage("size must be a non-zero 24-bit integer"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("v128:0:128"),
+                       FailedWithMessage("ABI alignment must be non-zero"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("i32:32:24"),
-      FailedWithMessage("Invalid preferred alignment, must be a power of 2"));
+      FailedWithMessage(
+          "preferred alignment must be a power of two times the byte width"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("A16777216"),
-      FailedWithMessage("Invalid address space, must be a 24-bit integer"));
+      FailedWithMessage("address space must be a 24-bit integer"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("G16777216"),
-      FailedWithMessage("Invalid address space, must be a 24-bit integer"));
+      FailedWithMessage("address space must be a 24-bit integer"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("P16777216"),
-      FailedWithMessage("Invalid address space, must be a 24-bit integer"));
+      FailedWithMessage("address space must be a 24-bit integer"));
   EXPECT_THAT_EXPECTED(
       DataLayout::parse("Fi24"),
-      FailedWithMessage("Alignment is neither 0 nor a power of 2"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("i8:16"),
-      FailedWithMessage("Invalid ABI alignment, i8 must be naturally aligned"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("S24"),
-      FailedWithMessage("Alignment is neither 0 nor a power of 2"));
+      FailedWithMessage(
+          "ABI alignment must be a power of two times the byte width"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("i8:16"),
+                       FailedWithMessage("i8 must be 8-bit aligned"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("S24"),
+                       FailedWithMessage("stack natural alignment must be a "
+                                         "power of two times the byte width"));
 }
 
 TEST(DataLayout, LayoutStringFormat) {



More information about the llvm-commits mailing list