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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits