[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