luismarques added a comment.
This patch seems to be in pretty good shape now.
One thing that might be useful (important?) to add is additional diagnostics
when run in verbose mode. Currently `clang -v` will indicate that it found the
GCC installation and will list the multilibs but there will be no indication
that the multilib list came from GCC. Also, if things like the `ExecuteAndWait`
(in `getRISCVMultilibFromGCC`) fail shouldn't we print some kind of diagnostic,
at least in verbose mode? Otherwise when problems occur it might be tricky to
figure out what's going on.
================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:726
+StringRef riscv::getRISCVCodeModel(const llvm::opt::ArgList &Args) {
+ // Default code model is small(medlow).
+ StringRef CodeModel;
----------------
Nitpicking suggestion: `\\ Default code model is 'small' (what GCC calls
'medlow').`
================
Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:1657
+ for (StringRef Line : Lines) {
+ if (Line.empty())
+ continue;
----------------
Maybe trim whitespace before checking for empty lines, for extra robustness?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97916/new/
https://reviews.llvm.org/D97916
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
