[RISCV][NFC] Make generated intrinsic records more human-readable #133710
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 @llvm/pr-subscribers-clang Author: Pengcheng Wang (wangpc-pp) ChangesWe add comment markers and print enum names instead of numbers. For required extensions, we print the feature list instead of raw Full diff: https://github.com/llvm/llvm-project/pull/133710.diff 4 Files Affected:
diff --git a/clang/include/clang/Support/RISCVVIntrinsicUtils.h b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
index 8f2a4f54a1b7f..dd0817f225258 100644
--- a/clang/include/clang/Support/RISCVVIntrinsicUtils.h
+++ b/clang/include/clang/Support/RISCVVIntrinsicUtils.h
@@ -11,6 +11,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/BitmaskEnum.h"
+#include "llvm/ADT/Bitset.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include <cstdint>
@@ -376,6 +377,8 @@ enum PolicyScheme : uint8_t {
HasPolicyOperand,
};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, enum PolicyScheme PS);
+
// TODO refactor RVVIntrinsic class design after support all intrinsic
// combination. This represents an instantiation of an intrinsic with a
// particular type and prototype
@@ -507,6 +510,23 @@ enum RVVRequire {
RVV_REQ_NUM,
};
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, enum RVVRequire Require);
+
+struct RequiredExtensions {
+ llvm::Bitset<RVV_REQ_NUM> Bits;
+ RequiredExtensions() {}
+ RequiredExtensions(std::initializer_list<RVVRequire> Init) {
+ for (auto I : Init)
+ Bits.set(I);
+ }
+
+ void set(unsigned I) { Bits.set(I); }
+ bool operator[](unsigned I) const { return Bits[I]; }
+};
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const RequiredExtensions &Exts);
+
// Raw RVV intrinsic info, used to expand later.
// This struct is highly compact for minimized code size.
struct RVVIntrinsicRecord {
@@ -518,7 +538,7 @@ struct RVVIntrinsicRecord {
const char *OverloadedName;
// Required target features for this intrinsic.
- uint32_t RequiredExtensions[(RVV_REQ_NUM + 31) / 32];
+ RequiredExtensions RequiredExtensions;
// Prototype for this intrinsic, index of RVVSignatureTable.
uint16_t PrototypeIndex;
diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp
index 746609604d1ba..b9f843b1920a1 100644
--- a/clang/lib/Sema/SemaRISCV.cpp
+++ b/clang/lib/Sema/SemaRISCV.cpp
@@ -232,8 +232,7 @@ void RISCVIntrinsicManagerImpl::ConstructRVVIntrinsics(
for (auto &Record : Recs) {
// Check requirements.
if (llvm::any_of(FeatureCheckList, [&](const auto &Item) {
- return ((Record.RequiredExtensions[Item.second / 32] &
- (1U << (Item.second % 32))) != 0) &&
+ return Record.RequiredExtensions[Item.second] &&
!TI.hasFeature(Item.first);
}))
continue;
diff --git a/clang/lib/Support/RISCVVIntrinsicUtils.cpp b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
index e44fbb0181830..2b809a33037da 100644
--- a/clang/lib/Support/RISCVVIntrinsicUtils.cpp
+++ b/clang/lib/Support/RISCVVIntrinsicUtils.cpp
@@ -1196,36 +1196,93 @@ SmallVector<PrototypeDescriptor> parsePrototypes(StringRef Prototypes) {
return PrototypeDescriptors;
}
+#define STRINGIFY(NAME) \
+ case NAME: \
+ OS << #NAME; \
+ break;
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, enum PolicyScheme PS) {
+ switch (PS) {
+ STRINGIFY(SchemeNone)
+ STRINGIFY(HasPassthruOperand)
+ STRINGIFY(HasPolicyOperand)
+ }
+ return OS;
+}
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, enum RVVRequire Require) {
+ switch (Require) {
+ STRINGIFY(RVV_REQ_RV64)
+ STRINGIFY(RVV_REQ_Zvfhmin)
+ STRINGIFY(RVV_REQ_Xsfvcp)
+ STRINGIFY(RVV_REQ_Xsfvfnrclipxfqf)
+ STRINGIFY(RVV_REQ_Xsfvfwmaccqqq)
+ STRINGIFY(RVV_REQ_Xsfvqmaccdod)
+ STRINGIFY(RVV_REQ_Xsfvqmaccqoq)
+ STRINGIFY(RVV_REQ_Zvbb)
+ STRINGIFY(RVV_REQ_Zvbc)
+ STRINGIFY(RVV_REQ_Zvkb)
+ STRINGIFY(RVV_REQ_Zvkg)
+ STRINGIFY(RVV_REQ_Zvkned)
+ STRINGIFY(RVV_REQ_Zvknha)
+ STRINGIFY(RVV_REQ_Zvknhb)
+ STRINGIFY(RVV_REQ_Zvksed)
+ STRINGIFY(RVV_REQ_Zvksh)
+ STRINGIFY(RVV_REQ_Zvfbfwma)
+ STRINGIFY(RVV_REQ_Zvfbfmin)
+ STRINGIFY(RVV_REQ_Zvfh)
+ STRINGIFY(RVV_REQ_Experimental)
+ default:
+ break;
+ }
+ return OS;
+}
+
+#undef STRINGIFY
+
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS,
+ const RequiredExtensions &Exts) {
+ OS << "{";
+ const char *Sep = "";
+ for (unsigned I = 0; I < RVV_REQ_NUM; I++) {
+ if (Exts[I]) {
+ OS << Sep << static_cast<RVVRequire>(I);
+ Sep = ", ";
+ }
+ }
+ OS << "}";
+ return OS;
+}
+
raw_ostream &operator<<(raw_ostream &OS, const RVVIntrinsicRecord &Record) {
OS << "{";
- OS << "\"" << Record.Name << "\",";
+ OS << "/*Name=*/\"" << Record.Name << "\", ";
if (Record.OverloadedName == nullptr ||
StringRef(Record.OverloadedName).empty())
- OS << "nullptr,";
+ OS << "/*OverloadedName=*/nullptr, ";
else
- OS << "\"" << Record.OverloadedName << "\",";
- OS << "{";
- for (uint32_t Exts : Record.RequiredExtensions)
- OS << Exts << ',';
- OS << "},";
- OS << Record.PrototypeIndex << ",";
- OS << Record.SuffixIndex << ",";
- OS << Record.OverloadedSuffixIndex << ",";
- OS << (int)Record.PrototypeLength << ",";
- OS << (int)Record.SuffixLength << ",";
- OS << (int)Record.OverloadedSuffixSize << ",";
- OS << (int)Record.TypeRangeMask << ",";
- OS << (int)Record.Log2LMULMask << ",";
- OS << (int)Record.NF << ",";
- OS << (int)Record.HasMasked << ",";
- OS << (int)Record.HasVL << ",";
- OS << (int)Record.HasMaskedOffOperand << ",";
- OS << (int)Record.HasTailPolicy << ",";
- OS << (int)Record.HasMaskPolicy << ",";
- OS << (int)Record.HasFRMRoundModeOp << ",";
- OS << (int)Record.IsTuple << ",";
- OS << (int)Record.UnMaskedPolicyScheme << ",";
- OS << (int)Record.MaskedPolicyScheme << ",";
+ OS << "/*OverloadedName=*/\"" << Record.OverloadedName << "\", ";
+ OS << "/*RequiredExtensions=*/" << Record.RequiredExtensions << ", ";
+ OS << "/*PrototypeIndex=*/" << Record.PrototypeIndex << ", ";
+ OS << "/*SuffixIndex=*/" << Record.SuffixIndex << ", ";
+ OS << "/*OverloadedSuffixIndex=*/" << Record.OverloadedSuffixIndex << ", ";
+ OS << "/*PrototypeLength=*/" << (int)Record.PrototypeLength << ", ";
+ OS << "/*SuffixLength=*/" << (int)Record.SuffixLength << ", ";
+ OS << "/*OverloadedSuffixSize=*/" << (int)Record.OverloadedSuffixSize << ", ";
+ OS << "/*TypeRangeMask=*/" << (int)Record.TypeRangeMask << ", ";
+ OS << "/*Log2LMULMask=*/" << (int)Record.Log2LMULMask << ", ";
+ OS << "/*NF=*/" << (int)Record.NF << ", ";
+ OS << "/*HasMasked=*/" << (int)Record.HasMasked << ", ";
+ OS << "/*HasVL=*/" << (int)Record.HasVL << ", ";
+ OS << "/*HasMaskedOffOperand=*/" << (int)Record.HasMaskedOffOperand << ", ";
+ OS << "/*HasTailPolicy=*/" << (int)Record.HasTailPolicy << ", ";
+ OS << "/*HasMaskPolicy=*/" << (int)Record.HasMaskPolicy << ", ";
+ OS << "/*HasFRMRoundModeOp=*/" << (int)Record.HasFRMRoundModeOp << ", ";
+ OS << "/*IsTuple=*/" << (int)Record.IsTuple << ", ";
+ OS << "/*UnMaskedPolicyScheme=*/" << (PolicyScheme)Record.UnMaskedPolicyScheme
+ << ", ";
+ OS << "/*MaskedPolicyScheme=*/" << (PolicyScheme)Record.MaskedPolicyScheme
+ << ", ";
OS << "},\n";
return OS;
}
diff --git a/clang/utils/TableGen/RISCVVEmitter.cpp b/clang/utils/TableGen/RISCVVEmitter.cpp
index 02e5e51f6d095..b264fb9d822ef 100644
--- a/clang/utils/TableGen/RISCVVEmitter.cpp
+++ b/clang/utils/TableGen/RISCVVEmitter.cpp
@@ -45,7 +45,7 @@ struct SemaRecord {
unsigned Log2LMULMask;
// Required extensions for this intrinsic.
- uint32_t RequiredExtensions[(RVV_REQ_NUM + 31) / 32];
+ RequiredExtensions RequiredExtensions;
// Prototype for this intrinsic.
SmallVector<PrototypeDescriptor> Prototype;
@@ -769,7 +769,6 @@ void RVVEmitter::createRVVIntrinsics(
SR.Log2LMULMask = Log2LMULMask;
- memset(SR.RequiredExtensions, 0, sizeof(SR.RequiredExtensions));
for (auto RequiredFeature : RequiredFeatures) {
unsigned RequireExt =
StringSwitch<RVVRequire>(RequiredFeature)
@@ -793,7 +792,7 @@ void RVVEmitter::createRVVIntrinsics(
.Case("Zvfbfmin", RVV_REQ_Zvfbfmin)
.Case("Zvfh", RVV_REQ_Zvfh)
.Case("Experimental", RVV_REQ_Experimental);
- SR.RequiredExtensions[RequireExt / 32] |= 1U << (RequireExt % 32);
+ SR.RequiredExtensions.set(RequireExt);
}
SR.NF = NF;
@@ -837,8 +836,7 @@ void RVVEmitter::createRVVIntrinsicRecords(std::vector<RVVIntrinsicRecord> &Out,
R.PrototypeLength = SR.Prototype.size();
R.SuffixLength = SR.Suffix.size();
R.OverloadedSuffixSize = SR.OverloadedSuffix.size();
- memcpy(R.RequiredExtensions, SR.RequiredExtensions,
- sizeof(R.RequiredExtensions));
+ R.RequiredExtensions = SR.RequiredExtensions;
R.TypeRangeMask = SR.TypeRangeMask;
R.Log2LMULMask = SR.Log2LMULMask;
R.NF = SR.NF;
|
|
Examples: {/*Name=*/"vbrev_v", /*OverloadedName=*/"vbrev", /*RequiredExtensions=*/{RVV_REQ_Zvbb}, /*PrototypeIndex=*/417, /*SuffixIndex=*/47, /*OverloadedSuffixIndex=*/0, /*PrototypeLength=*/2, /*SuffixLength=*/1, /*OverloadedSuffixSize=*/0, /*TypeRangeMask=*/15, /*Log2LMULMask=*/127, /*NF=*/1, /*HasMasked=*/1, /*HasVL=*/1, /*HasMaskedOffOperand=*/1, /*HasTailPolicy=*/1, /*HasMaskPolicy=*/1, /*HasFRMRoundModeOp=*/0, /*IsTuple=*/0, /*UnMaskedPolicyScheme=*/HasPassthruOperand, /*MaskedPolicyScheme=*/HasPolicyOperand, },
{/*Name=*/"sf_vc_x_se", /*OverloadedName=*/"sf_vc_x_se", /*RequiredExtensions=*/{RVV_REQ_RV64, RVV_REQ_Xsfvcp}, /*PrototypeIndex=*/167, /*SuffixIndex=*/0, /*OverloadedSuffixIndex=*/0, /*PrototypeLength=*/7, /*SuffixLength=*/0, /*OverloadedSuffixSize=*/0, /*TypeRangeMask=*/8, /*Log2LMULMask=*/8, /*NF=*/1, /*HasMasked=*/0, /*HasVL=*/1, /*HasMaskedOffOperand=*/1, /*HasTailPolicy=*/1, /*HasMaskPolicy=*/1, /*HasFRMRoundModeOp=*/0, /*IsTuple=*/0, /*UnMaskedPolicyScheme=*/SchemeNone, /*MaskedPolicyScheme=*/HasPolicyOperand, }, |
kito-cheng
reviewed
Mar 31, 2025
|
ping. |
|
Does this have any effect on the build time of the compiler? This file is already large and I assume this significantly increases the size. |
topperc
reviewed
Apr 11, 2025
Yes, the .inc size is 6 times larger. I don't know how to assess the impact, theoretically the time to read file and tokenize will increase but I think the impact won't be large because the file is only about 1.5 MB and the comments will be discarded after tokenizer. Do we really care how long it takes to compile the compiler? |
Compile of the compiler does get brought up sometimes. Here's a recent example #132252 (comment) |
I guess its not as bad as the 34MB RISCVGenInstrInfo.inc so maybe it won't be a problem. |
We add comment markers and print enum names instead of numbers. For required extensions, we print the feature list instead of raw bits.
wangpc-pp
force-pushed
the
main-riscv-clang-intrinsic-readable
branch
from
c1641cc to
2143e37
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/203/builds/7536 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/205/builds/6327 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/10/builds/3389 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/204/builds/6349 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/181/builds/17515 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/163/builds/17069 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/133/builds/14454 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/174/builds/16139 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/140/builds/21113 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/152/builds/1769 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/8/builds/13936 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/3/builds/14487 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/17448 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/144/builds/22657 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/7561 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/8576 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/10849 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/7/builds/13381 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/99/builds/5910 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/105/builds/8316 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/75/builds/6187 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/95/builds/11961 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/28/builds/8361 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/158/builds/7856 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/97/builds/6061 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/10398 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/118/builds/5768 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/76/builds/8722 Here is the relevant piece of the build log for the reference |
|
@wangpc-pp you might want to revert this or fix it soon. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/45/builds/11119 Here is the relevant piece of the build log for the reference |
Reverted. Thanks! |
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…vm#133710) We add comment markers and print enum names instead of numbers. For required extensions, we print the feature list instead of raw bits.
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…able (llvm#133710)" This reverts commit d0cf5cd. Error: "declaration of ‘clang::RISCV::RequiredExtensions {anonymous}::SemaRecord::RequiredExtensions’ changes meaning of ‘RequiredExtensions’ [-fpermissive]"
var-const
pushed a commit
to ldionne/llvm-project
that referenced
this pull request
Apr 17, 2025
…vm#133710) We add comment markers and print enum names instead of numbers. For required extensions, we print the feature list instead of raw bits. This recommits d0cf5cd which was reverted by 21ff45d.
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.
We add comment markers and print enum names instead of numbers.
For required extensions, we print the feature list instead of raw
bits.