This patch adds necessary driver support for the RISC-V Linux target, focused on supporting a multilib Linux SDK produced by https://github.com/riscv/riscv-gnu-toolchain/.
Details
Diff Detail
Event Timeline
lib/Driver/ToolChains/RISCV.h | ||
---|---|---|
42 | I wonder if we should construct the linker name based on the default triple value? "riscv64-unknown-elf-ld" or "riscv32-unknown-elf-ld" as this is likely what is in the user's PATH if they have installed the gnu tools. The linker name could be modified in ConstructJob if the triple is different from the default. |
lib/Driver/ToolChains/RISCV.cpp | ||
---|---|---|
86 | How about if our sysroot is linux (as opposed to elf)? There won't be any libgloss.a, right? Also there won't be a crt0.o (instead there will be crt1.o). | |
lib/Driver/ToolChains/RISCV.h | ||
42 | I agree with @johnrusso . I always have errors: "riscv32-ld not found" and as a workaround I end up creating symlink pointing to the actual <triple>-ld. |
lib/Driver/ToolChains/RISCV.cpp | ||
---|---|---|
86 | Supporting linux target is desirable early on because most of us will be using Qemu for running tests. |
lib/Driver/ToolChains/RISCV.cpp | ||
---|---|---|
86 | Linux targets are not currently supported, as they require the ilp32d or lp64d (hard double precision) ABI. The only fiddly part is actually in the Clang frontend, handling structs composed of two reals or one integer + one real. This is obviously high up on the todo list. |
@asb I cherry-picked this patch and was able to compile a simple program for elf triple. By manually adding a few libs on the link line I was also able to make it link for linux triple.
Could you please respond to the comment about risv32-ld? Other than that this patch LGTM!
test/Driver/riscv64-toolchain.c | ||
---|---|---|
1 | I just saw that this test fails with the error: error: backend data layout 'e-m:e-i64:64-n32:64-S128' does not match expected target description 'e-m:e-p:64:64-i64:64-i128:128-n64-S128' |
Consider this a WIP update. This is not yet ready for merging, but could still benefit from feedback.
This update adds support for RISC-V to the Linux toolchain driver, which includes support for multilib selection. Unfortunately I had to add a special case to lib/Driver/Toolchains/Linux.cpp for adding lib paths laid out according to the RISC-V multilib conventions (e.g. $SYSROOT/lib32/ilp32, $SYSROOT/usr/lib32/ilp32). I notice that Mips had to do something similar, and ended up defining the MipsLinux class.
@jroelofs: Did you have any views one way or the other about extending BareMetal to support RISC-V as well as ARM?
lib/Driver/ToolChains/RISCV.cpp | ||
---|---|---|
86 | It seems I was mistaken - soft float linux targets with glibc are working fine, meaning there's no need to wait for hardfloat ABI lowering. Note that Linux targets are handled in lib/Driver/ToolChains/Linux.cpp | |
lib/Driver/ToolChains/RISCV.h | ||
42 | Now updated so the bin dir from the given GCC install (passed by --gcc-toolchain) is added to the PATH, and the default search logic will look for an ld named after after the target triple. |
test/Driver/riscv64-toolchain.c | ||
---|---|---|
1 | Pushed a fix for this: https://reviews.llvm.org/D40145 |
Can you push this as a patch to review/commit instead of RFC? It has received a lot of comments/corrections already and I think it is getting in a shape we can merge.
lib/Driver/ToolChains/RISCV.cpp | ||
---|---|---|
86 | I meant user code compiled with target=riscv32-unknown-linux march=rv32imafdc and mabi=ilp32. This would run fine on qemu. | |
test/Preprocessor/init.c | ||
9991 | Shouldn't we just check for the target specific defines in these tests? |
I might move the baremetal support to separate patch and discuss that on the mailing list. I'll remove the [RFC] tag in the next update, adding tests.
test/Preprocessor/init.c | ||
---|---|---|
9991 | Many of the other tests seem to test a similarly large set of defines, plus I think you _do_ want to check the majority of these (especially the ones related to C types). |
lib/Driver/ToolChains/Arch/RISCV.cpp | ||
---|---|---|
47 โ | (On Diff #123229) | Can you also add a case for pushing back +c here? |
Patch updates:
- Address review comments
- (u)int64_type is long int on RV64 rather than long long int
- Remove driver for bare metal RV32 target (to be posted in a subsequent patch and maybe discussed on cfe-dev).
From my perspective, the only deficiency in this patch is now the tests (which I'll add to this review ASAP). Let me know if you see other remaining problems.
Update to add test cases based on the multilib Linux SDK produced by https://github.com/riscv/riscv-gnu-toolchain/.
Apologies, the last version had a few lines of debug code left in.
I should say that this is to the best of my knowledge ready to merge (i.e. there are no outstanding flagged issues). Particularly now that the majority of the RV32I codegen patches are merged in LLVM, it would be really helpful to get this merged in to clang asap in order to make larger scale testing of the LLVM backend possible without out-of-tree patches.
Adding some reviewers to hopefully find someone comfortable reviewing the Linux multilib bits. (Not sure I'm adding the right people; this stuff hasn't been touched for a while, as far as I can tell.)
The rest LGTM.
Please merge this patch, it looks in good shape. This patch is required for any RISCV build.
This has broken our toolchain build, the log is here: https://logs.chromium.org/v/?s=fuchsia%2Fbuildbucket%2Fcr-buildbucket.appspot.com%2F8957686819564148864%2F%2B%2Fsteps%2Fcheck_clang%2F0%2Fstdout. The failure is:
******************** TEST 'Clang :: Driver/riscv32-toolchain.c' FAILED ******************** Script: -- /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/clang /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c -### -no-canonical-prefixes -target riscv32 2>&1 | /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/FileCheck -check-prefix=CC1 /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/clang /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c -### -no-canonical-prefixes -target riscv32-linux-unknown-elf --gcc-toolchain=/b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk --sysroot=/b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 | /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/FileCheck -check-prefix=CC1-RV32-LINUX-ILP32 /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/clang /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c -### -no-canonical-prefixes -target riscv32-linux-unknown-elf -march=rv32imafd -mabi=ilp32d --gcc-toolchain=/b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk --sysroot=/b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/Inputs/multilib_riscv_linux_sdk/sysroot 2>&1 | /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/FileCheck -check-prefix=CC1-RV32-LINUX-ILP32D /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/clang -target riscv32 /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c -emit-llvm -S -o - | /b/s/w/ir/tmp/rt/clangM7g4Ie/llvm_build_dir/tools/clang/stage2-bins/bin/FileCheck /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c -- Exit Code: 1 Command Output (stderr): -- /b/s/w/ir/kitchen-workdir/llvm-project/clang/test/Driver/riscv32-toolchain.c:13:26: error: expected string not found in input // CC1-RV32-LINUX-ILP32: "{{.*}}/Inputs/multilib_riscv_linux_sdk/lib/gcc/riscv64-unknown-linux-gnu/7.2.0/../../../../riscv64-unknown-linux-gnu/bin{{/|\\\\}}ld" ^ <stdin>:1:1: note: scanning from here Fuchsia clang version 7.0.0 (https://fuchsia.googlesource.com/a/third_party/clang 6d538af698b9b96bc18bfbe62173dca1c103aaca) (https://fuchsia.googlesource.com/a/third_party/llvm 0b5677cbf87647986caf9fa818adb7f8c355cf1f) (based on LLVM 7.0.0svn) ^ --
I think the problem is that in our toolchain, we configure LLD as the default linker which is why that check is failing.
I suppose we need riscv64-unknown-elf here too?