Skip to content

Conversation

@topperc
Copy link
Collaborator

@topperc topperc commented Mar 11, 2025

I was reviewing encodings to put the disassembling of vendor instructions after after standard instructions and found that these overlap with c.fldsp and c.fsdsp.

…+D or Zcd.

I was reviewing encodings to put the disassembling of vendor instructions
after after standard instructions and found that these overlap with
c.fldsp and c.fsdsp.
@llvmbot
Copy link
Member

llvmbot commented Mar 11, 2025

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

Author: Craig Topper (topperc)

Changes

I was reviewing encodings to put the disassembling of vendor instructions after after standard instructions and found that these overlap with c.fldsp and c.fsdsp.


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

2 Files Affected:

  • (modified) llvm/lib/TargetParser/RISCVISAInfo.cpp (+13-8)
  • (modified) llvm/unittests/TargetParser/RISCVISAInfoTest.cpp (+24)
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index dd74f79f04b92..4276e94ec9b3e 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -740,13 +740,16 @@ Error RISCVISAInfo::checkDependency() {
   bool HasZfinx = Exts.count("zfinx") != 0;
   bool HasVector = Exts.count("zve32x") != 0;
   bool HasZvl = MinVLen != 0;
-  bool HasZcmt = Exts.count("zcmt") != 0;
+  bool HasZcmp = Exts.count("zcmp") != 0;
+  bool HasXqccmp = Exts.count("xqccmp") != 0;
+
   static constexpr StringLiteral XqciExts[] = {
       {"xqcia"},   {"xqciac"}, {"xqcibm"},  {"xqcicli"},
       {"xqcicm"},  {"xqcics"}, {"xqcicsr"}, {"xqciint"},
       {"xqcilia"}, {"xqcilo"}, {"xqcilsm"}, {"xqcisls"}};
-  bool HasZcmp = Exts.count("zcmp") != 0;
-  bool HasXqccmp = Exts.count("xqccmp") != 0;
+  static constexpr StringLiteral ZcdOverlaps[] = {
+    {"zcmt"}, {"zcmp"}, {"xqciac"}, {"xqcicm"}
+  };
 
   if (HasI && HasE)
     return getIncompatibleError("i", "e");
@@ -757,11 +760,13 @@ Error RISCVISAInfo::checkDependency() {
   if (HasZvl && !HasVector)
     return getExtensionRequiresError("zvl*b", "v' or 'zve*");
 
-  if ((HasZcmt || Exts.count("zcmp")) && HasD && (HasC || Exts.count("zcd")))
-    return getError(Twine("'") + (HasZcmt ? "zcmt" : "zcmp") +
-                    "' extension is incompatible with '" +
-                    (HasC ? "c" : "zcd") +
-                    "' extension when 'd' extension is enabled");
+  if (HasD && (HasC || Exts.count("zcd")))
+    for (auto Ext : ZcdOverlaps)
+      if (Exts.count(Ext.str()))
+        return getError(Twine("'") + Ext +
+                        "' extension is incompatible with '" +
+                        (HasC ? "c" : "zcd") +
+                        "' extension when 'd' extension is enabled");
 
   if (XLen != 32 && Exts.count("zcf"))
     return getError("'zcf' is only supported for 'rv32'");
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index e24b2d47f6bc4..c86108193fd9a 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -663,6 +663,30 @@ TEST(ParseArchString, RejectsConflictingExtensions) {
         ::testing::EndsWith(" is only supported for 'rv32'"));
   }
 
+  for (StringRef Input : {"rv32idc_xqciac0p3"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'xqciac' extension is incompatible with 'c' extension when 'd' "
+              "extension is enabled");
+  }
+
+  for (StringRef Input : {"rv32i_zcd_xqciac0p3"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'xqciac' extension is incompatible with 'zcd' extension when 'd' "
+              "extension is enabled");
+  }
+
+  for (StringRef Input : {"rv32idc_xqcicm0p2"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'xqcicm' extension is incompatible with 'c' extension when 'd' "
+              "extension is enabled");
+  }
+
+  for (StringRef Input : {"rv32i_zcd_xqcicm0p2"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'xqcicm' extension is incompatible with 'zcd' extension when 'd' "
+              "extension is enabled");
+  }
+
   for (StringRef Input : {"rv32i_zcmp_xqccmp0p1", "rv64i_zcmp_xqccmp0p1"}) {
     EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
               "'zcmp' and 'xqccmp' extensions are incompatible");

@github-actions
Copy link

github-actions bot commented Mar 11, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@topperc topperc changed the title [RISCV] Add an error that Xqciac and Xqcicm are not compatible with C+D or Zcd. [RISCV] Add an error that Xqccmp, Xqciac, and Xqcicm are not compatible with C+D or Zcd. Mar 11, 2025
Copy link
Contributor

@hchandel hchandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks

@topperc topperc merged commit 90a8322 into llvm:main Mar 12, 2025
11 checks passed
@topperc topperc deleted the pr/xqc-zcd-overlap branch March 12, 2025 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants