[Mlir-commits] [llvm] [mlir] [DataLayout] Refactor parsing of i/f/v/a specifications (PR #104699)
Sergei Barannikov
llvmlistbot at llvm.org
Mon Aug 19 04:23:39 PDT 2024
https://github.com/s-barannikov updated https://github.com/llvm/llvm-project/pull/104699
>From 0161445f3ca1aabe8b156e118909b9a272a692ca Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Fri, 16 Aug 2024 22:48:25 +0300
Subject: [PATCH 1/4] [DataLayout] Refactor parsing of i/f/v/a specifications
---
llvm/include/llvm/IR/DataLayout.h | 23 ++-
llvm/lib/IR/DataLayout.cpp | 186 ++++++++++-----------
llvm/test/Transforms/InstCombine/crash.ll | 2 +-
llvm/test/Transforms/InstCombine/phi.ll | 2 +-
llvm/unittests/IR/DataLayoutTest.cpp | 190 +++++++++++++++++-----
5 files changed, 258 insertions(+), 145 deletions(-)
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..bae516f33f808f 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -376,6 +376,86 @@ static Error getAddrSpace(StringRef R, unsigned &AddrSpace) {
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, ':');
+
+ if (Components.size() < 2 || Components.size() > 3)
+ return createSpecFormatError(Twine(Specifier) + "<size>:<abi>[:<pref>]");
+
+ // Size. Required, cannot be zero.
+ unsigned BitWidth;
+ if (Error Err = parseSize(Components[0], BitWidth))
+ return Err;
+
+ // 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 one byte 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();
+}
+
+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 zero.
+ if (!Components[0].empty()) {
+ unsigned BitWidth;
+ if (!to_integer(Components[0], BitWidth, 10) || BitWidth != 0)
+ return createStringError("size must be zero");
+ }
+
+ // 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;
+
+ // Preferred alignment. Optional, defaults to the ABI alignment.
+ // Can be zero, meaning use one byte 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();
+}
+
Error DataLayout::parsePointerSpec(StringRef Spec) {
// p[<n>]:<size>:<abi>[:<pref>[:<idx>]]
SmallVector<StringRef, 5> Components;
@@ -451,6 +531,12 @@ Error DataLayout::parseSpecification(StringRef Spec) {
assert(!Spec.empty() && "Empty specification is handled by the caller");
char Specifier = Spec.front();
+ if (Specifier == 'i' || Specifier == 'f' || Specifier == 'v')
+ return parsePrimitiveSpec(Spec);
+
+ if (Specifier == 'a')
+ return parseAggregateSpec(Spec);
+
if (Specifier == 'p')
return parsePointerSpec(Spec);
@@ -477,78 +563,6 @@ Error DataLayout::parseSpecification(StringRef Spec) {
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;
-
- break;
- }
case 'n': // Native integer types.
while (true) {
unsigned Width;
@@ -668,32 +682,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 +708,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/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..7049447a423afa 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -19,6 +19,8 @@ using namespace llvm;
namespace {
+class DataLayoutTest : public ::testing::Test {};
+
// TODO: Split into multiple TESTs.
TEST(DataLayoutTest, ParseErrors) {
EXPECT_THAT_EXPECTED(
@@ -30,12 +32,6 @@ TEST(DataLayoutTest, ParseErrors) {
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"));
- EXPECT_THAT_EXPECTED(
- DataLayout::parse("a:"),
- FailedWithMessage("Trailing separator in datalayout string"));
EXPECT_THAT_EXPECTED(
DataLayout::parse("m"),
FailedWithMessage("Expected mangling specifier in datalayout string"));
@@ -43,38 +39,10 @@ TEST(DataLayoutTest, ParseErrors) {
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"));
EXPECT_THAT_EXPECTED(
DataLayout::parse(":32"),
FailedWithMessage(
"Expected token before separator in datalayout string"));
- EXPECT_THAT_EXPECTED(
- DataLayout::parse("i64:64:16"),
- FailedWithMessage(
- "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"));
- EXPECT_THAT_EXPECTED(
- DataLayout::parse("i64:16777216:16777216"),
- FailedWithMessage("Invalid ABI alignment, must be a 16bit 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"));
- EXPECT_THAT_EXPECTED(
- DataLayout::parse("i32:32:24"),
- FailedWithMessage("Invalid preferred alignment, must be a power of 2"));
EXPECT_THAT_EXPECTED(
DataLayout::parse("A16777216"),
FailedWithMessage("Invalid address space, must be a 24-bit integer"));
@@ -87,9 +55,8 @@ TEST(DataLayoutTest, ParseErrors) {
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("i8:16"),
+ FailedWithMessage("i8 must be 8-bit aligned"));
EXPECT_THAT_EXPECTED(
DataLayout::parse("S24"),
FailedWithMessage("Alignment is neither 0 nor a power of 2"));
@@ -105,6 +72,155 @@ TEST(DataLayout, LayoutStringFormat) {
FailedWithMessage("empty specification is not allowed"));
}
+class DataLayoutPrimitiveSpecificationTest
+ : public DataLayoutTest,
+ public ::testing::WithParamInterface<char> {
+ char Specifier;
+
+public:
+ DataLayoutPrimitiveSpecificationTest() : Specifier(GetParam()) {}
+
+ std::string format(StringRef Str) const {
+ std::string Res = Str.str();
+ std::replace(Res.begin(), Res.end(), '!', Specifier);
+ return Res;
+ }
+};
+
+INSTANTIATE_TEST_SUITE_P(PrmitiveSpecifiers,
+ DataLayoutPrimitiveSpecificationTest,
+ ::testing::Values('i', 'f', 'v'));
+
+TEST_P(DataLayoutPrimitiveSpecificationTest, ParsePrimitiveSpec) {
+ for (StringRef Str : {"!1:16", "!8:8:8", "!16:32:64", "!16777215:8:0",
+ "!16777215:32768:32768"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(format(Str)), Succeeded());
+
+ for (StringRef Str : {"!", "!1", "!32:32:32:32", "!16:32:64:128"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage(format("malformed specification, must be of the form "
+ "\"!<size>:<abi>[:<pref>]\"")));
+
+ // size
+ for (StringRef Str : {"!:8", "!:16:16", "!:32:64"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(format(Str)),
+ FailedWithMessage("size component cannot be empty"));
+
+ for (StringRef Str :
+ {"!0:8", "!0x8:8", "!x:8:8", "!0:16:32", "!16777216:64:64"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage("size must be a non-zero 24-bit integer"));
+
+ // ABI alignment
+ for (StringRef Str : {"!8:", "!16::16", "!32::64"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage("ABI alignment component cannot be empty"));
+
+ for (StringRef Str : {"!1:x", "!8:8x:8", "!16:65536:65536"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage("ABI alignment must be a 16-bit integer"));
+
+ for (StringRef Str : {"!8:0", "!16:0:16", "!32:0:64"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(format(Str)),
+ FailedWithMessage("ABI alignment must be non-zero"));
+
+ for (StringRef Str : {"!1:1", "!8:4", "!16:6:16", "!32:24:64"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage(
+ "ABI alignment must be a power of two times the byte width"));
+
+ // preferred alignment
+ for (StringRef Str : {"!1:8:", "!16:16:", "!64:32:"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage("preferred alignment component cannot be empty"));
+
+ for (StringRef Str : {"!1:8:x", "!8:8:0x8", "!16:32:65536"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage("preferred alignment must be a 16-bit integer"));
+
+ for (StringRef Str : {"!1:8:12", "!8:8:17", "!16:32:40"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage(
+ "preferred alignment must be a power of two times the byte width"));
+
+ for (StringRef Str : {"!1:16:8", "!16:16:0", "!64:32:16"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage(
+ "preferred alignment cannot be less than the ABI alignment"));
+
+ // Additional check for byte-sized integer.
+ if (GetParam() == 'i') {
+ for (StringRef Str : {"!8:16", "!8:16:8", "!8:16:32"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(format(Str)),
+ FailedWithMessage("i8 must be 8-bit aligned"));
+ }
+}
+
+TEST(DataLayoutTest, ParseAggregateSpec) {
+ for (StringRef Str : {"a:8", "a:0:16", "a0:8:0", "a0:32:64", "a:32768:32768"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
+
+ for (StringRef Str : {"a", "a0", "a:32:32:32", "a0:32:64:128"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("malformed specification, must be of the form "
+ "\"a:<abi>[:<pref>]\""));
+
+ // size
+ for (StringRef Str : {"a1:8", "a0x0:8", "ax:16:32"})
+ EXPECT_THAT_EXPECTED(DataLayout::parse(Str),
+ FailedWithMessage("size must be zero"));
+
+ // ABI alignment
+ for (StringRef Str : {"a:", "a0:", "a::32"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("ABI alignment component cannot be empty"));
+
+ for (StringRef Str : {"a:x", "a0:0x0", "a:65536", "a0:65536:65536"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("ABI alignment must be a 16-bit integer"));
+
+ for (StringRef Str : {"a:1", "a:4", "a:9:16", "a0:24: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 : {"a:8:", "a0:16:", "a0:0:"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("preferred alignment component cannot be empty"));
+
+ for (StringRef Str : {"a:16:x", "a0:8:0x8", "a:16:65536"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("preferred alignment must be a 16-bit integer"));
+
+ for (StringRef Str : {"a:8:12", "a:16:17", "a0:32:40"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage(
+ "preferred alignment must be a power of two times the byte width"));
+
+ for (StringRef Str : {"a:16:8", "a:16:0", "a0:32:16"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage(
+ "preferred alignment cannot be less than the ABI alignment"));
+}
+
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",
>From f52d721cafb2c55a42a42cc91c0592c6cf119e38 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Sun, 18 Aug 2024 15:13:31 +0300
Subject: [PATCH 2/4] Remove repeated test
---
llvm/unittests/IR/DataLayoutTest.cpp | 2 --
1 file changed, 2 deletions(-)
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 7049447a423afa..6a373e21cd8774 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -55,8 +55,6 @@ TEST(DataLayoutTest, ParseErrors) {
EXPECT_THAT_EXPECTED(
DataLayout::parse("Fi24"),
FailedWithMessage("Alignment is neither 0 nor a power of 2"));
- EXPECT_THAT_EXPECTED(DataLayout::parse("i8:16"),
- FailedWithMessage("i8 must be 8-bit aligned"));
EXPECT_THAT_EXPECTED(
DataLayout::parse("S24"),
FailedWithMessage("Alignment is neither 0 nor a power of 2"));
>From 83e0e0af570fd2509c4b2615c8d31dfbb73955c8 Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Mon, 19 Aug 2024 08:33:41 +0300
Subject: [PATCH 3/4] Require non-zero preferred alignments
---
llvm/lib/IR/DataLayout.cpp | 10 +++-------
llvm/unittests/IR/DataLayoutTest.cpp | 20 +++++++++++++++-----
2 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/llvm/lib/IR/DataLayout.cpp b/llvm/lib/IR/DataLayout.cpp
index bae516f33f808f..5efdaaccf1fc7f 100644
--- a/llvm/lib/IR/DataLayout.cpp
+++ b/llvm/lib/IR/DataLayout.cpp
@@ -391,7 +391,7 @@ Error DataLayout::parsePrimitiveSpec(StringRef Spec) {
if (Error Err = parseSize(Components[0], BitWidth))
return Err;
- // ABI alignment. Required, cannot be zero.
+ // ABI alignment.
Align ABIAlign;
if (Error Err = parseAlignment(Components[1], ABIAlign, "ABI"))
return Err;
@@ -400,11 +400,9 @@ Error DataLayout::parsePrimitiveSpec(StringRef Spec) {
return createStringError("i8 must be 8-bit aligned");
// Preferred alignment. Optional, defaults to the ABI alignment.
- // Can be zero, meaning use one byte alignment.
Align PrefAlign = ABIAlign;
if (Components.size() > 2)
- if (Error Err = parseAlignment(Components[2], PrefAlign, "preferred",
- /*AllowZero=*/true))
+ if (Error Err = parseAlignment(Components[2], PrefAlign, "preferred"))
return Err;
if (PrefAlign < ABIAlign)
@@ -440,11 +438,9 @@ Error DataLayout::parseAggregateSpec(StringRef Spec) {
return Err;
// Preferred alignment. Optional, defaults to the ABI alignment.
- // Can be zero, meaning use one byte alignment.
Align PrefAlign = ABIAlign;
if (Components.size() > 2)
- if (Error Err = parseAlignment(Components[2], PrefAlign, "preferred",
- /*AllowZero=*/true))
+ if (Error Err = parseAlignment(Components[2], PrefAlign, "preferred"))
return Err;
if (PrefAlign < ABIAlign)
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 6a373e21cd8774..9464b489b65f43 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -90,8 +90,8 @@ INSTANTIATE_TEST_SUITE_P(PrmitiveSpecifiers,
::testing::Values('i', 'f', 'v'));
TEST_P(DataLayoutPrimitiveSpecificationTest, ParsePrimitiveSpec) {
- for (StringRef Str : {"!1:16", "!8:8:8", "!16:32:64", "!16777215:8:0",
- "!16777215:32768:32768"})
+ for (StringRef Str :
+ {"!1:16", "!8:8:8", "!16:32:64", "!16777215:32768:32768"})
EXPECT_THAT_EXPECTED(DataLayout::parse(format(Str)), Succeeded());
for (StringRef Str : {"!", "!1", "!32:32:32:32", "!16:32:64:128"})
@@ -143,13 +143,18 @@ TEST_P(DataLayoutPrimitiveSpecificationTest, ParsePrimitiveSpec) {
DataLayout::parse(format(Str)),
FailedWithMessage("preferred alignment must be a 16-bit integer"));
+ for (StringRef Str : {"!8:8:0", "!32:16:0"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(format(Str)),
+ FailedWithMessage("preferred alignment must be non-zero"));
+
for (StringRef Str : {"!1:8:12", "!8:8:17", "!16:32:40"})
EXPECT_THAT_EXPECTED(
DataLayout::parse(format(Str)),
FailedWithMessage(
"preferred alignment must be a power of two times the byte width"));
- for (StringRef Str : {"!1:16:8", "!16:16:0", "!64:32:16"})
+ for (StringRef Str : {"!1:16:8", "!64:32:16"})
EXPECT_THAT_EXPECTED(
DataLayout::parse(format(Str)),
FailedWithMessage(
@@ -164,7 +169,7 @@ TEST_P(DataLayoutPrimitiveSpecificationTest, ParsePrimitiveSpec) {
}
TEST(DataLayoutTest, ParseAggregateSpec) {
- for (StringRef Str : {"a:8", "a:0:16", "a0:8:0", "a0:32:64", "a:32768:32768"})
+ for (StringRef Str : {"a:8", "a:0:16", "a0:32:64", "a:32768:32768"})
EXPECT_THAT_EXPECTED(DataLayout::parse(Str), Succeeded());
for (StringRef Str : {"a", "a0", "a:32:32:32", "a0:32:64:128"})
@@ -206,13 +211,18 @@ TEST(DataLayoutTest, ParseAggregateSpec) {
DataLayout::parse(Str),
FailedWithMessage("preferred alignment must be a 16-bit integer"));
+ for (StringRef Str : {"a:0:0", "a0:16:0"})
+ EXPECT_THAT_EXPECTED(
+ DataLayout::parse(Str),
+ FailedWithMessage("preferred alignment must be non-zero"));
+
for (StringRef Str : {"a:8:12", "a:16:17", "a0:32:40"})
EXPECT_THAT_EXPECTED(
DataLayout::parse(Str),
FailedWithMessage(
"preferred alignment must be a power of two times the byte width"));
- for (StringRef Str : {"a:16:8", "a:16:0", "a0:32:16"})
+ for (StringRef Str : {"a:16:8", "a0:32:16"})
EXPECT_THAT_EXPECTED(
DataLayout::parse(Str),
FailedWithMessage(
>From e62a8932f4c59425c75e6cbaec1a2935a24a61bf Mon Sep 17 00:00:00 2001
From: Sergei Barannikov <barannikov88 at gmail.com>
Date: Mon, 19 Aug 2024 14:23:17 +0300
Subject: [PATCH 4/4] "fix" mlir test
---
mlir/test/Target/LLVMIR/Import/import-failure.ll | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mlir/test/Target/LLVMIR/Import/import-failure.ll b/mlir/test/Target/LLVMIR/Import/import-failure.ll
index 9c24ed2b75746b..9028559257eb0d 100644
--- a/mlir/test/Target/LLVMIR/Import/import-failure.ll
+++ b/mlir/test/Target/LLVMIR/Import/import-failure.ll
@@ -360,7 +360,7 @@ declare void @llvm.experimental.noalias.scope.decl(metadata)
; // -----
; CHECK: import-failure.ll
-; CHECK-SAME: error: cannot translate data layout: i8:8:8:8
+; CHECK-SAME: error: malformed specification
target datalayout = "e-i8:8:8:8"
; // -----
More information about the Mlir-commits
mailing list