[llvm] [RISCV] Refactor and improve RISCVISAInfo::checkDependency() (PR #104658)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

<details>
<summary>Changes</summary>

-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.

---
Full diff: https://github.com/llvm/llvm-project/pull/104658.diff


2 Files Affected:

- (modified) llvm/lib/TargetParser/RISCVISAInfo.cpp (+29-30) 
- (modified) llvm/unittests/TargetParser/RISCVISAInfoTest.cpp (+77) 


``````````diff
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 59df02daf63265..d47305293eee5a 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,32 +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(
-        "'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");
+    return getExtensionRequiresError("zvl*b", "v' or 'zve*");
 
-  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 (!HasVector)
+    for (auto Ext :
+         {"zvbb", "zvkb", "zvkg", "zvkned", "zvknha", "zvksed", "zvksh"})
+      if (Exts.count(Ext))
+        return getExtensionRequiresError(Ext, "v' or 'zve*");
 
-  if (Exts.count("zvknhb") && !Exts.count("zve64x"))
-    return getError(
-        "'zvknhb' requires 'v' or 'zve64*' extension to also be specified");
+  if (!Exts.count("zve64x"))
+    for (auto Ext : {"zvknhb", "zvbc"})
+      if (Exts.count(Ext))
+        return getExtensionRequiresError(Ext, "v' or 'zve64*");
 
   if ((HasZcmt || Exts.count("zcmp")) && HasD && (HasC || Exts.count("zcd")))
     return getError(Twine("'") + (HasZcmt ? "zcmt" : "zcmp") +
@@ -769,23 +771,20 @@ 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 getExtensionRequiresError(Ext, "a' or 'zaamo");
 
   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 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();
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index bede4e64696c55..173e79272b855b 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()),
+              "'zvkb' requires 'v' or 'zve*' extension to also be specified");
+  }
+
+  for (StringRef Input : {"rv32i_zvkg"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'zvkg' requires 'v' or 'zve*' extension to also be specified");
+  }
+
+  for (StringRef Input : {"rv32i_zvkned"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'zvkned' requires 'v' or 'zve*' extension to also be specified");
+  }
+
+  for (StringRef Input : {"rv32i_zvknha"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'zvknha' requires 'v' or 'zve*' extension to also be specified");
+  }
+
+  for (StringRef Input : {"rv32i_zvksed"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'zvksed' requires 'v' or 'zve*' extension to also be specified");
+  }
+
+  for (StringRef Input : {"rv32i_zvksh"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'zvksh' 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) {

``````````

</details>


https://github.com/llvm/llvm-project/pull/104658