Skip to content

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Apr 21, 2025

Fixes #136564.

@llvmbot
Copy link
Member

llvmbot commented Apr 21, 2025

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

Author: Craig Topper (topperc)

Changes

Fixes #136564.


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

2 Files Affected:

  • (modified) llvm/lib/TargetParser/RISCVISAInfo.cpp (+3)
  • (modified) llvm/unittests/TargetParser/RISCVISAInfoTest.cpp (+10)
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index 524d6dc01e8aa..1e7144ce6d22b 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -789,6 +789,9 @@ Error RISCVISAInfo::checkDependency() {
       return getIncompatibleError("zclsd", "zcf");
   }
 
+  if (XLen != 32 && Exts.count("zilsd") != 0)
+    return getError("'zilsd' is only supported for 'rv32'");
+
   for (auto Ext : XqciExts)
     if (Exts.count(Ext.str()) && (XLen != 32))
       return getError("'" + Twine(Ext) + "'" + " is only supported for 'rv32'");
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index ff0a5e64ab3e1..6e190ad3e7969 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -658,6 +658,16 @@ TEST(ParseArchString, RejectsConflictingExtensions) {
               "'xwchc' and 'zcb' extensions are incompatible");
   }
 
+  for (StringRef Input : {"rv64i_zilsd"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'zilsd' is only supported for 'rv32'");
+  }
+
+  for (StringRef Input : {"rv64i_zclsd"}) {
+    EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
+              "'zclsd' is only supported for 'rv32'");
+  }
+
   for (StringRef Input : {"rv32i_zcf_zclsd"}) {
     EXPECT_EQ(toString(RISCVISAInfo::parseArchString(Input, true).takeError()),
               "'zclsd' and 'zcf' extensions are incompatible");

Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

Commit message should say RV64 otherwise LGTM

Copy link
Member

@lenary lenary left a comment

Choose a reason for hiding this comment

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

LGTM with the suggested title update

@topperc topperc changed the title [RISCV] Report error if Zilsd is used on RV32. [RISCV] Report error if Zilsd is used on RV64. Apr 21, 2025
@topperc topperc merged commit 4c0ea47 into llvm:main Apr 21, 2025
10 of 13 checks passed
@topperc topperc deleted the pr/zilsd branch April 21, 2025 19:26
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
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.

[RISCV] clang mistakenly permits Zilsd extension for rv64
4 participants