Page MenuHomePhabricator

Build system changes for RISCV
ClosedPublic

Authored by azharudd on Mar 6 2018, 7:54 AM.

Details

Summary

Build system changes for RISCV. Makes it possible to build just the RISCV target alone.

Diff Detail

Repository
rL LLVM

Event Timeline

azharudd created this revision.Mar 6 2018, 7:54 AM
asb accepted this revision.Mar 8 2018, 2:02 PM

Hi Azharuddin, many thanks for the contribution. I hadn't spotted this part of LLVM's CMake build system, and it's definitely useful to be able to build just the RISCV backend. I've tried this locally, and the problem I'm seeing is that a good number of Clang and LLVM tests fail when you don't build X86 support. On my system, 351 unexpected failures across Clang+LLVM. This is a problem with the tests rather than this patch (they should really be disabled if there is no X86 support), but it does limit the usefulness of just building _only_ the RISC-V backend. Is it expected that you build an LLVM with X86 and RISCV, ensure 100% tests pass, then build with just the RISCV backend and ignore any new failures?

The problems I'm seeing aren't introduced by this patch, so LGTM. I'd be curious on how/if you work around the tests issue though.

This revision is now accepted and ready to land.Mar 8 2018, 2:02 PM
asb added a comment.Mar 8 2018, 2:11 PM

Actually, looking again I'm not having any problems building just the RISCV backend even without this patch. e.g. an invocation like the following seems fine:

cmake -G Ninja -DCMAKE_BUILD_TYPE="Debug" \
  -DBUILD_SHARED_LIBS=True -DLLVM_USE_SPLIT_DWARF=True \
  -DLLVM_OPTIMIZED_TABLEGEN=True \
  -DLLVM_BUILD_TESTS=True \
  -DLLVM_TARGETS_TO_BUILD="RISCV" \
  -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="RISCV" ../
cmake --build .

Could you please clarify exactly which build problems this fixes?

In D44153#1031976, @asb wrote:

Actually, looking again I'm not having any problems building just the RISCV backend even without this patch. e.g. an invocation like the following seems fine:

cmake -G Ninja -DCMAKE_BUILD_TYPE="Debug" \
  -DBUILD_SHARED_LIBS=True -DLLVM_USE_SPLIT_DWARF=True \
  -DLLVM_OPTIMIZED_TABLEGEN=True \
  -DLLVM_BUILD_TESTS=True \
  -DLLVM_TARGETS_TO_BUILD="RISCV" \
  -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="RISCV" ../
cmake --build .

Could you please clarify exactly which build problems this fixes?

Alex, add -DLLVM_TARGET_ARCH=riscv32-unknown-linux-gnu to your cmake command above, and you'll hit the "Unknown architecture ..." error. Since we are cross-compiling, we'll need that option to default to riscv target arch. Without that option, it will target the host as the native arch.

In D44153#1031969, @asb wrote:

Hi Azharuddin, many thanks for the contribution. I hadn't spotted this part of LLVM's CMake build system, and it's definitely useful to be able to build just the RISCV backend. I've tried this locally, and the problem I'm seeing is that a good number of Clang and LLVM tests fail when you don't build X86 support. On my system, 351 unexpected failures across Clang+LLVM. This is a problem with the tests rather than this patch (they should really be disabled if there is no X86 support), but it does limit the usefulness of just building _only_ the RISC-V backend. Is it expected that you build an LLVM with X86 and RISCV, ensure 100% tests pass, then build with just the RISCV backend and ignore any new failures?

The problems I'm seeing aren't introduced by this patch, so LGTM. I'd be curious on how/if you work around the tests issue though.

I don't see any additional failures with this change. It shouldn't cause any issues with existing builds. You are right -- it is a problem with the tests and not this patch. If there are any test failures when building just RISC-V -- particularly the target independent ones -- we'll have to fix RISC-V for those tests (we can ignore those failures for now). If there are any target specific tests requiring certain target, they should be marked as such.

This revision was automatically updated to reflect the committed changes.
asb added a comment.Mar 13 2018, 12:18 PM
In D44153#1031976, @asb wrote:

Actually, looking again I'm not having any problems building just the RISCV backend even without this patch. e.g. an invocation like the following seems fine:

cmake -G Ninja -DCMAKE_BUILD_TYPE="Debug" \
  -DBUILD_SHARED_LIBS=True -DLLVM_USE_SPLIT_DWARF=True \
  -DLLVM_OPTIMIZED_TABLEGEN=True \
  -DLLVM_BUILD_TESTS=True \
  -DLLVM_TARGETS_TO_BUILD="RISCV" \
  -DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD="RISCV" ../
cmake --build .

Could you please clarify exactly which build problems this fixes?

Alex, add -DLLVM_TARGET_ARCH=riscv32-unknown-linux-gnu to your cmake command above, and you'll hit the "Unknown architecture ..." error. Since we are cross-compiling, we'll need that option to default to riscv target arch. Without that option, it will target the host as the native arch.

Thanks for the clarification. I suppose a more accurate description of this patch would be that it enables LLVM_TARGET_ARCH to be set to riscv32 or riscv64, as it seems that building with -DLLVM_TARGETS_TO_BUILD="RISCV" did work as expected (building only lib/Target/RISCV) just fine prior to this patch.