jrtc27 added inline comments.
================
Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:49-50
+ /// Parse RISCV ISA info from arch string.
+ static std::unique_ptr<RISCVISAInfo>
+ parseArchString(llvm::Error &Error, StringRef Arch,
+ bool EnableExperimentalExtension,
----------------
Use llvm::Expected<...> as the return value to avoid a separate error out param
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:40
+
+static const RISCVSupportedExtensionInfo SupportedExtensionInfos[] = {
+ {"i", RISCVExtensionVersion{2, 0}}, {"e", RISCVExtensionVersion{1, 9}},
----------------
Is the "Infos" suffix on the variable name really needed?
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:41
+static const RISCVSupportedExtensionInfo SupportedExtensionInfos[] = {
+ {"i", RISCVExtensionVersion{2, 0}}, {"e", RISCVExtensionVersion{1, 9}},
+ {"m", RISCVExtensionVersion{2, 0}}, {"a", RISCVExtensionVersion{2, 0}},
----------------
I'd keep these all one per line
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:107
+struct FindByName {
+ FindByName(StringRef Ext) : Ext(Ext){};
+ StringRef Ext;
----------------
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:378
+ llvm::find_if(ExtensionInfos, FindByName(ExtName));
+ if (ExtensionInfoIterator == ExtensionInfos.end())
+ continue;
----------------
No error?..
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105168/new/
https://reviews.llvm.org/D105168
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
