[PATCH] D40947: [cmake] Make setting of CMAKE_C(XX)_COMPILER flags overridable for cross-builds

[Don Hinton via Phabricator via llvm-commits]

hintonda added a comment.

In https://reviews.llvm.org/D40947#952317, @labath wrote:

> In https://reviews.llvm.org/D40947#951479, @hintonda wrote:
>
> > Did this fix an existing problem?
>
>
> Yes. It allowed us to cross-compile with a command line like:
>  `-DCMAKE_TOOLCHAIN_FILE=/android/toolchain/file.cmake -DCROSS_TOOLCHAIN_FLAGS_NATIVE=-DCMAKE_TOOLCHAIN_FILE=/host/toolchain/file.cmake`
>
> Without this we would have had to append additional -DCMAKE_C??_COMPILER to the command line (duplicating the logic already present in the toolchain file). The other thing that seemed to work is to use `set(CMAKE_CXX_COMPILER ... FORCE)` in the toolchain file, but that also seemed like an ugly workaround.


I was only asking about this specific change, i.e., adding logic to handle `${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake` which doesn't exist in the default case.  I agree that CMAKE_(C|CXX)_COMPILER should be passed.

> However, I agree with the rest of your analysis.
> 
>> It looks like some of this logic is OBE.
>> 
>> Here's my reasoning.
>> 
>> As far as I can tell, this code is only run when either `-DLLVM_OPTIMIZED_TABLEGEN=ON` or `CMAKE_SYSTEM_NAME` variable is passed or set in the appropriate cache or toolchain file -- which cmake ultimately uses to set `CMAKE_CROSSCOMPILING=ON` internally.
>> 
>> In both cases, these variables are used to set `LLVM_USE_HOST_TOOLS`, which, if true, includes CrossCompile.cmake in llvm/CMakeLists.txt.  CrossCompile.cmake defines two functions, `llvm_create_cross_target` and `llvm_create_cross_target_internal`, and ultimately calls `llvm_create_cross_target_internal(NATIVE "" Release)` -- I can't find any other places where either of these two functions are called.
>> 
>> If this is in fact the only call to `llvm_create_cross_target_internal`, in this case with toolchain = "", the toolchain file will be `${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake` which doesn't match any existing file, and is therefore never passed via `CROSS_TOOLCHAIN_FLAGS_INIT` to the custom cmake command.
>> 
>> Are there any calls to `llvm_create_cross_target` or `llvm_create_cross_target_internal` that I've missed where a valid toolchain is actually passed?  If not, is there a reason to maintain the toolchain logic in `llvm_create_cross_target`?
> 
> The analysis seems correct, although I can't claim I am sufficiently familiar with all llvm sub-projects to be certain one of them isn't using this. I preserved the `${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake` logic because I wanted to be conservative. What I am certain about is that we aren't using that logic, and wouldn't be sorry to see it go (if that's what you're proposing -- it's not really clear to me where you're heading with this).

I'd suggest getting rid of it since the toolchain file has already been loaded in this case -- which is how we got here in the first place.  Toolchain files set CMAKE_SYSTEM_NAME which is what triggers this, so they've already been loaded and set the appropriate variables, including CMAKE_(C|CXX)_COMPILER.


Repository:
  rL LLVM

https://reviews.llvm.org/D40947