[llvm-dev] Target mistakenly added to `LLVM_ALL_TARGETS` (RISCV)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[llvm-dev] Target mistakenly added to `LLVM_ALL_TARGETS` (RISCV)

George Karpenkov via llvm-dev
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
_______________________________________________
LLVM Developers mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
Loading...