[Mlir-commits] [mlir] c91cc45 - [DataLayout] Refactor the rest of `parseSpecification` (#104545)

llvmlistbot at llvm.org llvmlistbot at llvm.org
Tue Aug 20 01:25:54 PDT 2024


Author: Sergei Barannikov
Date: 2024-08-20T11:25:49+03:00
New Revision: c91cc459d396dd4625d3dbf9a2af6fac8151eb4b

URL: https://github.com/llvm/llvm-project/commit/c91cc459d396dd4625d3dbf9a2af6fac8151eb4b
DIFF: https://github.com/llvm/llvm-project/commit/c91cc459d396dd4625d3dbf9a2af6fac8151eb4b.diff

LOG: [DataLayout] Refactor the rest of `parseSpecification` (#104545)

The aim is to improve test coverage of data layout string parsing.

Pull Request: https://github.com/llvm/llvm-project/pull/104545

Added: 
    llvm/test/Bitcode/invalid-stack-align.ll
    llvm/test/Bitcode/invalid-stack-align.ll.bc

Modified: 
    lld/test/ELF/exclude-libs.s
    llvm/lib/IR/DataLayout.cpp
    llvm/test/Assembler/invalid-datalayout-override.ll
    llvm/test/Bitcode/function-default-address-spaces.ll
    llvm/test/CodeGen/PowerPC/pr15359.ll
    llvm/test/CodeGen/PowerPC/pr16556-2.ll
    llvm/test/CodeGen/PowerPC/pr16556.ll
    llvm/unittests/IR/DataLayoutTest.cpp
    mlir/lib/Target/LLVMIR/DataLayoutImporter.cpp
    mlir/test/Target/LLVMIR/Import/data-layout.ll

Removed: 
    llvm/test/Bitcode/invalid-functionptr-align.ll
    llvm/test/Bitcode/invalid-functionptr-align.ll.bc


################################################################################
diff  --git a/lld/test/ELF/exclude-libs.s b/lld/test/ELF/exclude-libs.s
index 513eb20ba6b482..c25c1eddc2e0d1 100644
--- a/lld/test/ELF/exclude-libs.s
+++ b/lld/test/ELF/exclude-libs.s
@@ -3,7 +3,7 @@
 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux \
 // RUN:   %p/Inputs/exclude-libs.s -o %t2.o
-// RUN: llvm-as --data-layout=elf %p/Inputs/exclude-libs.ll -o %t3.o
+// RUN: llvm-as --data-layout=e %p/Inputs/exclude-libs.ll -o %t3.o
 // RUN: mkdir -p %t.dir
 // RUN: rm -f %t.dir/exc.a
 // RUN: llvm-ar rcs %t.dir/exc.a %t2.o %t3.o

diff  --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 5efdaaccf1fc7f..d295d1f5785eb9 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,46 +332,6 @@ 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();
-}
-
-/// 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();
-}
-
-/// 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))
-    return Err;
-  if (Result % 8)
-    return reportError("number of bits must be a byte width multiple");
-  Result /= 8;
-  return Error::success();
-}
-
-static Error getAddrSpace(StringRef R, unsigned &AddrSpace) {
-  if (Error Err = getInt(R, AddrSpace))
-    return Err;
-  if (!isUInt<24>(AddrSpace))
-    return reportError("Invalid address space, must be a 24-bit integer");
-  return Error::success();
-}
-
 Error DataLayout::parsePrimitiveSpec(StringRef Spec) {
   // [ifv]<size>:<abi>[:<pref>]
   SmallVector<StringRef, 3> Components;
@@ -536,55 +492,45 @@ Error DataLayout::parseSpecification(StringRef Spec) {
   if (Specifier == 'p')
     return parsePointerSpec(Spec);
 
-  // Split at ':'.
-  std::pair<StringRef, StringRef> Split;
-  if (Error Err = ::split(Spec, ':', Split))
-    return Err;
-
-  // Aliases used below.
-  StringRef &Tok = Split.first;   // Current token.
-  StringRef &Rest = Split.second; // The rest of the string.
-
-  char SpecifierChar = Tok.front();
-  Tok = Tok.substr(1);
-
-  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;
+  case 'E':
+    if (!Rest.empty())
+      return createStringError(
+          "malformed specification, must be just 'e' or 'E'");
+    BigEndian = Specifier == 'E';
     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>
+    if (Rest.empty())
+      return createSpecFormatError("S<size>");
+    Align Alignment;
+    if (Error Err = parseAlignment(Rest, Alignment, "stack natural"))
       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>
+    if (Rest.empty())
+      return createSpecFormatError("F<type><abi>");
+    char Type = Rest.front();
+    Rest = Rest.drop_front();
+    switch (Type) {
     case 'i':
       TheFunctionPtrAlignType = FunctionPtrAlignType::Independent;
       break;
@@ -592,44 +538,44 @@ 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"))
       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 (Rest.empty())
+      return createSpecFormatError("P<address space>");
+    if (Error Err = parseAddrSpace(Rest, ProgramAddrSpace))
       return Err;
     break;
   }
   case 'A': { // Default stack/alloca address space.
-    if (Error Err = getAddrSpace(Tok, AllocaAddrSpace))
+    if (Rest.empty())
+      return createSpecFormatError("A<address space>");
+    if (Error Err = parseAddrSpace(Rest, AllocaAddrSpace))
       return Err;
     break;
   }
   case 'G': { // Default address space for global variables.
-    if (Error Err = getAddrSpace(Tok, DefaultGlobalsAddrSpace))
+    if (Rest.empty())
+      return createSpecFormatError("G<address space>");
+    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 mode");
     switch (Rest[0]) {
     default:
-      return reportError("Unknown mangling in datalayout string");
+      return createStringError("unknown mangling mode");
     case 'e':
       ManglingMode = MM_ELF;
       break;
@@ -654,7 +600,7 @@ Error DataLayout::parseSpecification(StringRef Spec) {
     }
     break;
   default:
-    return reportError("Unknown specifier in datalayout string");
+    return createStringError("unknown specifier '" + Twine(Specifier) + "'");
   }
 
   return Error::success();

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-stack-align.ll b/llvm/test/Bitcode/invalid-stack-align.ll
new file mode 100644
index 00000000000000..a5cca2ad6be986
--- /dev/null
+++ b/llvm/test/Bitcode/invalid-stack-align.ll
@@ -0,0 +1,5 @@
+; Bitcode with invalid natural stack alignment.
+
+; RUN: not llvm-dis %s.bc -o - 2>&1 | FileCheck %s
+
+CHECK: error: stack natural alignment must be a power of two times the byte width

diff  --git a/llvm/test/Bitcode/invalid-functionptr-align.ll.bc b/llvm/test/Bitcode/invalid-stack-align.ll.bc
similarity index 100%
rename from llvm/test/Bitcode/invalid-functionptr-align.ll.bc
rename to llvm/test/Bitcode/invalid-stack-align.ll.bc

diff  --git a/llvm/test/CodeGen/PowerPC/pr15359.ll b/llvm/test/CodeGen/PowerPC/pr15359.ll
index c7be78e5c3cead..5c30790f69d703 100644
--- a/llvm/test/CodeGen/PowerPC/pr15359.ll
+++ b/llvm/test/CodeGen/PowerPC/pr15359.ll
@@ -1,7 +1,7 @@
 ; RUN: llc -verify-machineinstrs -O0 -mcpu=pwr7 -filetype=obj %s -o - | \
 ; RUN: llvm-readobj --symbols - | FileCheck %s
 
-target datalayout = "E-p:64:64:64-S0-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:128:128-v64:64:64-v128:128:128-a0:0:64-n32:64"
+target datalayout = "E-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:128:128-v64:64:64-v128:128:128-a0:0:64-n32:64"
 target triple = "powerpc64-unknown-linux-gnu"
 
 @nextIdx = external thread_local global i32

diff  --git a/llvm/test/CodeGen/PowerPC/pr16556-2.ll b/llvm/test/CodeGen/PowerPC/pr16556-2.ll
index 9b7b3b21ff30bd..8f871cafcf6388 100644
--- a/llvm/test/CodeGen/PowerPC/pr16556-2.ll
+++ b/llvm/test/CodeGen/PowerPC/pr16556-2.ll
@@ -3,7 +3,7 @@
 ; This test formerly failed because of wrong custom lowering for
 ; fptosi of ppc_fp128.
 
-target datalayout = "E-p:32:32:32-S0-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:64:128-v64:64:64-v128:128:128-a0:0:64-n32"
+target datalayout = "E-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:64:128-v64:64:64-v128:128:128-a0:0:64-n32"
 target triple = "powerpc-unknown-linux-gnu"
 
 %core.time.TickDuration = type { i64 }

diff  --git a/llvm/test/CodeGen/PowerPC/pr16556.ll b/llvm/test/CodeGen/PowerPC/pr16556.ll
index 95f4d1df5b27b2..ae680d37639101 100644
--- a/llvm/test/CodeGen/PowerPC/pr16556.ll
+++ b/llvm/test/CodeGen/PowerPC/pr16556.ll
@@ -2,7 +2,7 @@
 
 ; This test formerly failed due to no handling for a ppc_fp128 undef.
 
-target datalayout = "E-p:32:32:32-S0-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:64:128-v64:64:64-v128:128:128-a0:0:64-n32"
+target datalayout = "E-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f16:16:16-f32:32:32-f64:64:64-f128:64:128-v64:64:64-v128:128:128-a0:0:64-n32"
 target triple = "powerpc-unknown-linux-gnu"
 
 %core.time.TickDuration.37.125 = type { i64 }

diff  --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 9464b489b65f43..396d44af19f53f 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -21,53 +21,130 @@ namespace {
 
 class DataLayoutTest : public ::testing::Test {};
 
-// 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("n0"),
-      FailedWithMessage("Zero width native integer type in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("m"),
-      FailedWithMessage("Expected mangling specifier in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("m."),
-      FailedWithMessage("Unexpected trailing characters after mangling "
-                        "specifier in datalayout string"));
+TEST(DataLayout, LayoutStringFormat) {
+  for (StringRef Str : {"", "e", "m:e", "m:e-e"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
+  for (StringRef Str : {"-", "e-", "-m:e", "m:e--e"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("empty specification is not allowed"));
+}
+
+TEST(DataLayoutTest, InvalidSpecifier) {
+  EXPECT_THAT_EXPECTED(DataLayout::parse("^"),
+                       FailedWithMessage("unknown specifier '^'"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("I8:8"),
+                       FailedWithMessage("unknown specifier 'I'"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("e-X"),
+                       FailedWithMessage("unknown specifier 'X'"));
+  EXPECT_THAT_EXPECTED(DataLayout::parse("p0:32:32-64"),
+                       FailedWithMessage("unknown specifier '6'"));
+}
+
+TEST(DataLayoutTest, ParseEndianness) {
+  EXPECT_THAT_EXPECTED(DataLayout::parse("e"), Succeeded());
+  EXPECT_THAT_EXPECTED(DataLayout::parse("E"), Succeeded());
+
+  for (StringRef Str : {"ee", "e0", "e:0", "E0:E", "El", "E:B"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("malformed specification, must be just 'e' or 'E'"));
+}
+
+TEST(DataLayoutTest, ParseMangling) {
+  for (StringRef Str : {"m:a", "m:e", "m:l", "m:m", "m:o", "m:w", "m:x"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
+  for (StringRef Str : {"m", "ms:m", "m:"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage(
+            "malformed specification, must be of the form \"m:<mangling>\""));
+
+  for (StringRef Str : {"m:ms", "m:E", "m:0"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str),
+                         FailedWithMessage("unknown mangling mode"));
+}
+
+TEST(DataLayoutTest, ParseStackNaturalAlign) {
+  for (StringRef Str : {"S8", "S32768"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
   EXPECT_THAT_EXPECTED(
-      DataLayout::parse(":32"),
+      DataLayout::parse("S"),
       FailedWithMessage(
-          "Expected token before separator in datalayout string"));
-  EXPECT_THAT_EXPECTED(
-      DataLayout::parse("A16777216"),
-      FailedWithMessage("Invalid address space, must be a 24-bit integer"));
+          "malformed specification, must be of the form \"S<size>\""));
+
+  for (StringRef Str : {"SX", "S0x20", "S65536"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("stack natural alignment must be a 16-bit integer"));
+
   EXPECT_THAT_EXPECTED(
-      DataLayout::parse("G16777216"),
-      FailedWithMessage("Invalid address space, must be a 24-bit integer"));
+      DataLayout::parse("S0"),
+      FailedWithMessage("stack natural alignment must be non-zero"));
+
+  for (StringRef Str : {"S1", "S7", "S24", "S65535"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("stack natural alignment must be a power of two "
+                          "times the byte width"));
+}
+
+TEST(DataLayoutTest, ParseAddrSpace) {
+  for (StringRef Str : {"P0", "A0", "G0", "P1", "A1", "G1", "P16777215",
+                        "A16777215", "G16777215"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
+  for (StringRef Str : {"P", "A", "G"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage(("malformed specification, must be of the form \"" +
+                           Twine(Str.front()) + "<address space>\"")
+                              .str()));
+
+  for (StringRef Str : {"Px", "A0x1", "G16777216"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("address space must be a 24-bit integer"));
+}
+
+TEST(DataLayoutTest, ParseFuncPtrSpec) {
+  for (StringRef Str : {"Fi8", "Fn16", "Fi32768", "Fn32768"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
   EXPECT_THAT_EXPECTED(
-      DataLayout::parse("P16777216"),
-      FailedWithMessage("Invalid address space, must be a 24-bit integer"));
+      DataLayout::parse("F"),
+      FailedWithMessage(
+          "malformed specification, must be of the form \"F<type><abi>\""));
+
   EXPECT_THAT_EXPECTED(
-      DataLayout::parse("Fi24"),
-      FailedWithMessage("Alignment is neither 0 nor a power of 2"));
+      DataLayout::parse("FN"),
+      FailedWithMessage("unknown function pointer alignment type 'N'"));
   EXPECT_THAT_EXPECTED(
-      DataLayout::parse("S24"),
-      FailedWithMessage("Alignment is neither 0 nor a power of 2"));
-}
+      DataLayout::parse("F32"),
+      FailedWithMessage("unknown function pointer alignment type '3'"));
 
-TEST(DataLayout, LayoutStringFormat) {
-  for (StringRef Str : {"", "e", "m:e", "m:e-e"})
-    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+  for (StringRef Str : {"Fi", "Fn"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("ABI alignment component cannot be empty"));
 
-  for (StringRef Str : {"-", "e-", "-m:e", "m:e--e"})
+  for (StringRef Str : {"Fii", "Fn32x", "Fi65536", "Fn65536"})
     EXPECT_THAT_EXPECTED(
         DataLayout::parse(Str),
-        FailedWithMessage("empty specification is not allowed"));
+        FailedWithMessage("ABI alignment must be a 16-bit integer"));
+
+  for (StringRef Str : {"Fi0", "Fn0"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str),
+                         FailedWithMessage("ABI alignment must be non-zero"));
+
+  for (StringRef Str : {"Fi12", "Fn24"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage(
+            "ABI alignment must be a power of two times the byte width"));
 }
 
 class DataLayoutPrimitiveSpecificationTest
@@ -326,6 +403,21 @@ TEST(DataLayout, ParsePointerSpec) {
         FailedWithMessage("index size cannot be larger than the pointer size"));
 }
 
+TEST(DataLayoutTest, ParseNativeIntegersSpec) {
+  for (StringRef Str : {"n1", "n1:8", "n24:12:16777215"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
+  for (StringRef Str : {"n", "n1:", "n:8", "n16::32"})
+    EXPECT_THAT_EXPECTED(DataLayout::parse(Str),
+                         FailedWithMessage("size component cannot be empty"));
+
+  for (StringRef Str : {"n0", "n0x8:16", "n8:0", "n16:0:32", "n16777216",
+                        "n16:16777216", "n32:64:16777216"})
+    EXPECT_THAT_EXPECTED(
+        DataLayout::parse(Str),
+        FailedWithMessage("size must be a non-zero 24-bit integer"));
+}
+
 TEST(DataLayout, ParseNonIntegralAddrSpace) {
   for (StringRef Str : {"ni:1", "ni:16777215", "ni:1:16777215"})
     EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
@@ -494,12 +586,12 @@ TEST(DataLayoutTest, FunctionPtrAlign) {
   EXPECT_EQ(MaybeAlign(2), DataLayout("Fn16").getFunctionPtrAlign());
   EXPECT_EQ(MaybeAlign(4), DataLayout("Fn32").getFunctionPtrAlign());
   EXPECT_EQ(MaybeAlign(8), DataLayout("Fn64").getFunctionPtrAlign());
-  EXPECT_EQ(DataLayout::FunctionPtrAlignType::Independent, \
-      DataLayout("").getFunctionPtrAlignType());
-  EXPECT_EQ(DataLayout::FunctionPtrAlignType::Independent, \
-      DataLayout("Fi8").getFunctionPtrAlignType());
-  EXPECT_EQ(DataLayout::FunctionPtrAlignType::MultipleOfFunctionAlign, \
-      DataLayout("Fn8").getFunctionPtrAlignType());
+  EXPECT_EQ(DataLayout::FunctionPtrAlignType::Independent,
+            DataLayout("").getFunctionPtrAlignType());
+  EXPECT_EQ(DataLayout::FunctionPtrAlignType::Independent,
+            DataLayout("Fi8").getFunctionPtrAlignType());
+  EXPECT_EQ(DataLayout::FunctionPtrAlignType::MultipleOfFunctionAlign,
+            DataLayout("Fn8").getFunctionPtrAlignType());
   EXPECT_EQ(DataLayout("Fi8"), DataLayout("Fi8"));
   EXPECT_NE(DataLayout("Fi8"), DataLayout("Fi16"));
   EXPECT_NE(DataLayout("Fi8"), DataLayout("Fn8"));

diff  --git a/mlir/lib/Target/LLVMIR/DataLayoutImporter.cpp b/mlir/lib/Target/LLVMIR/DataLayoutImporter.cpp
index 392f552f480cc8..35001757f214e1 100644
--- a/mlir/lib/Target/LLVMIR/DataLayoutImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DataLayoutImporter.cpp
@@ -197,9 +197,9 @@ DataLayoutImporter::tryToEmplaceStackAlignmentEntry(StringRef token) {
   if (failed(alignment))
     return failure();
 
-  // Only store the stack alignment if it has a non-default value.
+  // Stack alignment shouldn't be zero.
   if (*alignment == 0)
-    return success();
+    return failure();
   OpBuilder builder(context);
   keyEntries.try_emplace(key, DataLayoutEntryAttr::get(
                                   key, builder.getI64IntegerAttr(*alignment)));

diff  --git a/mlir/test/Target/LLVMIR/Import/data-layout.ll b/mlir/test/Target/LLVMIR/Import/data-layout.ll
index de90ac1e271140..ee6f4dd994f1d0 100644
--- a/mlir/test/Target/LLVMIR/Import/data-layout.ll
+++ b/mlir/test/Target/LLVMIR/Import/data-layout.ll
@@ -47,5 +47,4 @@ target datalayout = "A1-E-p270:16:32:64:8-p271:16:32:64-i64:64:128"
 ; CHECK: dlti.dl_spec =
 ; CHECK: #dlti.dl_spec<
 ; CHECK-NOT:   #dlti.dl_entry<"dlti.alloca_memory_space"
-; CHECK-NOT:   #dlti.dl_entry<"dlti.stack_alignment"
-target datalayout = "A0-S0"
+target datalayout = "A0"


        


More information about the Mlir-commits mailing list