frasercrmck added inline comments.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:127
+
+bool RISCVISAInfo::isSupportedExtensionFeature(StringRef Ext) {
+ bool IsExperimental = stripExperimentalPrefix(Ext);
----------------
This looks like a `find_if` if that'd make it any simpler.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:147
+ unsigned MinorVersion) {
+ for (auto const &SupportedExtensionInfo :
+ filterSupportedExtensionInfosByName(Ext)) {
----------------
This also looks like a `find_if`
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:231
+ size_t RHSLen = RHS.length();
+ int LHSRank, RHSRank;
+ if (LHSLen == 1 && RHSLen != 1)
----------------
Not sure why these need to be declared up here.
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:310
+ errc::invalid_argument,
+ "Failed to parsing major version number for extension '" + Ext + "'");
+
----------------
`Failed to parse ...`
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:315
+ errc::invalid_argument,
+ "Failed to parsing minor version number for extension '" + Ext + "'");
+
----------------
`Failed to parse`
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:359
+ }
+ // return true;
+ return Error::success();
----------------
Commented-out code
================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:414
+ // RISC-V ISA strings must be lowercase.
+ if (llvm::any_of(Arch, [](char C) { return isupper(C); })) {
+ return createStringError(errc::invalid_argument,
----------------
I think you can just have `if (llvm::any_of(Arch, isupper))` here.
================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2036
+ for (auto Feature : RISCVFeatureKV) {
+ if (llvm::RISCVISAInfo::isSupportedExtensionFeature(Feature.Key)) {
+ clearFeatureBits(Feature.Value, Feature.Key);
----------------
Don't need `{}` here
================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2055
+
+ for (auto Feature : RISCVFeatureKV) {
+ if (ISAInfo.hasExtension(Feature.Key)) {
----------------
Don't need `{}` here either
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
