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

Craig Topper via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Oct 13 09:52:59 PDT 2021


craig.topper accepted this revision.
craig.topper added a comment.
This revision is now accepted and ready to land.

LGTM other than that comment about removing 'b' from AllStdExts.



================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:39
+
+static constexpr StringLiteral AllStdExts = "mafdqlcbjtpvn";
+
----------------
'b' shouldn't be in this list anymore?


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:388
+                            const std::vector<std::string> &Features) {
+  std::unique_ptr<RISCVISAInfo> ISAInfo(new RISCVISAInfo());
+  assert(XLen == 32 || XLen == 64);
----------------
kito-cheng wrote:
> craig.topper wrote:
> > Use
> > 
> > ```
> > auto ISAInfo = std::make_unique<RISCVISAInfo>()
> > ```
> `std::make_unique<RISCVISAInfo>()` require a `public` constructor, but I would prefer keep that in `private`.
I agree with keeping the constructor private. I didn't realize make_unique had that restriction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105168



More information about the cfe-commits mailing list