Disable compiler launcher on external projects and multi stage clang

[haowei]

Summary

When using compiler caching programs like ccache, there is no point to use them on external projects or multi-stage clang builds. As these builds use fresh from source code toolchain and will pollute the build cache. If a compiler launcher is still required, a user can explicitly define CMAKE_C_COMPILER_LAUNCHER and CMAKE_CXX_COMPILER_LAUNCHER in CLANG_BOOTSTRAP_PASSTHROUGH and LLVM_EXTERNAL_PROJECT_PASSTHROUGHflags to enable compiler launcher in these builds.

[phosek]

A more generic version might be to have a list of variables to (not) passthrough.

First option is to remove {C,CXX}_COMPILER_LAUNCHER from _BOOTSTRAP_DEFAULT_PASSTHROUGH and then include it as needed by setting something like CLANG_BOOTSTRAP_EXTRA_PASSTHROUGH=C_COMPILER_LAUNCHER;CXX_COMPILER_LAUNCHER.
Second option is to provide a way to filter out variable from passthrough, for example CLANG_BOOTSTRAP_NO_PASSTHROUGH=C_COMPILER_LAUNCHER;CXX_COMPILER_LAUNCHER.

I slightly prefer the first option, but it'd be a breaking change, so the second option might be the only feasible one.

llvm/cmake/modules/LLVMExternalProjectUtils.cmake

318–323

I think it's better to avoid setting these at all if disabled rather than setting them to an empty string. You can just append these to cmake_args.

[phosek]

@beanz @smeenai do you have any preferences/other ideas?

[smeenai]

Hmm, what cache key does ccache use for compilers? Is it the --version output string, the path, or some combination? If the path is involved then I don't see any value in using ccache for configuring anything past stage1, since the compiler will (presumably) be installed somewhere else afterwards and not used directly from the build tree. If it's just --version, what are the cache pollution concerns? The compiler binaries output by all the stages should produce the same outputs given the same inputs (and presumably have the same --version), right?

[haowei]

In D144603#4162048, @smeenai wrote:

Hmm, what cache key does ccache use for compilers? Is it the --version output string, the path, or some combination? If the path is involved then I don't see any value in using ccache for configuring anything past stage1, since the compiler will (presumably) be installed somewhere else afterwards and not used directly from the build tree. If it's just --version, what are the cache pollution concerns? The compiler binaries output by all the stages should produce the same outputs given the same inputs (and presumably have the same --version), right?

I am not sure about ccache. We use an internal distributed build cache system when we build LLVM. The compiler binary hash and the --version output will both be used to calculate the cache key so there is little value to use caching past stage1 as the runtime build and stage2+ will be built from a fresh from source compiler and it will always result in cache miss.

I don't believe ccache only uses --version output as cache key since two compiler binary with the same version string can have different output (e.g. 2 WIP source tree but track the same git revision).

So think it would be beneficial to add a flag to avoid using compiler launcher past stage1.

[smeenai]

In D144603#4162192, @haowei wrote:

In D144603#4162048, @smeenai wrote:

Hmm, what cache key does ccache use for compilers? Is it the --version output string, the path, or some combination? If the path is involved then I don't see any value in using ccache for configuring anything past stage1, since the compiler will (presumably) be installed somewhere else afterwards and not used directly from the build tree. If it's just --version, what are the cache pollution concerns? The compiler binaries output by all the stages should produce the same outputs given the same inputs (and presumably have the same --version), right?

I am not sure about ccache. We use an internal distributed build cache system when we build LLVM. The compiler binary hash and the --version output will both be used to calculate the cache key so there is little value to use caching past stage1 as the runtime build and stage2+ will be built from a fresh from source compiler and it will always result in cache miss.

I don't believe ccache only uses --version output as cache key since two compiler binary with the same version string can have different output (e.g. 2 WIP source tree but track the same git revision).

So think it would be beneficial to add a flag to avoid using compiler launcher past stage1.

Yeah, that sounds good. I also prefer @phosek's first option of letting users opt in to compiler launchers for boostrapping builds if they want, since I don't see a lot of value in doing that in general, but being conservative and having an option for variables to not pass through is likely the most expedient approach here, and that works for me.

[phosek]

