[CMake] Don't pass CMAKE_C(XX)_COMPILER to the nested NATIVE build

[mstorsjo]

Summary

Originally, the nested build was set up with the CMake command
execute_process which implicitly passes CC/CXX variables for
the configured compiler, which then was picked up by the nested
CMake. (This CMake behaviour, to implicitly pass such variables
is up for discussion and might change in the future; see
https://gitlab.kitware.com/cmake/cmake/-/issues/21378.)

How the nested cmake build is set up was changed in
aa7d6db5c8fc449b2908c6d629d6d9a067f49896 / D40229 - the old behaviour
was brought along by manually passing
-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER} to the nested cmake
configuration. This was then later made optional in
f5f0fffea5ace079cc208fafa65150d23935a4d9 / D40947. But still,
the default if the user doesn't pass
CROSS_TOOLCHAIN_FLAGS_${target_name} (e.g. CROSS_TOOLCHAIN_FLAGS_NATIVE)
is to pass in the surrounding build's compiler - which usually
doesn't work, and is quite non-obvious to figure out.

Just drop the default passing of the outer compiler. This should
avoid surprising cases of using the cross compiler for the native
build for essentially all new users trying to cross compile, until
they've discovered CROSS_TOOLCHAIN_FLAGS_NATIVE.

This was already suggested at the end in D40229, but apparently never
acted upon.

[hintonda]

Thanks for looking into this.

I haven't looked into this issue in a while, so I've added a few reviewers who've been more active than I lately, and will try to take a look this week.

[mstorsjo]

Adding @smeenai who also has experience with the cross compilation setup.

[phosek]

I looked at the change and read your description but still don't understand the motivation, Can you explain why is using CMAKE_C(XX)_COMPILER a problem and how do we know that the default that's picked up by CMake will be the correct one?

[mstorsjo]

In D126313#3543213, @phosek wrote:

I looked at the change and read your description but still don't understand the motivation, Can you explain why is using CMAKE_C(XX)_COMPILER a problem

When I cross compile LLVM on linux, targeting Windows, I configure that build with e.g. -DCMAKE_CROSSCOMPILING=TRUE -DCMAKE_SYSTEM_NAME=Windows -DCMAKE_C_COMPILER=x86_64-w64-mingw32-clang -DCMAKE_CXX_COMPILER=x86_64-w64-mingw32-clang++. Now this cross build will try to build native tblgen executables with x86_64-w64-mingw32-clang++, which obviously isn't going to work.

and how do we know that the default that's picked up by CMake will be the correct one?

The assumption is that if you run CMake without CMAKE_CROSSCOMPILING and without specifying CMAKE_CXX_COMPILER (which it will do for the nested native build), it will pick the system's default compiler (cc and c++ on unix, or MSVC and/or gcc on windows, etc) which is supposed to produce executables that can run on the build host.

[phosek]

In D126313#3543240, @mstorsjo wrote:

In D126313#3543213, @phosek wrote:

I looked at the change and read your description but still don't understand the motivation, Can you explain why is using CMAKE_C(XX)_COMPILER a problem

When I cross compile LLVM on linux, targeting Windows, I configure that build with e.g. -DCMAKE_CROSSCOMPILING=TRUE -DCMAKE_SYSTEM_NAME=Windows -DCMAKE_C_COMPILER=x86_64-w64-mingw32-clang -DCMAKE_CXX_COMPILER=x86_64-w64-mingw32-clang++. Now this cross build will try to build native tblgen executables with x86_64-w64-mingw32-clang++, which obviously isn't going to work.

and how do we know that the default that's picked up by CMake will be the correct one?

The assumption is that if you run CMake without CMAKE_CROSSCOMPILING and without specifying CMAKE_CXX_COMPILER (which it will do for the nested native build), it will pick the system's default compiler (cc and c++ on unix, or MSVC and/or gcc on windows, etc) which is supposed to produce executables that can run on the build host.

Thanks for clarification, that helps and I understand the motivation now.

Not passing through CMAKE_C(XX)_COMPILER to the sub-build makes sense, but not setting CMAKE_C(XX)_COMPILER may be a problem. For example, our bots there's no host compiler (intentionally), we fetch the host compiler before the configuration step and pass it explicitly via CMAKE_C(XX)_COMPILER so in our case, with this change the sub-build would fail to find a compiler. We might need another way to set the compiler for the sub-build.

[mstorsjo]

