Skip to content

Conversation

@llvmbot
Copy link
Member

@llvmbot llvmbot commented Apr 27, 2025

Backport 6c33735

Requested by: @wangpc-pp

@llvmbot
Copy link
Member Author

llvmbot commented Apr 27, 2025

@topperc What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from topperc April 27, 2025 03:20
@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V labels Apr 27, 2025
@llvmbot
Copy link
Member Author

llvmbot commented Apr 27, 2025

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

@llvm/pr-subscribers-clang

Author: None (llvmbot)

Changes

Backport 6c33735

Requested by: @wangpc-pp


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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) llvm/lib/TargetParser/RISCVISAInfo.cpp (+15-3)
  • (modified) llvm/unittests/TargetParser/RISCVISAInfoTest.cpp (+8)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b8f26ec9a5447..47ef2f80ac3f2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1267,6 +1267,8 @@ RISC-V Support
 - The option ``-mcmodel=large`` for the large code model is supported.
 - Bump RVV intrinsic to version 1.0, the spec: https://github.com/riscv-non-isa/rvv-intrinsic-doc/releases/tag/v1.0.0-rc4
 
+- `Zicsr` / `Zifencei` are allowed to be duplicated in the presence of `g` in `-march`.
+
 CUDA/HIP Language Changes
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed a bug about overriding a constexpr pure-virtual member function with a non-constexpr virtual member function which causes compilation failure when including standard C++ header `format`.
diff --git a/llvm/lib/TargetParser/RISCVISAInfo.cpp b/llvm/lib/TargetParser/RISCVISAInfo.cpp
index c78d60fd86b3f..64ec411cb06e1 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"
@@ -718,6 +717,19 @@ RISCVISAInfo::parseArchString(StringRef Arch, bool EnableExperimentalExtension,
     } while (!Ext.empty());
   }
 
+  // We add Zicsr/Zifenci as final to allow duplicated "zicsr"/"zifencei" like
+  // "rv64g_zicsr_zifencei".
+  if (Baseline == 'g') {
+    for (const char *Ext : RISCVGImplicationsZi) {
+      if (ISAInfo->Exts.count(Ext))
+        continue;
+
+      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 7ebfcf915a7c5..5089bc0fd479a 100644
--- a/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
+++ b/llvm/unittests/TargetParser/RISCVISAInfoTest.cpp
@@ -507,6 +507,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()),

@github-project-automation github-project-automation bot moved this from Needs Triage to Needs Merge in LLVM Release Status Apr 27, 2025
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@tstellar tstellar moved this from Needs Merge to Needs Test Fix in LLVM Release Status Apr 29, 2025
@tstellar
Copy link
Collaborator

This has some failing tests.

@wangpc-pp
Copy link
Contributor

This has some failing tests.

The failure is not related to this PR I think, it is about compiler-rt/XRay:

/var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6vh59-1/llvm-project/github-pull-requests/compiler-rt/test/xray/TestCases/Posix/basic-filtering.cpp:57:15: error: TRACE-NOT: excluded string found in input
// TRACE-NOT: - { type: 0, func-id: {{.*}}, function: {{.*filtered.*}}, {{.*}} }
              ^
<stdin>:10:2: note: found here
 - { type: 0, func-id: 1, function: 'filtered()', cpu: 0, thread: 2646461, process: 2646461, kind: function-enter, tsc: 1745726047361388709, data: '' }
 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Input file: <stdin>
Check file: /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-6vh59-1/llvm-project/github-pull-requests/compiler-rt/test/xray/TestCases/Posix/basic-filtering.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
        .
        .
        .
        5:  constant-tsc: true 
        6:  nonstop-tsc: true 
        7:  cycle-frequency: 1000000000 
        8: records: 
        9:  - { type: 0, func-id: 4, function: main, cpu: 0, thread: 2646461, process: 2646461, kind: function-enter, tsc: 1745726047361383559, data: '' } 
       10:  - { type: 0, func-id: 1, function: 'filtered()', cpu: 0, thread: 2646461, process: 2646461, kind: function-enter, tsc: 1745726047361388709, data: '' } 
not:57      !~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  error: no match expected
       11:  - { type: 0, func-id: 1, function: 'filtered()', cpu: 0, thread: 2646461, process: 2646461, kind: function-exit, tsc: 1745726047363841187, data: '' } 
       12:  - { type: 0, func-id: 3, function: 'always_shows()', cpu: 0, thread: 2646461, process: 2646461, kind: function-enter, tsc: 1745726047363842317, data: '' } 
       13:  - { type: 0, func-id: 3, function: 'always_shows()', cpu: 0, thread: 2646461, process: 2646461, kind: function-exit, tsc: 1745726047365914946, data: '' } 
       14:  - { type: 0, func-id: 4, function: main, cpu: 0, thread: 2646461, process: 2646461, kind: function-exit, tsc: 1745726047365916026, data: '' } 
       15: ... 
>>>>>>

@wangpc-pp
Copy link
Contributor

Thanks @tstellar! Now all checks are passed!

This matches GCC and we supported it in LLVM 17/18.

Fixes llvm#136803

(cherry picked from commit 6c33735)
@tstellar tstellar merged commit a708fb7 into llvm:release/20.x May 13, 2025
9 of 10 checks passed
@github-project-automation github-project-automation bot moved this from Needs Test Fix to Done in LLVM Release Status May 13, 2025
@github-actions
Copy link

@wangpc-pp (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

@wangpc-pp
Copy link
Contributor

@wangpc-pp (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

It is in the clang/docs/ReleaseNotes.rst: Zicsr / Zifencei are allowed to be duplicated in the presence of g in -march.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend:RISC-V clang Clang issues not falling into any other category release:note

Projects

Development

Successfully merging this pull request may close these issues.

5 participants