craig.topper added inline comments.
================ Comment at: llvm/include/llvm/Support/RISCVISAInfo.h:42 + // keep entries in canonical order of extension. + typedef std::map<std::string, RISCVExtensionInfo, ExtensionComparator> + OrderedExtensionMap; ---------------- Could this be a sorted vector? Would require a good spot to sort it after all extensions have been added. Are all the extensions added from the parse functions? ================ Comment at: llvm/lib/Support/RISCVISAInfo.cpp:279 + Features.push_back("+experimental-zvamo"); + } else if (isExperimentalExtension(ExtName)) + Features.push_back(Args.MakeArgString("+experimental-" + ExtName)); ---------------- Consistently use curly braces on all of the blocks ================ Comment at: llvm/lib/Support/RISCVISAInfo.cpp:303 + + if (MajorStr.size() && In.consume_front("p")) { + MinorStr = std::string(In.take_while(isDigit)); ---------------- MajorStr.size() -> !MajorStr.empty() ================ Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:55 +namespace llvm { +extern const SubtargetFeatureKV RISCVFeatureKV[]; +} ---------------- Can this be `extern const SubtargetFeatureKV RISCVFeatureKV[RISCV::NumSubtargetFeatures];` then I think you can get rid of the ArrayRef? 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