jrtc27 added inline comments.
================ Comment at: clang/lib/Basic/Targets/RISCV.cpp:122 StringRef CodeModel = getTargetOpts().CodeModel; + unsigned FLEN = ISAInfo.getFLEN(); if (CodeModel == "default") ---------------- Capitalise these as FLen and XLen everywhere ================ Comment at: clang/lib/Basic/Targets/RISCV.cpp:220 + if (ISAInfo.isSupportedExtension(Feature, + /* CheckExperimental */ true)) + return ISAInfo.hasExtension(Feature); ---------------- is the right formatting I think? ================ Comment at: clang/lib/Basic/Targets/RISCV.cpp:230 + unsigned XLEN = getTriple().isArch64Bit() ? 64 : 32; + if (auto E = ISAInfo.parse(XLEN, Features)) + return false; ---------------- Unused variable; I think this should either be consumeError'ed or propagated up? ================ Comment at: llvm/lib/Support/RISCVISAInfo.cpp:644 + "invalid extension prefix '%s'", + Ext.str().c_str()); + } ---------------- craig.topper wrote: > craig.topper wrote: > > does createStringError really require going to from StringRef to > > std::string to char*? > I guess it does if you use it with %s. Maybe better to use > > ``` > "invalid extension prefix '" + Ext + "'" > ``` > > since the format string is really a Twine. That will break if the user-provided extension contains a %. Sadly `.str().c_str()` is what you need. 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