Or put another way - normally, when the nested native build is set up, the user probably doesn't have much opinion on exactly how that is built, just build llvm-tblgen (and the other -tblgen executables) with as little extra frills as possible. If the user specifically wants this to happen with some specific compiler, then it's possible to pass -DCROSS_TOOLCHAIN_FLAGS_NATIVE=-DCMAKE_CXX_COMPILER=clang++ and similar. But for the cases where the user doesn't really care, passing the explicitly cross compiler e.g. <triple>-g++ as tool to use for the native build really never is the right thing to do - and then relying on CMake's implicit default is certainly safer.

(This thing does come up on irc/discord semi-regularly, where currently, users trying to cross compile LLVM will need to pass -DCROSS_TOOLCHAIN_FLAGS_NATIVE=, to explicitly set it to an empty string, in order for it _not_ to pass on the cross compiler.)

[mstorsjo]

In D126313#3543242, @phosek wrote:

Thanks for clarification, that helps and I understand the motivation now.

Not passing through CMAKE_C(XX)_COMPILER to the sub-build makes sense, but not setting CMAKE_C(XX)_COMPILER may be a problem. For example, our bots there's no host compiler (intentionally), we fetch the host compiler before the configuration step and pass it explicitly via CMAKE_C(XX)_COMPILER so in our case, with this change the sub-build would fail to find a compiler. We might need another way to set the compiler for the sub-build.

So in your case, you use the same e.g. clang executable for both host and cross compilation, and just control it with CMAKE_CXX_COMPILER_TARGET on the cross side, and by omitting that option in the native part, it defaults to building for the host?

[smeenai]

This makes sense to me; we set CROSS_TOOLCHAIN_FLAGS_NATIVE to explicitly specify options anyway. I'll let @phosek confirm he's happy with this as well though.

[mstorsjo]

Ping @phosek, can you follow up on this?

[mstorsjo]

@phosek - Can you follow up on this one?

[beanz]

I think this behavior change should only be made if CMAKE_CROSSCOMPILING=True. There are common cases where people configure LLVM targeting the host using a compiler that isn't the default host compiler (think compiling on Linux using clang). Since the Native toolchain is also used for using optimized tablegen when building a debug configuration, this code isn't strictly for cross-compilation.

[mstorsjo]

In D126313#3599863, @beanz wrote:

I think this behavior change should only be made if CMAKE_CROSSCOMPILING=True. There are common cases where people configure LLVM targeting the host using a compiler that isn't the default host compiler (think compiling on Linux using clang). Since the Native toolchain is also used for using optimized tablegen when building a debug configuration, this code isn't strictly for cross-compilation.

Oh, thanks, that's a good point - I hadn't considered that case!

[mstorsjo]

Keep passing the compiler to the nested cmake when not cross compiling.

[beanz]

llvm/cmake/modules/CrossCompile.cmake

19

You can omit this, and instead just have elif( NOT CMAKE_CROSSCOMPILING). Variables that aren't initialized are empty.

[mstorsjo]

Use elseif (NOT CMAKE_CROSSCOMPILING), skipping the dummy empty initialization.

[beanz]

LGTM.

[mstorsjo]

In D126313#3600017, @beanz wrote:

LGTM.

Thanks!

@phosek - I hope this will work for you, and/or is manageable by manually passing those variables in -DCROSS_TOOLCHAIN_FLAGS_NATIVE if needed?

[phosek]

In D126313#3600032, @mstorsjo wrote:

In D126313#3600017, @beanz wrote:

LGTM.

Thanks!

@phosek - I hope this will work for you, and/or is manageable by manually passing those variables in -DCROSS_TOOLCHAIN_FLAGS_NATIVE if needed?

LGTM

[smeenai]

LGTM

llvm/cmake/modules/CrossCompile.cmake

if(NOT DEFINED ${project_name}_${target_name}_BUILD)
     set(${project_name}_${target_name}_BUILD
       ${${project_name}_${target_name}_BUILD} PARENT_SCOPE)
     message(STATUS "Setting native build dir to " ${${project_name}_${target_name}_BUILD})
   endif(NOT DEFINED ${project_name}_${target_name}_BUILD)
 
   if (EXISTS ${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake)
     set(CROSS_TOOLCHAIN_FLAGS_INIT
       -DCMAKE_TOOLCHAIN_FILE=\"${LLVM_MAIN_SRC_DIR}/cmake/platforms/${toolchain}.cmake\")
-  else()
+  elseif (NOT CMAKE_CROSSCOMPILING)
     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}")
 
   # project specific version of the flags up above