Skip to content

[RISCV] Use hasFeature instead of checkFeature in llvm-exegesis. NFC #131401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 17, 2025

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Mar 14, 2025

Until recently checkFeature was quite slow. #130936

I was curious where we use checkFeature and noticed these. I thought we could use hasFeature instead of going through strings.

Until recently checkFeature was quite slow. llvm#130936

I was curious where we use checkFeature and noticed these. I thought
we could use hasFeature instead of going through strings.
@llvmbot
Copy link
Member

llvmbot commented Mar 14, 2025

@llvm/pr-subscribers-tools-llvm-exegesis

Author: Craig Topper (topperc)

Changes

Until recently checkFeature was quite slow. #130936

I was curious where we use checkFeature and noticed these. I thought we could use hasFeature instead of going through strings.


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

1 Files Affected:

  • (modified) llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp (+27-9)
diff --git a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
index 6d97a7ecfffb8..b29dd8739aa2e 100644
--- a/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
+++ b/llvm/tools/llvm-exegesis/lib/RISCV/Target.cpp
@@ -156,15 +156,33 @@ template <class BaseT> class RISCVSnippetGenerator : public BaseT {
     // FIXME: We could have obtained these two constants from RISCVSubtarget
     // but in order to get that from TargetMachine, we need a Function.
     const MCSubtargetInfo &STI = State.getSubtargetInfo();
-    ELEN = STI.checkFeatures("+zve64x") ? 64 : 32;
-
-    std::string ZvlQuery;
-    for (unsigned Size = 32; Size <= 65536; Size *= 2) {
-      ZvlQuery = "+zvl";
-      raw_string_ostream SS(ZvlQuery);
-      SS << Size << "b";
-      if (STI.checkFeatures(SS.str()) && ZvlVLen < Size)
-        ZvlVLen = Size;
+    ELEN = STI.hasFeature(RISCV::FeatureStdExtZve64x) ? 64 : 32;
+
+    static_assert(RISCV::FeatureStdExtZvl64b == RISCV::FeatureStdExtZvl32b + 1);
+    static_assert(RISCV::FeatureStdExtZvl128b ==
+                  RISCV::FeatureStdExtZvl32b + 2);
+    static_assert(RISCV::FeatureStdExtZvl256b ==
+                  RISCV::FeatureStdExtZvl32b + 3);
+    static_assert(RISCV::FeatureStdExtZvl512b ==
+                  RISCV::FeatureStdExtZvl32b + 4);
+    static_assert(RISCV::FeatureStdExtZvl1024b ==
+                  RISCV::FeatureStdExtZvl32b + 5);
+    static_assert(RISCV::FeatureStdExtZvl2048b ==
+                  RISCV::FeatureStdExtZvl32b + 6);
+    static_assert(RISCV::FeatureStdExtZvl4096b ==
+                  RISCV::FeatureStdExtZvl32b + 7);
+    static_assert(RISCV::FeatureStdExtZvl8192b ==
+                  RISCV::FeatureStdExtZvl32b + 8);
+    static_assert(RISCV::FeatureStdExtZvl16384b ==
+                  RISCV::FeatureStdExtZvl32b + 9);
+    static_assert(RISCV::FeatureStdExtZvl32768b ==
+                  RISCV::FeatureStdExtZvl32b + 10);
+    static_assert(RISCV::FeatureStdExtZvl65536b ==
+                  RISCV::FeatureStdExtZvl32b + 11);
+    for (unsigned Feature = RISCV::FeatureStdExtZvl32b, Size = 32;
+         Size <= 65536; ++Feature, Size *= 2) {
+      if (STI.hasFeature(Feature))
+        ZvlVLen = std::max(ZvlVLen, Size);
     }
   }
 

Copy link
Member

@mshockwave mshockwave left a comment

Choose a reason for hiding this comment

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

LGTM

@topperc topperc merged commit b00ad36 into llvm:main Mar 17, 2025
11 checks passed
@topperc topperc deleted the pr/exegesis branch March 17, 2025 16:05
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.

3 participants