[llvm] [DataLayout] Refactor parsing of "ni" specification (PR #104546)
Sergei Barannikov via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 16 01:52:07 PDT 2024
https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/104546
>From 5cc8cacb31590d0d779c7d5d48d39be61ed86908 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 16 Aug 2024 08:24:49 +0300
Subject: [PATCH 1/2] [DataLayout] Refactor parsing of "ni" specification
Part of #104545.
This introduces `parseAddrSpace` function, intended as a replacement
for `getAddrSpace`, which doesn't check for trailing characters after
the address space number. `getAddrSpace` will be removed after switching
all uses to `parseAddrSpace`.
---
llvm/lib/IR/DataLayout.cpp | 57 ++++++++++++++++++++--------
llvm/unittests/IR/DataLayoutTest.cpp | 40 +++++++++++++++++++
2 files changed, 81 insertions(+), 16 deletions(-)
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index 24c29458abfa81..b8e9b67e166e82 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -272,6 +272,27 @@ 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 + "\"");
+}
+
+/// Attempts to parse an address space component of a specification,
+/// commonly referred to as <n> or <address space>.
+///
+/// \p Name should be the name of the component as specified by LangRef,
+/// it is used in diagnostic messages.
+static Error parseAddrSpace(StringRef Str, unsigned &AddrSpace,
+ StringRef Name) {
+ if (Str.empty())
+ return createStringError(Name + " is required");
+
+ if (!to_integer(Str, AddrSpace, 10) || !isUInt<24>(AddrSpace))
+ return createStringError(Name + " must be a 24-bit integer");
+
+ return Error::success();
+}
+
/// Checked version of split, to ensure mandatory subparts.
static Error split(StringRef Str, char Separator,
std::pair<StringRef, StringRef> &Split) {
@@ -313,6 +334,26 @@ static Error getAddrSpace(StringRef R, unsigned &AddrSpace) {
}
Error DataLayout::parseSpecification(StringRef Spec) {
+ // The "ni" specifier is the only two-character specifier. Handle it first.
+ if (Spec.starts_with("ni")) {
+ // ni:<address space>[:<address space>]...
+ StringRef Rest = Spec.drop_front(2);
+
+ // Drop the first ':', then split the rest of the string the usual way.
+ if (!Rest.consume_front(":"))
+ return createSpecFormatError("ni:<address space>[:<address space>]...");
+
+ for (StringRef Str : split(Rest, ':')) {
+ unsigned AddrSpace;
+ if (Error Err = parseAddrSpace(Str, AddrSpace, "<address space>"))
+ return Err;
+ if (AddrSpace == 0)
+ return createStringError("address space 0 cannot be non-integral");
+ NonIntegralAddressSpaces.push_back(AddrSpace);
+ }
+ return Error::success();
+ }
+
// Split at ':'.
std::pair<StringRef, StringRef> Split;
if (Error Err = ::split(Spec, ':', Split))
@@ -322,22 +363,6 @@ Error DataLayout::parseSpecification(StringRef Spec) {
StringRef &Tok = Split.first; // Current token.
StringRef &Rest = Split.second; // The rest of the string.
- if (Tok == "ni") {
- do {
- if (Error Err = ::split(Rest, ':', Split))
- return Err;
- Rest = Split.second;
- unsigned AS;
- if (Error Err = getInt(Split.first, AS))
- return Err;
- if (AS == 0)
- return reportError("Address space 0 can never be non-integral");
- NonIntegralAddressSpaces.push_back(AS);
- } while (!Rest.empty());
-
- return Error::success();
- }
-
char SpecifierChar = Tok.front();
Tok = Tok.substr(1);
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index f5c930ebdbb9c4..6df5c110f642db 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -134,6 +134,46 @@ TEST(DataLayout, LayoutStringFormat) {
FailedWithMessage("empty specification is not allowed"));
}
+TEST(DataLayout, ParseNonIntegralAddrSpace) {
+ for (StringRef Str : {"ni:1", "ni:16777215", "ni:1:16777215"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
+ for (StringRef Str : {"ni", "ni42", "nix"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("malformed specification, must be of the form "
+ "\"ni:<address space>[:<address space>]...\""));
+
+ for (StringRef Str : {"ni:", "ni::42", "ni:42:"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(Str),
+ FailedWithMessage("<address space> is required"));
+
+ for (StringRef Str : {"ni:x", "ni:16777216", "ni:42:16777216"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("<address space> must be a 24-bit integer"));
+
+ for (StringRef Str : {"ni:0", "ni:42:0"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("address space 0 cannot be non-integral"));
+}
+
+TEST(DataLayout, IsNonIntegralAddressSpace) {
+ DataLayout Default;
+ EXPECT_THAT(Default.getNonIntegralAddressSpaces(), ::testing::SizeIs(0));
+ EXPECT_FALSE(Default.isNonIntegralAddressSpace(0));
+ EXPECT_FALSE(Default.isNonIntegralAddressSpace(1));
+
+ DataLayout Custom = cantFail(DataLayout::parse("ni:2:16777215"));
+ EXPECT_THAT(Custom.getNonIntegralAddressSpaces(),
+ ::testing::ElementsAreArray({2U, 16777215U}));
+ EXPECT_FALSE(Custom.isNonIntegralAddressSpace(0));
+ EXPECT_FALSE(Custom.isNonIntegralAddressSpace(1));
+ EXPECT_TRUE(Custom.isNonIntegralAddressSpace(2));
+ EXPECT_TRUE(Custom.isNonIntegralAddressSpace(16777215));
+}
+
TEST(DataLayoutTest, CopyAssignmentInvalidatesStructLayout) {
DataLayout DL1 = cantFail(DataLayout::parse("p:32:32"));
DataLayout DL2 = cantFail(DataLayout::parse("p:64:64"));
>From 041f89dc026228728a1bb331bec2ee4728f668b0 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 16 Aug 2024 11:50:11 +0300
Subject: [PATCH 2/2] Change the diagnostic message to plain English, a few
minor fixes
---
llvm/lib/IR/DataLayout.cpp | 12 ++++--------
llvm/unittests/IR/DataLayoutTest.cpp | 9 +++++----
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index b8e9b67e166e82..fb77e8376c8a41 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -279,16 +279,12 @@ static Error createSpecFormatError(Twine Format) {
/// Attempts to parse an address space component of a specification,
/// commonly referred to as <n> or <address space>.
-///
-/// \p Name should be the name of the component as specified by LangRef,
-/// it is used in diagnostic messages.
-static Error parseAddrSpace(StringRef Str, unsigned &AddrSpace,
- StringRef Name) {
+static Error parseAddrSpace(StringRef Str, unsigned &AddrSpace) {
if (Str.empty())
- return createStringError(Name + " is required");
+ return createStringError("address space component cannot be empty");
if (!to_integer(Str, AddrSpace, 10) || !isUInt<24>(AddrSpace))
- return createStringError(Name + " must be a 24-bit integer");
+ return createStringError("address space must be a 24-bit integer");
return Error::success();
}
@@ -345,7 +341,7 @@ Error DataLayout::parseSpecification(StringRef Spec) {
for (StringRef Str : split(Rest, ':')) {
unsigned AddrSpace;
- if (Error Err = parseAddrSpace(Str, AddrSpace, "<address space>"))
+ if (Error Err = parseAddrSpace(Str, AddrSpace))
return Err;
if (AddrSpace == 0)
return createStringError("address space 0 cannot be non-integral");
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 6df5c110f642db..a40ac5b83a83e4 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -145,13 +145,14 @@ TEST(DataLayout, ParseNonIntegralAddrSpace) {
"\"ni:<address space>[:<address space>]...\""));
for (StringRef Str : {"ni:", "ni::42", "ni:42:"})
- EXPECT_THAT_EXPECTED(DataLayout::parse(Str),
- FailedWithMessage("<address space> is required"));
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("address space component cannot be empty"));
- for (StringRef Str : {"ni:x", "ni:16777216", "ni:42:16777216"})
+ for (StringRef Str : {"ni:x", "ni:42:0x1", "ni:16777216", "ni:42:16777216"})
EXPECT_THAT_EXPECTED(
DataLayout::parse(Str),
- FailedWithMessage("<address space> must be a 24-bit integer"));
+ FailedWithMessage("address space must be a 24-bit integer"));
for (StringRef Str : {"ni:0", "ni:42:0"})
EXPECT_THAT_EXPECTED(
More information about the llvm-commits
mailing list