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

Mandeep Singh Grang via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 31 11:25:56 PST 2018


mgrang added a comment.

In https://reviews.llvm.org/D42673#992056, @asb wrote:

> 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), with a line changed (setting a different name for the linker). Was it meant to be a one-line patch on top of that work?
>
>   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:
> 4. 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
> 5. 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)?


Alex,
This patch is based on your previously submitted patch as part of https://reviews.llvm.org/D39963. The issue is that if I pass -target riscv32 then it invokes riscv32-gcc but if I pass -target riscv32-unknown-linux-gnu then is correctly invokes riscv32-unknown-linux-gnu-ld. So what we need to sort out is the behavior we want for the -target option. We should let it fail to find the linker if the complete triple is not passed rather than invoking the gcc driver (that's the behavior on ARM).


Repository:
  rC Clang

https://reviews.llvm.org/D42673





More information about the cfe-commits mailing list