[llvm] [DataLayout] Refactor the rest of `parseSpecification` (PR #104545)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 19 20:27:28 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Sergei Barannikov (s-barannikov)
<details>
<summary>Changes</summary>
The aim is to improve test coverage of data layout string parsing.
---
Full diff: https://github.com/llvm/llvm-project/pull/104545.diff
7 Files Affected:
- (modified) llvm/lib/IR/DataLayout.cpp (+43-97)
- (modified) llvm/test/Assembler/invalid-datalayout-override.ll (+1-1)
- (modified) llvm/test/Bitcode/function-default-address-spaces.ll (+1-1)
- (removed) llvm/test/Bitcode/invalid-functionptr-align.ll (-5)
- (added) llvm/test/Bitcode/invalid-stack-align.ll (+5)
- (renamed) llvm/test/Bitcode/invalid-stack-align.ll.bc ()
- (modified) llvm/unittests/IR/DataLayoutTest.cpp (+135-43)
``````````diff
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/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 9464b489b65f43..6054cf923de646 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", "S0", "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"));
``````````
</details>
https://github.com/llvm/llvm-project/pull/104545
More information about the llvm-commits
mailing list