[RISCV] Allow Zicsr/Zifencei to duplicate with g
#136842
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-backend-risc-v Author: Pengcheng Wang (wangpc-pp) ChangesThis matches GCC and we supported it in LLVM 17/18. Fixes #136803 Full diff: https://github.com/llvm/llvm-project/pull/136842.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5ccd346a93b4f..632d8396ed3b6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -632,6 +632,8 @@ RISC-V Support
Qualcomm's `Xqciint` extension to save and restore some GPRs in interrupt
service routines.
+- `Zicsr` / `Zifencei` are allowed to duplicate with `g` in `-march`.
+
CUDA/HIP Language Changes
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index ff0174210f87f..a63ad98bf26a4 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -45,9 +45,8 @@ struct RISCVProfile {
} // end anonymous namespace
-static const char *RISCVGImplications[] = {
- "i", "m", "a", "f", "d", "zicsr", "zifencei"
-};
+static const char *RISCVGImplications[] = {"i", "m", "a", "f", "d"};
+static const char *RISCVGImplicationsZi[] = {+"zicsr", "zifencei"};
#define GET_SUPPORTED_EXTENSIONS
#include "llvm/TargetParser/RISCVTargetParserDef.inc"
@@ -717,6 +716,15 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
} while (!Ext.empty());
}
+ // We add Zicsr/Zifenci as final to allow duplicated "zicsr"/"zifencei".
+ if (Baseline == 'g') {
+ for (const char *Ext : RISCVGImplicationsZi) {
+ auto Version = findDefaultVersion(Ext);
+ assert(Version && "Default extension version not found?");
+ ISAInfo->Exts[std::string(Ext)] = {Version->Major, Version->Minor};
+ }
+ }
+
return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
}
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index 43896fede57d8..0bea138560895 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -512,6 +512,14 @@ TEST(ParseArchString, RejectsDoubleOrTrailingUnderscore) {
}
TEST(ParseArchString, RejectsDuplicateExtensionNames) {
+ // Zicsr/Zifencei are allowed to duplicate with "g".
+ ASSERT_THAT_EXPECTED(RISCVISAInfo::parseArchString("rv64g_zicsr", true),
+ Succeeded());
+ ASSERT_THAT_EXPECTED(RISCVISAInfo::parseArchString("rv64g_zifencei", true),
+ Succeeded());
+ ASSERT_THAT_EXPECTED(
+ RISCVISAInfo::parseArchString("rv64g_zicsr_zifencei", true), Succeeded());
+
EXPECT_EQ(toString(RISCVISAInfo::parseArchString("rv64ii", true).takeError()),
"invalid standard user-level extension 'i'");
EXPECT_EQ(toString(RISCVISAInfo::parseArchString("rv32ee", true).takeError()),
|
|
@llvm/pr-subscribers-clang Author: Pengcheng Wang (wangpc-pp) ChangesThis matches GCC and we supported it in LLVM 17/18. Fixes #136803 Full diff: https://github.com/llvm/llvm-project/pull/136842.diff 3 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5ccd346a93b4f..632d8396ed3b6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -632,6 +632,8 @@ RISC-V Support
Qualcomm's `Xqciint` extension to save and restore some GPRs in interrupt
service routines.
+- `Zicsr` / `Zifencei` are allowed to duplicate with `g` in `-march`.
+
CUDA/HIP Language Changes
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index ff0174210f87f..a63ad98bf26a4 100644
--- a/llvm/lib/TargetParser/RISCVISAInfo.cpp
+++ b/llvm/lib/TargetParser/RISCVISAInfo.cpp
@@ -45,9 +45,8 @@ struct RISCVProfile {
} // end anonymous namespace
-static const char *RISCVGImplications[] = {
- "i", "m", "a", "f", "d", "zicsr", "zifencei"
-};
+static const char *RISCVGImplications[] = {"i", "m", "a", "f", "d"};
+static const char *RISCVGImplicationsZi[] = {+"zicsr", "zifencei"};
#define GET_SUPPORTED_EXTENSIONS
#include "llvm/TargetParser/RISCVTargetParserDef.inc"
@@ -717,6 +716,15 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
} while (!Ext.empty());
}
+ // We add Zicsr/Zifenci as final to allow duplicated "zicsr"/"zifencei".
+ if (Baseline == 'g') {
+ for (const char *Ext : RISCVGImplicationsZi) {
+ auto Version = findDefaultVersion(Ext);
+ assert(Version && "Default extension version not found?");
+ ISAInfo->Exts[std::string(Ext)] = {Version->Major, Version->Minor};
+ }
+ }
+
return RISCVISAInfo::postProcessAndChecking(std::move(ISAInfo));
}
diff --git a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
index 43896fede57d8..0bea138560895 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -512,6 +512,14 @@ TEST(ParseArchString, RejectsDoubleOrTrailingUnderscore) {
}
TEST(ParseArchString, RejectsDuplicateExtensionNames) {
+ // Zicsr/Zifencei are allowed to duplicate with "g".
+ ASSERT_THAT_EXPECTED(RISCVISAInfo::parseArchString("rv64g_zicsr", true),
+ Succeeded());
+ ASSERT_THAT_EXPECTED(RISCVISAInfo::parseArchString("rv64g_zifencei", true),
+ Succeeded());
+ ASSERT_THAT_EXPECTED(
+ RISCVISAInfo::parseArchString("rv64g_zicsr_zifencei", true), Succeeded());
+
EXPECT_EQ(toString(RISCVISAInfo::parseArchString("rv64ii", true).takeError()),
"invalid standard user-level extension 'i'");
EXPECT_EQ(toString(RISCVISAInfo::parseArchString("rv32ee", true).takeError()),
|
wangpc-pp
requested review from
asb,
preames,
lukel97,
kito-cheng,
BeMg and
topperc
This matches GCC and we supported it in LLVM 17/18. Fixes llvm#136803
wangpc-pp
force-pushed
the
main-riscv-march-zicsr-zifencei
branch
from
e133a5f to
723976e
Compare
github-project-automation
bot
moved this from Needs Triage
to Needs Merge
in LLVM Release Status
|
If you're planning on backporting this, should the release notes be added in a separate commit? I'm wondering if you'll end up with issues trying to cherry-pick it otherwise |
topperc
reviewed
Apr 23, 2025
I think we'd typically land a change in the most logical way for the main branch and then handle unpicking the backport manually - it just means the tooling needs a little bit of help but it's not complex to manually drop the release note change. I don't feel strongly given what a minor detail it is, but that would be my default. |
This reverts commit ddd46bf.
|
According to the sync-up meeting, we can proceed this patch as-is. I will merge this in a few days. I added back the release note. @asb |
github-project-automation
bot
moved this from Needs Pull Request
to Done
in LLVM Release Status
|
/cherry-pick 6c33735 |
|
/pull-request #137490 |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/4224 Here is the relevant piece of the build log for the reference |
jyli0116
pushed a commit
to jyli0116/llvm-project
that referenced
this pull request
Apr 28, 2025
This matches GCC and we supported it in LLVM 17/18. Fixes llvm#136803
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This matches GCC and we supported it in LLVM 17/18. Fixes llvm#136803
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This matches GCC and we supported it in LLVM 17/18. Fixes llvm#136803
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
This matches GCC and we supported it in LLVM 17/18. Fixes llvm#136803
|
This matter was addressed at the RVI Toolchain SIG meeting on 05/08/25. It was concluded that the duplication of implied extensions, such as in the cases of G and Zifencei, and F and Zicsr, can be silently disregarded. Both GCC and LLVM toolchains are consistent with this approach through the application of this LLVM patch. However, the duplication of unrelated extensions should result in an error message indicating an invalid duplicated extension, which is already implemented in both toolchains. |
Ankur-0429
pushed a commit
to Ankur-0429/llvm-project
that referenced
this pull request
May 9, 2025
This matches GCC and we supported it in LLVM 17/18. Fixes llvm#136803
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
May 13, 2025
This matches GCC and we supported it in LLVM 17/18. Fixes llvm#136803 (cherry picked from commit 6c33735)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This matches GCC and we supported it in LLVM 17/18.
Fixes #136803