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

[labath]

Summary

r319898 made it possible to override these variables via the
CROSS_TOOLCHAIN_FLAGS setting, but this only worked if one explicitly
specifies these variables there. If, instead, one uses
CROSS_TOOLCHAIN_FLAGS to specify a toolchain file (as our internal
builds do, to point cmake to a checked-in toolchain), the
CMAKE_C(XX)_COMPILER flags would still win over the ones specified by
the toolchain file.

To fix is I make the mere presence of these flags overridable. I do this
by putting them as a default value for the CROSS_TOOLCHAIN_FLAGS
setting, so they can be overridden at cmake configuration time.

[hintonda]

cmake/modules/CrossCompile.cmake

36

Perhaps we could just set CC and CXX:

COMMAND env CC=${CMAKE_C_COMPILER} CXX=${CMAKE_CXX_COMPILER} ${CMAKE_COMMAND}

[labath]

cmake/modules/CrossCompile.cmake

36

env isn't really a thing on windows. llvm tests use it, so we may be able to assume that all windows users have gnuwin32 or similar installed, but I'm still not that fond of it, as there is a difference in requiring gnuwin32 to build, vs. requiring it just for testing. (There's also the possibility to build an env command in cmake https://cmake.org/pipermail/cmake/2011-December/048038.html but that may be a bit too much.)

So, I am not against that, but I would prefer this solution instead (unless we can find some reason it breaks something).

[hintonda]

LGTM.

[hintonda]

cmake/modules/CrossCompile.cmake

11

Will the toolchain file always set the compiler? Any way to guarantee that?

[labath]

cmake/modules/CrossCompile.cmake

11

All toolchain files I've seen set the compiler (that's kinda the whole point).

I suppose one could create a toolchain file that sets all other toolchain-y variables (CMAKE_SYSROOT, CMAKE_AR, CMAKE_CXX_STANDARD_INCLUDE_DIRECTORIES, ...), but leaves the actual choice of the compiler up to the user. However, if one is dealing with a file like that, then the compilers we would specify here would probably not be correct anyway...

[hintonda]

cmake/modules/CrossCompile.cmake

11

Great, and thanks again for cleaning this up.

[hintonda]

Did this fix an existing problem? 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?

[labath]

In 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.

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).

[hintonda]

In D40947#952317, @labath wrote:

In 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.

[hintonda]

In D40947#952330, @hintonda wrote:

In D40947#952317, @labath wrote:

In 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.

Btw, if you actually did include a toolchain file here, and it set CMAKE_SYSTEM_NAME, it could cause an infinite recursion depending on how the call call was made.

[hintonda]

In D40947#952427, @hintonda wrote:

Btw, if you actually did include a toolchain file here, and it set CMAKE_SYSTEM_NAME, it could cause an infinite recursion depending on how the call call was made.

I can confirm that the following file will cause the recursion and ultimate build-time failure:

cat llvm/cmake/platforms/.cmake
set(CMAKE_SYSTEM_NAME Darwin)

"llvm/cmake/platforms/.cmake" matches ${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake when toolchain is blank -- the default case.

cmake/modules/CrossCompile.cmake

 function(llvm_create_cross_target_internal target_name toolchain buildtype)
 
   if(NOT DEFINED LLVM_${target_name}_BUILD)
     set(LLVM_${target_name}_BUILD "${CMAKE_BINARY_DIR}/${target_name}")
     set(LLVM_${target_name}_BUILD ${LLVM_${target_name}_BUILD} PARENT_SCOPE)
     message(STATUS "Setting native build dir to " ${LLVM_${target_name}_BUILD})
   endif(NOT DEFINED LLVM_${target_name}_BUILD)
 
   if (EXISTS ${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake)
-    set(CROSS_TOOLCHAIN_FLAGS_${target_name} 
-        -DCMAKE_TOOLCHAIN_FILE=\"${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake\"
-        CACHE STRING "Toolchain file for ${target_name}")
+    set(CROSS_TOOLCHAIN_FLAGS_INIT
+      -DCMAKE_TOOLCHAIN_FILE=\"${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake\")
+  else()
+    set(CROSS_TOOLCHAIN_FLAGS_INIT
+      -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
+      -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
+      )
   endif()
+  set(CROSS_TOOLCHAIN_FLAGS_${target_name} ${CROSS_TOOLCHAIN_FLAGS_INIT}
+    CACHE STRING "Toolchain configuration for ${target_name}")
 
   if (buildtype)
     set(build_type_flags "-DCMAKE_BUILD_TYPE=${buildtype}")
   endif()
   if (LLVM_USE_LINKER AND NOT CMAKE_CROSSCOMPILING)
     set(linker_flag "-DLLVM_USE_LINKER=${LLVM_USE_LINKER}")
   endif()
   if (LLVM_EXTERNAL_CLANG_SOURCE_DIR)
     # Propagate LLVM_EXTERNAL_CLANG_SOURCE_DIR so that clang-tblgen can be built
     set(external_clang_dir "-DLLVM_EXTERNAL_CLANG_SOURCE_DIR=${LLVM_EXTERNAL_CLANG_SOURCE_DIR}")
   endif()
 
   add_custom_command(OUTPUT ${LLVM_${target_name}_BUILD}
     COMMAND ${CMAKE_COMMAND} -E make_directory ${LLVM_${target_name}_BUILD}
     COMMENT "Creating ${LLVM_${target_name}_BUILD}...")
 
   add_custom_target(CREATE_LLVM_${target_name}
                     DEPENDS ${LLVM_${target_name}_BUILD})
 
   add_custom_command(OUTPUT ${LLVM_${target_name}_BUILD}/CMakeCache.txt
     COMMAND ${CMAKE_COMMAND} -G "${CMAKE_GENERATOR}"
-        -DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
-        -DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
         ${CROSS_TOOLCHAIN_FLAGS_${target_name}} ${CMAKE_SOURCE_DIR}
         -DLLVM_TARGET_IS_CROSSCOMPILE_HOST=TRUE
         -DLLVM_TARGETS_TO_BUILD=Native
         ${build_type_flags} ${linker_flag} ${external_clang_dir}
     WORKING_DIRECTORY ${LLVM_${target_name}_BUILD}
     DEPENDS CREATE_LLVM_${target_name}
     COMMENT "Configuring ${target_name} LLVM...")