I agree, in that case let's remove C_COMPILER_LAUNCHER;CXX_COMPILER_LAUNCHER from the list of default passthrough variables which seems like a reasonable default, and provide CLANG_BOOTSTRAP_EXTRA_PASSTHROUGH so developers have a way to pass these through to the next stage if they want to.

[haowei]

In D144603#4162223, @phosek wrote:

I agree, in that case let's remove C_COMPILER_LAUNCHER;CXX_COMPILER_LAUNCHER from the list of default passthrough variables which seems like a reasonable default, and provide CLANG_BOOTSTRAP_EXTRA_PASSTHROUGH so developers have a way to pass these through to the next stage if they want to.

There is already a CLANG_BOOTSTRAP_PASSTHROUGH flag https://github.com/llvm/llvm-project/blob/dc1c8917afd3f2b306797890a56be66087feb832/clang/CMakeLists.txt#L749 that works in the way you describe the CLANG_BOOTSTRAP_EXTRA_PASSTHROUGH flag.

I think remove the compiler launcher from default pass through flags are fine. What about the using compiler launcher in runtime builds? I don't think we should read CLANG_BOOTSTRAP_PASSTHROUGH to determine if we should pass through compiler launcher flags.

[phosek]

In D144603#4162974, @haowei wrote:

I think remove the compiler launcher from default pass through flags are fine. What about the using compiler launcher in runtime builds? I don't think we should read CLANG_BOOTSTRAP_PASSTHROUGH to determine if we should pass through compiler launcher flags.

I'd suggest a similar approach, that is remove C_COMPILER_LAUNCHER and CXX_COMPILER_LAUNCHER from the default passthrough list and provide a new LLVM_EXTERNAL_PROJECT_PASSTHROUGH variable.

[fangism]

In D144603#4163751, @phosek wrote:

In D144603#4162974, @haowei wrote:

I think remove the compiler launcher from default pass through flags are fine. What about the using compiler launcher in runtime builds? I don't think we should read CLANG_BOOTSTRAP_PASSTHROUGH to determine if we should pass through compiler launcher flags.

I'd suggest a similar approach, that is remove C_COMPILER_LAUNCHER and CXX_COMPILER_LAUNCHER from the default passthrough list and provide a new LLVM_EXTERNAL_PROJECT_PASSTHROUGH variable.

Will the PASSTHROUGH approach let us choose a different LAUNCHER for the external projects? For example, we could still distribute those compiles, albeit without expecting any caching.

[phosek]

In D144603#4165569, @fangism wrote:

In D144603#4163751, @phosek wrote:

In D144603#4162974, @haowei wrote:

I think remove the compiler launcher from default pass through flags are fine. What about the using compiler launcher in runtime builds? I don't think we should read CLANG_BOOTSTRAP_PASSTHROUGH to determine if we should pass through compiler launcher flags.

I'd suggest a similar approach, that is remove C_COMPILER_LAUNCHER and CXX_COMPILER_LAUNCHER from the default passthrough list and provide a new LLVM_EXTERNAL_PROJECT_PASSTHROUGH variable.

Will the PASSTHROUGH approach let us choose a different LAUNCHER for the external projects? For example, we could still distribute those compiles, albeit without expecting any caching.

No, we would need a different option for that. I'd start with the passthrough approach which is generic and then look into the possibility of overriding the launcher for subprojects if there's a compelling use case.

[fangism]

What's the current status of this changeset?

[phosek]

LGTM

clang/CMakeLists.txt

set(_BOOTSTRAP_DEFAULT_PASSTHROUGH
     LLVM_VERSION_PATCH
     CLANG_VERSION_MAJOR
     CLANG_VERSION_MINOR
     CLANG_VERSION_PATCHLEVEL
     CLANG_VENDOR
     LLVM_VERSION_SUFFIX
     LLVM_BINUTILS_INCDIR
     CLANG_REPOSITORY_STRING
-    CMAKE_C_COMPILER_LAUNCHER
-    CMAKE_CXX_COMPILER_LAUNCHER
     CMAKE_MAKE_PROGRAM
     CMAKE_OSX_ARCHITECTURES
     CMAKE_BUILD_TYPE
     LLVM_ENABLE_PROJECTS
     LLVM_ENABLE_RUNTIMES)
 
   # We don't need to depend on compiler-rt/libcxx if we're building instrumented
   # because the next stage will use the same compiler used to build this stage.

