[PATCH] D141479: [RISCV] Generate march string from target features
Francesco Petrogalli via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 11 07:00:54 PST 2023
fpetrogalli added inline comments.
================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:30
+
+static std::string getMArch(int XLen, const Record &Rec) {
+ std::vector<Record *> Features = Rec.getValueAsListOfDefs("Features");
----------------
I think you should reference here the pieces of the standard that describe the rules for building the March string.
================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:31
+static std::string getMArch(int XLen, const Record &Rec) {
+ std::vector<Record *> Features = Rec.getValueAsListOfDefs("Features");
+ std::vector<std::string> FeatureVector;
----------------
Do you need this variable? it is used only once...
================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:38
+ if (llvm::RISCVISAInfo::isSupportedExtensionFeature(FeatureName))
+ FeatureVector.push_back(std::string("+") + FeatureName.str());
+ }
----------------
I think you can avoid constructing a `std::string` here?
================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:41
+
+ auto ISAInfo = llvm::RISCVISAInfo::parseFeatures(XLen, FeatureVector);
+ if (!ISAInfo)
----------------
Please avoid `auto` if the type cannot be deducted from the context.
================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:66
+ int XLen = getXLen(Rec);
+ std::string EnumFeatures = getEnumFeatures(XLen);
+ std::string MArch = Rec.getValueAsString("DefaultMarch").str();
----------------
Do we need this variable? It is used only once...
================
Comment at: llvm/utils/TableGen/RISCVTargetDefEmitter.cpp:67
+ std::string EnumFeatures = getEnumFeatures(XLen);
+ std::string MArch = Rec.getValueAsString("DefaultMarch").str();
+
----------------
You could use StringRef and then check MArch.empty() in line 70.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D141479/new/
https://reviews.llvm.org/D141479
More information about the llvm-commits
mailing list