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

[Kito Cheng via Phabricator via llvm-commits]

kito-cheng added inline comments.


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:39
+
+static constexpr StringLiteral AllStdExts = "mafdqlcbjtpvn";
+
----------------
craig.topper wrote:
> 'b' shouldn't be in this list anymore?
I would prefer to keep that for sync with ISA manual, like other unsupported extension like `J`, `T` and ``N`, we remove `B` from `SupportedExtensions`, so keep this should be harmless.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2089
     else
       return Error(ValueExprLoc, "bad arch string " + Arch);
   }
----------------
khchen wrote:
> maybe we could have a NFC patch later to add 
> ```
> // Emit the arch string if needed.
> if (!IsIntegerValue)
>   getTargetStreamer().emitTextAttribute(Tag, ISAInfo->toString());
> ```
> here to reuse `ISAInfo`, and remove 2098~2117 code.
> 
Good point, thanks :)
Let improve that by following patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105168