lenary added a comment. In D68391#1694622 <https://reviews.llvm.org/D68391#1694622>, @edward-jones wrote:
> Rebased and added tests > > I've made this use the Triple from the driver rather than the parsed LLVM > triple, this means the Triple doesn't get normalized which seems like more > desirable behavior. Yeah, using the user-provided triple seems the correct choice, as the user will expect it to match the directory name. Would be useful to add a comment saying something along the same lines where you do that. > I've added to the riscv{32,64}-toolchain.c test files, however the added > tests cannot be run without a shell so I've had to disable those tests on > Windows. If necessary I can split these new tests out into separate files. I think splitting the tests might be sensible, so that we can run as many as possible on platforms without a shell/windows. > I realize that there don't appear to be any tests for the case where no GCC > install is found and no sysroot is provided to the driver. At the moment this > will result in a generic linker command using the system linker, such as: > > /usr/bin/ld crt0.o crtbegin.o ... -lgcc crtend.o > > Is this the desired behaviour? And if so should a test be added for this too? I think defaulting to root if no sysroot/gcc install is found is better than erroring. In all likelihood a compile/link task will fail anyway because it cannot link the results together. In the case where the compile/link does work, then there's no issue. It would be useful to have a test for this. In D68391#1707652 <https://reviews.llvm.org/D68391#1707652>, @luismarques wrote: > This is indeed an issue that would be nice to fix, I've often been annoyed by > clang just defaulting to the root when some misconfiguration occurs. I have > to wonder though, this patch only changes the clang RISC-V toolchain driver, > but the problem isn't specific to RISC-V. Couldn't this tweak be generalized > and made to apply to multiple/all target drivers? I think that for baremetal risc-v, we can change this default without asking the wider community. For other targets, someone should email cfe-dev first with the proposal. I agree it makes sense, but I also imagine it could break a lot of builds unexpectedly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68391/new/ https://reviews.llvm.org/D68391 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits