[PATCH] D24156: [CMake] Explicitly add --target option to compiler flags

[Renato Golin via llvm-commits]

rengolin added a comment.

In https://reviews.llvm.org/D24156#539033, @beanz wrote:

> `COMPILER_RT_DEFAULT_TARGET_TRIPLE` defaults today to whatever the `TARGET_TRIPLE` is from LLVM, which should be the default target triple of clang. It can be overridden in a dozen or so ways, but that is the default expected behavior.


Ok.

> This comes into play in the target tests where it differentiates Arm32 from AArch64 by passing the `-m64` compiler flag, and it passes `-mcpu` to the ArmV7 builds without passing `-target`.


The `-mcpu` case is real, but you should not use `-m64` to pick between 32 and 64 bit ARM. This works on x86 because they're the same back-end, but should not work between AArch32 and AArch64.

> In general these issues cause the configuration checks to fail or silently incorrectly succeed, when you are building multi-arch libraries with a multi-targeting compiler.


Yes, this is the *real* problem.

> I'm trying to come up with a solution where I can support building multi-targeting cross-compilers simply from a single configuration. My experimental patch is https://reviews.llvm.org/P7504, this supports configuring LLVM & Clang something like:

> 

>   cmake -G ... -DLLVM_TARGETS_TO_BUILD="X86;ARM;AArch64" -DLLVM_BUILTIN_TARGETS="i686-unknown-freebsd;armv7-unknown-freebsd;aarch64-unknown-freebsd"

> 

> 

> Which will generate `builtins-<triple>` targets as part of `all`. Allowing you to simply build a multi-target compiler from a single build configuration.


This would be awesome! But how do you expect to pass the other flags (`-mfloat-abi`, `-mtune`, etc) to each individual target?

> If the `COMPILER_RT_DEFAULT_TARGET ` is `arm-linux-gnueabi`, and that gets passed to `-target` is that a problem? Anecdotally it seems to me that it isn't, but @fjricci implied that it is.


I don't know if @fjricci was referring to the same problem I was, but in my case it's not a problem, per se, to pass that to `-target`, unless this is not what you want.

So, if this comes from `-DLLVM_BUILTIN_TARGETS`, than I assume you know what you're doing and it works fine. If it comes from the default LLVM_TARGET, then it's likely not what you want, and silent errors will be the enemy.

All in all, I think we should move to a more annoying driver that understand the problems across architectures (for example, error out instead of using the system linker for cross architectures), but that's orthogonal to your patch.

The only problem I see with your patch as is is that you can't still pass the correct architectural options to each target, which may be restrictive, but not *broken*. Otherwise, looks good.

cheers,
--renato


https://reviews.llvm.org/D24156