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

Reply via email to