[PATCH] D42673: [RISCV] Pick the correct RISCV linker instead of calling riscv-gcc to link

Tue Jan 30 10:28:00 PST 2018

asb added a comment.

My main concern with this patch is that the description doesn't really match what it does. The current in-tree code _doesn't_ call gcc to link for the tested configuration (a multilib toolchain), and this is verified with the tests in test/Driver/riscv32-toolchain.c. https://reviews.llvm.org/D39963 originally added support for a baremetal toolchain, but was changed to focus on the multilib Linux toolchain for a couple of reasons:

1. This was a more straight-forward change
2. Downstream users such as yourself had a higher priority for building for a Linux target
3. It looks like we'd want to discuss the possibility of adding RISC-V support to lib/Driver/ToolChains/BareMetal rather than adding yet another target-specific toolchain

This patch seems identical to my changes to add a baremetal toolchain (https://github.com/lowRISC/riscv-llvm/blob/master/clang/0003-RISCV-Implement-clang-driver-for-the-baremetal-RISCV.patch which was previously submitted as part of https://reviews.llvm.org/D39963).

Are you compiling for a Linux target or for bare metal? One problem with the multilib detection code is that if it fails it tends to do so silently. I see two paths forward depending on the immediate problem you're seeing:

1. If you're seeing an issue with the correct linker being selected when compiling using a Linux toolchain, the correct fix would be to modify that detection logic committed in https://reviews.llvm.org/rL322276 and add new tests
2. If the issue is that you're now trying to use Clang for a bare-metal target, we should discuss whether to move forwards with my baremetal patch as-is or to try to modify lib/Driver/ToolChains/BareMetal - probably worth discussing this on cfe-dev.

The fact you've hardcoded "riscv32-unknown-linux-gnu-ld" as the linker name makes me wonder if the current issue you're seeing is 1)?

Repository:
  rC Clang

https://reviews.llvm.org/D42673