[llvm] [RISCV] Refactor and improve RISCVISAInfo::checkDependency() (PR #104658)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 16 19:03:15 PDT 2024
https://github.com/topperc created https://github.com/llvm/llvm-project/pull/104658
-Merge some ISA error reporting together and make some errors more precise.
-Add helper functions to exploit similarity of some error strings.
-Make extension names lower case in error messages.
>From 13a465f876909ede1b6ca2b54c85b1ebc8e10f8d Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 16 Aug 2024 17:14:57 -0700
Subject: [PATCH 1/4] [RISCV] Add more tests for
RISCVISAInfo::checkDependency(). NFC
---
.../TargetParser/RISCVISAInfoTest.cpp | 77 +++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index bede4e64696c55..b57fe73d344a41 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -620,6 +620,83 @@ TEST(ParseArchString, RejectsConflictingExtensions) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
"'zcf' is only supported for 'rv32'");
}
+
+ for (StringRef Input : {"rv64i_xwchc" }) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'Xwchc' is only supported for 'rv32'");
+ }
+
+ for (StringRef Input : {"rv32id_xwchc"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'D' and 'Xwchc' extensions are incompatible");
+ }
+
+ for (StringRef Input : {"rv32i_zcb_xwchc"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'Xwchc' and 'Zcb' extensions are incompatible");
+ }
+}
+
+TEST(ParseArchString, MissingDepency) {
+ for (StringRef Input : {"rv32i_zvl32b", "rv64i_zvl128b"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvl*b' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvbb"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvbb' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvbc"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvbc' requires 'v' or 'zve64*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvkb"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvkg"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvkned"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvknha"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvksed"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvksh"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zvknhb"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zvknhb' requires 'v' or 'zve64*' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zacas1p0"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zacas' requires 'a' or 'zaamo' extension to also be specified");
+ }
+
+ for (StringRef Input : {"rv32i_zabha"}) {
+ EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+ "'zabha' requires 'a' or 'zaamo' extension to also be specified");
+ }
}
TEST(ParseArchString, RejectsUnrecognizedProfileNames) {
>From 8479fe66bf5eb42974400d31b047234de056b78f Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 16 Aug 2024 14:20:49 -0700
Subject: [PATCH 2/4] [RISCV] Merge some ISA error reporting together and make
some errors more precise.
Loop over the extension names that have the same error message.
Print the name of Zvk* extensions instead of 'zvk*'.
---
llvm/lib/TargetParser/RISCVISAInfo.cpp | 50 +++++++++----------
.../TargetParser/RISCVISAInfoTest.cpp | 12 ++---
2 files changed, 29 insertions(+), 33 deletions(-)
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 59df02daf63265..3e1d26af0d238d 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -739,26 +739,23 @@ Error RISCVISAInfo::checkDependency() {
return getError("'f' and 'zfinx' extensions are incompatible");
if (HasZvl && !HasVector)
- return getError(
- "'zvl*b' requires 'v' or 'zve*' extension to also be specified");
-
- if (Exts.count("zvbb") && !HasVector)
- return getError(
- "'zvbb' requires 'v' or 'zve*' extension to also be specified");
-
- if (Exts.count("zvbc") && !Exts.count("zve64x"))
- return getError(
- "'zvbc' requires 'v' or 'zve64*' extension to also be specified");
-
- if ((Exts.count("zvkb") || Exts.count("zvkg") || Exts.count("zvkned") ||
- Exts.count("zvknha") || Exts.count("zvksed") || Exts.count("zvksh")) &&
- !HasVector)
- return getError(
- "'zvk*' requires 'v' or 'zve*' extension to also be specified");
-
- if (Exts.count("zvknhb") && !Exts.count("zve64x"))
- return getError(
- "'zvknhb' requires 'v' or 'zve64*' extension to also be specified");
+ return getError(Twine("'") + "zvl*b" +
+ "' requires 'v' or 'zve*' extension to also be specified");
+
+ if (!HasVector)
+ for (auto Ext :
+ {"zvbb", "zvkb", "zvkg", "zvkned", "zvknha", "zvksed", "zvksh"})
+ if (Exts.count(Ext))
+ return getError(
+ Twine("'") + Ext +
+ "' requires 'v' or 'zve*' extension to also be specified");
+
+ if (!Exts.count("zve64x"))
+ for (auto Ext : {"zvknhb", "zvbc"})
+ if (Exts.count(Ext))
+ return getError(
+ Twine("'") + Ext +
+ "' requires 'v' or 'zve64*' extension to also be specified");
if ((HasZcmt || Exts.count("zcmp")) && HasD && (HasC || Exts.count("zcd")))
return getError(Twine("'") + (HasZcmt ? "zcmt" : "zcmp") +
@@ -769,13 +766,12 @@ Error RISCVISAInfo::checkDependency() {
if (XLen != 32 && Exts.count("zcf"))
return getError("'zcf' is only supported for 'rv32'");
- if (Exts.count("zacas") && !(Exts.count("a") || Exts.count("zaamo")))
- return getError(
- "'zacas' requires 'a' or 'zaamo' extension to also be specified");
-
- if (Exts.count("zabha") && !(Exts.count("a") || Exts.count("zaamo")))
- return getError(
- "'zabha' requires 'a' or 'zaamo' extension to also be specified");
+ if (!(Exts.count("a") || Exts.count("zaamo")))
+ for (auto Ext : {"zacas", "zabha"})
+ if (Exts.count(Ext))
+ return getError(
+ Twine("'") + Ext +
+ "' requires 'a' or 'zaamo' extension to also be specified");
if (Exts.count("xwchc") != 0) {
if (XLen != 32)
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index b57fe73d344a41..f812394de2995f 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -655,32 +655,32 @@ TEST(ParseArchString, MissingDepency) {
for (StringRef Input : {"rv32i_zvkb"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ "'zvkb' requires 'v' or 'zve*' extension to also be specified");
}
for (StringRef Input : {"rv32i_zvkg"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ "'zvkg' requires 'v' or 'zve*' extension to also be specified");
}
for (StringRef Input : {"rv32i_zvkned"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ "'zvkned' requires 'v' or 'zve*' extension to also be specified");
}
for (StringRef Input : {"rv32i_zvknha"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ "'zvknha' requires 'v' or 'zve*' extension to also be specified");
}
for (StringRef Input : {"rv32i_zvksed"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ "'zvksed' requires 'v' or 'zve*' extension to also be specified");
}
for (StringRef Input : {"rv32i_zvksh"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'zvk*' requires 'v' or 'zve*' extension to also be specified");
+ "'zvksh' requires 'v' or 'zve*' extension to also be specified");
}
for (StringRef Input : {"rv32i_zvknhb"}) {
>From ee1eb888dae1eb27bde6be8c177eea443027a9d2 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 16 Aug 2024 16:06:28 -0700
Subject: [PATCH 3/4] [RISCV] Add helper functions to exploit similarity of
some error strings. NFC
---
llvm/lib/TargetParser/RISCVISAInfo.cpp | 33 ++++++++++++++------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 3e1d26af0d238d..7644df1fe8a404 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -721,6 +721,16 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
}
+static Error getIncompatibleError(StringRef Ext1, StringRef Ext2) {
+ return getError("'" + Ext1 + "' and '" + Ext2 +
+ "' extensions are incompatible");
+}
+
+static Error getExtensionRequiresError(StringRef Ext, StringRef ReqExt) {
+ return getError("'" + Ext + "' requires '" + ReqExt +
+ "' extension to also be specified");
+}
+
Error RISCVISAInfo::checkDependency() {
bool HasE = Exts.count("e") != 0;
bool HasI = Exts.count("i") != 0;
@@ -733,29 +743,24 @@ Error RISCVISAInfo::checkDependency() {
bool HasZcmt = Exts.count("zcmt") != 0;
if (HasI && HasE)
- return getError("'I' and 'E' extensions are incompatible");
+ return getIncompatibleError("I", "E");
if (HasF && HasZfinx)
- return getError("'f' and 'zfinx' extensions are incompatible");
+ return getIncompatibleError("f", "zfinx");
if (HasZvl && !HasVector)
- return getError(Twine("'") + "zvl*b" +
- "' requires 'v' or 'zve*' extension to also be specified");
+ return getExtensionRequiresError("zvl*b", "v' or 'zve*");
if (!HasVector)
for (auto Ext :
{"zvbb", "zvkb", "zvkg", "zvkned", "zvknha", "zvksed", "zvksh"})
if (Exts.count(Ext))
- return getError(
- Twine("'") + Ext +
- "' requires 'v' or 'zve*' extension to also be specified");
+ return getExtensionRequiresError(Ext, "v' or 'zve*");
if (!Exts.count("zve64x"))
for (auto Ext : {"zvknhb", "zvbc"})
if (Exts.count(Ext))
- return getError(
- Twine("'") + Ext +
- "' requires 'v' or 'zve64*' extension to also be specified");
+ return getExtensionRequiresError(Ext, "v' or 'zve64*");
if ((HasZcmt || Exts.count("zcmp")) && HasD && (HasC || Exts.count("zcd")))
return getError(Twine("'") + (HasZcmt ? "zcmt" : "zcmp") +
@@ -769,19 +774,17 @@ Error RISCVISAInfo::checkDependency() {
if (!(Exts.count("a") || Exts.count("zaamo")))
for (auto Ext : {"zacas", "zabha"})
if (Exts.count(Ext))
- return getError(
- Twine("'") + Ext +
- "' requires 'a' or 'zaamo' extension to also be specified");
+ return getExtensionRequiresError(Ext, "a' or 'zaamo");
if (Exts.count("xwchc") != 0) {
if (XLen != 32)
return getError("'Xwchc' is only supported for 'rv32'");
if (HasD)
- return getError("'D' and 'Xwchc' extensions are incompatible");
+ return getIncompatibleError("D", "Xwchc");
if (Exts.count("zcb") != 0)
- return getError("'Xwchc' and 'Zcb' extensions are incompatible");
+ return getIncompatibleError("Xwchc", "Zcb");
}
return Error::success();
>From 1e1f7dfcd0cbeeb129621f78eecde53b9aa647f8 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 16 Aug 2024 19:00:25 -0700
Subject: [PATCH 4/4] [RISCV] Make extension names lower case in error
messages.
---
llvm/lib/TargetParser/RISCVISAInfo.cpp | 8 ++++----
llvm/unittests/TargetParser/RISCVISAInfoTest.cpp | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 7644df1fe8a404..d47305293eee5a 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -743,7 +743,7 @@ Error RISCVISAInfo::checkDependency() {
bool HasZcmt = Exts.count("zcmt") != 0;
if (HasI && HasE)
- return getIncompatibleError("I", "E");
+ return getIncompatibleError("i", "e");
if (HasF && HasZfinx)
return getIncompatibleError("f", "zfinx");
@@ -778,13 +778,13 @@ Error RISCVISAInfo::checkDependency() {
if (Exts.count("xwchc") != 0) {
if (XLen != 32)
- return getError("'Xwchc' is only supported for 'rv32'");
+ return getError("'xwchc' is only supported for 'rv32'");
if (HasD)
- return getIncompatibleError("D", "Xwchc");
+ return getIncompatibleError("d", "xwchc");
if (Exts.count("zcb") != 0)
- return getIncompatibleError("Xwchc", "Zcb");
+ return getIncompatibleError("xwchc", "zcb");
}
return Error::success();
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index f812394de2995f..173e79272b855b 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -623,17 +623,17 @@ TEST(ParseArchString, RejectsConflictingExtensions) {
for (StringRef Input : {"rv64i_xwchc" }) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'Xwchc' is only supported for 'rv32'");
+ "'xwchc' is only supported for 'rv32'");
}
for (StringRef Input : {"rv32id_xwchc"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'D' and 'Xwchc' extensions are incompatible");
+ "'d' and 'xwchc' extensions are incompatible");
}
for (StringRef Input : {"rv32i_zcb_xwchc"}) {
EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
- "'Xwchc' and 'Zcb' extensions are incompatible");
+ "'xwchc' and 'zcb' extensions are incompatible");
}
}
More information about the llvm-commits
mailing list