llvm/cmake/modules/LLVMExternalProjectUtils.cmake

foreach(variableName ${variableNames})
       if(variableName MATCHES "^${prefix}")
         string(REPLACE ";" "|" value "${${variableName}}")
         list(APPEND PASSTHROUGH_VARIABLES
           -D${variableName}=${value})
       endif()
     endforeach()
   endforeach()
 
+  # Populate the non-project-specific passthrough variables
+  foreach(variableName ${LLVM_EXTERNAL_PROJECT_PASSTHROUGH})
+    if(DEFINED ${variableName})
+      if("${${variableName}}" STREQUAL "")
+        set(value "")
+      else()
+        string(REPLACE ";" "|" value "${${variableName}}")
+      endif()
+      list(APPEND PASSTHROUGH_VARIABLES
+        -D${variableName}=${value})
+    endif()
+  endforeach()
+
   if(ARG_USE_TOOLCHAIN AND NOT CMAKE_CROSSCOMPILING)
     if(CLANG_IN_TOOLCHAIN)
       if(is_msvc_target)
         set(compiler_args -DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang-cl${CMAKE_EXECUTABLE_SUFFIX}
                           -DCMAKE_CXX_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang-cl${CMAKE_EXECUTABLE_SUFFIX}
                           -DCMAKE_ASM_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang-cl${CMAKE_EXECUTABLE_SUFFIX})
       else()
         set(compiler_args -DCMAKE_C_COMPILER=${LLVM_RUNTIME_OUTPUT_INTDIR}/clang${CMAKE_EXECUTABLE_SUFFIX}
function(llvm_ExternalProject_Add name source_dir)
   endif()
 
   if(ARG_TARGET_TRIPLE)
     list(APPEND compiler_args -DCMAKE_C_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
     list(APPEND compiler_args -DCMAKE_CXX_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
     list(APPEND compiler_args -DCMAKE_ASM_COMPILER_TARGET=${ARG_TARGET_TRIPLE})
   endif()
 
   ExternalProject_Add(${name}
     DEPENDS ${ARG_DEPENDS} llvm-config
     ${name}-clobber
     PREFIX ${CMAKE_BINARY_DIR}/projects/${name}
     SOURCE_DIR ${source_dir}
     STAMP_DIR ${STAMP_DIR}
     BINARY_DIR ${BINARY_DIR}
     ${exclude}
     CMAKE_ARGS ${${nameCanon}_CMAKE_ARGS}
                --no-warn-unused-cli
                ${compiler_args}
                -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
                ${sysroot_arg}
                -DLLVM_BINARY_DIR=${PROJECT_BINARY_DIR}
                -DLLVM_CONFIG_PATH=${llvm_config_path}
                -DLLVM_ENABLE_WERROR=${LLVM_ENABLE_WERROR}
                -DLLVM_HOST_TRIPLE=${LLVM_HOST_TRIPLE}
                -DLLVM_HAVE_LINK_VERSION_SCRIPT=${LLVM_HAVE_LINK_VERSION_SCRIPT}
                -DLLVM_USE_RELATIVE_PATHS_IN_DEBUG_INFO=${LLVM_USE_RELATIVE_PATHS_IN_DEBUG_INFO}
                -DLLVM_USE_RELATIVE_PATHS_IN_FILES=${LLVM_USE_RELATIVE_PATHS_IN_FILES}
                -DLLVM_LIT_ARGS=${LLVM_LIT_ARGS}
                -DLLVM_SOURCE_PREFIX=${LLVM_SOURCE_PREFIX}
                -DPACKAGE_VERSION=${PACKAGE_VERSION}
                -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
                -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}
-               -DCMAKE_C_COMPILER_LAUNCHER=${CMAKE_C_COMPILER_LAUNCHER}
-               -DCMAKE_CXX_COMPILER_LAUNCHER=${CMAKE_CXX_COMPILER_LAUNCHER}
                -DCMAKE_EXPORT_COMPILE_COMMANDS=1
                ${cmake_args}
                ${PASSTHROUGH_VARIABLES}
     INSTALL_COMMAND ""
     STEP_TARGETS configure build
     BUILD_ALWAYS 1
     USES_TERMINAL_CONFIGURE 1
     USES_TERMINAL_BUILD 1