[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

[Kito Cheng via Phabricator via cfe-commits]

kito-cheng added inline comments.


================
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}},
----------------
jrtc27 wrote:
> I'd keep these all one per line
Although it's format by clang-format, but I like one per line too, let me manually update that.


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:107
+struct FindByName {
+  FindByName(StringRef Ext) : Ext(Ext){};
+  StringRef Ext;
----------------
jrtc27 wrote:
> 
It's format by clang-format, but either is fine to me :P


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:378
+        llvm::find_if(ExtensionInfos, FindByName(ExtName));
+    if (ExtensionInfoIterator == ExtensionInfos.end())
+      continue;
----------------
jrtc27 wrote:
> No error?..
Not all features is related to extension, like `relax` or `save-restore`, so here we just ignore, but add few comment here seem better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105168/new/

https://reviews.llvm.org/D105168