craig.topper added inline comments.
================
Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:33
+ bool operator!=(const RISCVExtensionVersion &Version) const {
+ return !operator==(Version);
+ }
----------------
Use `!(*this == Version)`
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:154
-static Optional<RISCVExtensionVersion> isExperimentalExtension(StringRef Ext) {
- auto ExtIterator =
- llvm::find_if(SupportedExperimentalExtensions, FindByName(Ext));
- if (ExtIterator == std::end(SupportedExperimentalExtensions))
- return None;
+static std::vector<RISCVExtensionVersion> isExperimentalExtension(StringRef
Ext) {
+ std::vector<RISCVExtensionVersion> Result;
----------------
This name was already bad and getting worse. It doesn't return a bool so
shouldn't start with `is`. It should be called something like
`getExperimentalExtensionVersions`.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:341
+ addVersionSuffix("experimental-", "zvlsseg", Major, Minor));
+ } else if (isExperimentalExtension(ExtName).size()) {
+ Features.push_back(
----------------
Use `!isExperimentalExtension(ExtName).empty()`
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:404
+ auto ExperimentalExtension = isExperimentalExtension(Ext);
+ if (ExperimentalExtension.size()) {
if (!EnableExperimentalExtension) {
----------------
!empty
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:481
+ auto Info = isSupportedExtensionFeature(ExtName);
+ if(!Info)
continue;
----------------
Please fix this clang-format issue
================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:56
+void RISCVSubtarget::initializeEnvironment() {
+ StdExtM = {0, 0};
+ StdExtA = {0, 0};
----------------
Add a reset method?
================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.cpp:91
StringRef ABIName) {
+ initializeEnvironment();
// Determine default and user-specified characteristics
----------------
Why do we need to initialize things now but didn't before?
================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:38
virtual void anchor();
- bool HasStdExtM = false;
- bool HasStdExtA = false;
- bool HasStdExtF = false;
- bool HasStdExtD = false;
- bool HasStdExtC = false;
- bool HasStdExtZba = false;
- bool HasStdExtZbb = false;
- bool HasStdExtZbc = false;
- bool HasStdExtZbe = false;
- bool HasStdExtZbf = false;
- bool HasStdExtZbm = false;
- bool HasStdExtZbp = false;
- bool HasStdExtZbr = false;
- bool HasStdExtZbs = false;
- bool HasStdExtZbt = false;
- bool HasStdExtV = false;
- bool HasStdExtZvlsseg = false;
- bool HasStdExtZfhmin = false;
- bool HasStdExtZfh = false;
+ RISCVExtensionVersion StdExtM = {0, 0};
+ RISCVExtensionVersion StdExtA = {0, 0};
----------------
Can we have a default constructor for RISCVExtensionVersion?
================
Comment at: llvm/lib/Target/RISCV/RISCVSubtarget.h:106
bool enableMachineScheduler() const override { return true; }
- bool hasStdExtM() const { return HasStdExtM; }
- bool hasStdExtA() const { return HasStdExtA; }
- bool hasStdExtF() const { return HasStdExtF; }
- bool hasStdExtD() const { return HasStdExtD; }
- bool hasStdExtC() const { return HasStdExtC; }
- bool hasStdExtZba() const { return HasStdExtZba; }
- bool hasStdExtZbb() const { return HasStdExtZbb; }
- bool hasStdExtZbc() const { return HasStdExtZbc; }
- bool hasStdExtZbe() const { return HasStdExtZbe; }
- bool hasStdExtZbf() const { return HasStdExtZbf; }
- bool hasStdExtZbm() const { return HasStdExtZbm; }
- bool hasStdExtZbp() const { return HasStdExtZbp; }
- bool hasStdExtZbr() const { return HasStdExtZbr; }
- bool hasStdExtZbs() const { return HasStdExtZbs; }
- bool hasStdExtZbt() const { return HasStdExtZbt; }
- bool hasStdExtV() const { return HasStdExtV; }
- bool hasStdExtZvlsseg() const { return HasStdExtZvlsseg; }
- bool hasStdExtZfhmin() const { return HasStdExtZfhmin; }
- bool hasStdExtZfh() const { return HasStdExtZfh; }
+ bool hasStdExtM() const { return StdExtM != RISCVExtensionVersion{0, 0}; }
+ bool hasStdExtA() const { return StdExtA != RISCVExtensionVersion{0, 0}; }
----------------
Add a helper method to RISCVExtensionVersion to check for a null version?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D115921/new/
https://reviews.llvm.org/D115921
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
