Alex Bradbury via llvm-dev
2017-Aug-09 19:14 UTC
[llvm-dev] Target mistakenly added to `LLVM_ALL_TARGETS` (RISCV)
Problem: Let's start with the bad news. RISCV is a currently an in-development, and experimental backend. The initial 'stub backend' commit <https://reviews.llvm.org/D23560> contained a silly semantic mistake, adding RISCV to `LLVM_ALL_TARGETS` in CMakeLists.txt. I failed to recognise that this was a list of all default targets rather than an exhaustive list, meaning that the beginnings of the backend have been building by default since they were committed. I became aware of this mistake yesterday. Obviously this is an embarrassing oversight, for which I apologise. Impact: The good news is that although a lot of time has passed since then, the impact should be relatively low. As there wasn't enough committed code to provide assembly and disassembly support, there shouldn't be any reason for users to notice this issue or its resolution. There is of course much more code up for review <https://reviews.llvm.org/differential/?authors=asb>. People using LLVM binaries that were built with all default targets will have seen a very slightly larger library, and might have spotted the riscv32 and riscv64 triples listed in e.g. `llc --version` or `llvm-mc --version`. Trying to use these triples would have resulted in an early failure. Current behaviour: $ llvm-mc -triple riscv32 /path/to/file.s error: unable to create instruction printer for target triple 'riscv32' with assembly variant 0. $ llc -mtriple riscv32 /path/to/file.s target does not support generation of this file type! Desired behaviour: $ llvm-mc -triple riscv32 /path/to/file.s error: unable to get target for 'riscv32', see --version and --triple. $ llc -mtriple riscv32 /path/to/file.s error: unable to get target for 'riscv32', see --version and --triple. Proposed resolution: Removing the unintended addition seems to be the best option all round. If there was a chance of this affecting end users, we might need to think about alternatives. I have authored a patch that does this, as well as adding a new comment to `LLVM_ALL_TARGETS`. <https://reviews.llvm.org/D36538>. I also intend to propose this patch for the LLVM 5.0 branch. The main purpose of this email to llvm-dev is to be extra sure nobody sees any downsides I'm missing to this resolution. `LLVM_ALL_TARGETS` and building experimental backends are clearly explained in docs/WritingAnLLVMBackend.rst, so I don't think documentation changes are required beyond a new comment in CMakeLists.txt. Thanks for your time. Alex