Adding support for overriding LLVM_ENABLE_RUNTIMES for runtimes builds.

[plotfi]

Summary

On some platforms, certain runtimes are not supported. For runtimes builds of those platforms it would be nice if we could disable certain runtimes.

[beanz]

I feel like the default behavior should be that if you set LLVM_ENABLE_RUNTIMES it builds them for all targets you request. If you want to disable some runtimes for certain targets that should be an opt-out setting rather than an opt-in to build for targets.

[compnerd]

@beanz - I can see where you are coming from there. But, the thing is that when you are building for a wide variety of targets at once, that doesn't make sense since not all the runtimes are compatible with all targets (e.g. unwind doesn't support Windows). Calculating the set difference and passing that along seems more complicated, though plausible. Note that this approach preserves the existing behaviour - unless you *explicitly* state the runtimes for a specific target, the LLVM_ENABLE_RUNTIMES passed globally will continue to be passed through, so from a user's perspective nothing has changed (and that is intentional - no one should need to update their CMake caches for this change).

[plotfi]

Ping?

[beanz]

@compnerd, it doesn't look like LLVM_ENABLE_RUNTIMES passed globally is still passed through if you are generating runtimes for multiple targets. It looks like that only gets passed through if you specify RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES.

That looks like a behavior change to me.

[compnerd]

@beanz - I must be mistaken about something. The condition is if LLVM_ENABLE_RUNTIMES is NOT specified for the current target, we pass in the global value (which was previously passed via the PASSTHROUGH_PREFIXES), and if it is specified, we will pass in the specified value. So, the only behaviour change that is visible is if the user specified a target specific LLVM_ENABLE_RUNTIMES value it is now honoured.

[beanz]

I see. Sorry for the noise. This looks fine.

@plotfi in the future when you upload diffs, please provide full context, it makes reading and understanding what is going on way easier.

[leonardchan]

Hi. I think this might be causing some linker failures on our x64 bots:

[304/396] Linking CXX shared library /b/s/w/ir/k/recipe_cleanup/clangdPn0KT/llvm_build_dir/lib/clang/10.0.0/lib/x86_64-unknown-fuchsia/libclang_rt.ubsan_standalone.so
FAILED: /b/s/w/ir/k/recipe_cleanup/clangdPn0KT/llvm_build_dir/lib/clang/10.0.0/lib/x86_64-unknown-fuchsia/libclang_rt.ubsan_standalone.so 
... The long link invocation
ld.lld: error: undefined symbol: _Unwind_GetIP
>>> referenced by sanitizer_symbolizer_markup.cpp:113 (compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp:113)
>>>               compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.x86_64.dir/sanitizer_symbolizer_markup.cpp.obj:(__sanitizer::Unwind_Trace(_Unwind_Context*, void*))

ld.lld: error: undefined symbol: _Unwind_Backtrace
>>> referenced by sanitizer_symbolizer_markup.cpp:124 (compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp:124)
>>>               compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.x86_64.dir/sanitizer_symbolizer_markup.cpp.obj:(__sanitizer::BufferedStackTrace::UnwindSlow(unsigned long, unsigned int))

ld.lld: error: undefined symbol: typeinfo for std::type_info
>>> referenced by ubsan_type_hash_itanium.cpp:231 (compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp:231)
>>>               compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_cxx.x86_64.dir/ubsan_type_hash_itanium.cpp.obj:(__ubsan::checkDynamicType(void*, void*, unsigned long))
... A bunch more link errors

Could you take a look? Thanks.

[plotfi]

Where are your bots?

PL

In D67195#1665496, @leonardchan wrote:

Hi. I think this might be causing some linker failures on our x64 bots:

[304/396] Linking CXX shared library /b/s/w/ir/k/recipe_cleanup/clangdPn0KT/llvm_build_dir/lib/clang/10.0.0/lib/x86_64-unknown-fuchsia/libclang_rt.ubsan_standalone.so
FAILED: /b/s/w/ir/k/recipe_cleanup/clangdPn0KT/llvm_build_dir/lib/clang/10.0.0/lib/x86_64-unknown-fuchsia/libclang_rt.ubsan_standalone.so 
... The long link invocation
ld.lld: error: undefined symbol: _Unwind_GetIP
>>> referenced by sanitizer_symbolizer_markup.cpp:113 (compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp:113)
>>>               compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.x86_64.dir/sanitizer_symbolizer_markup.cpp.obj:(__sanitizer::Unwind_Trace(_Unwind_Context*, void*))

ld.lld: error: undefined symbol: _Unwind_Backtrace
>>> referenced by sanitizer_symbolizer_markup.cpp:124 (compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_markup.cpp:124)
>>>               compiler-rt/lib/sanitizer_common/CMakeFiles/RTSanitizerCommonSymbolizer.x86_64.dir/sanitizer_symbolizer_markup.cpp.obj:(__sanitizer::BufferedStackTrace::UnwindSlow(unsigned long, unsigned int))

ld.lld: error: undefined symbol: typeinfo for std::type_info
>>> referenced by ubsan_type_hash_itanium.cpp:231 (compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp:231)
>>>               compiler-rt/lib/ubsan/CMakeFiles/RTUbsan_cxx.x86_64.dir/ubsan_type_hash_itanium.cpp.obj:(__ubsan::checkDynamicType(void*, void*, unsigned long))
... A bunch more link errors

Could you take a look? Thanks.

[leonardchan]

You can see the bot at: https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-x64-linux/b8902661942095266432

Build failure: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8902661942095266432/+/steps/clang/0/steps/build/0/stdout?format=raw

And cmake invocation if it helps: https://logs.chromium.org/logs/fuchsia/buildbucket/cr-buildbucket.appspot.com/8902661942095266432/+/steps/clang/0/steps/configure/0/logs/execution_details/0

[phosek]

I think I see the issue, if you look at LLVMExternalProjectUtils.cmake, it uses the following logic for PASSTHROUGH_PREFIXES:

list(APPEND ARG_PASSTHROUGH_PREFIXES ${nameCanon})
foreach(prefix ${ARG_PASSTHROUGH_PREFIXES})
  foreach(variableName ${variableNames})
    if(variableName MATCHES "^${prefix}")
      string(REPLACE ";" "|" value "${${variableName}}")
      list(APPEND PASSTHROUGH_VARIABLES
        -D${variableName}=${value})
    endif()
  endforeach()
endforeach()

We then set LIST_SEPARATOR | when invoking ExternalProject_Add.

Changing the list separator is really critical when passing through lists, as is the case for LLVM_ENABLE_RUNTIMES where the value would typically be -DLLVM_ENABLE_RUNTIMES=compiler-rt;libcxx;libcxxabi;libunwind. With your change, only the first value (i.e. compiler-rt) would be passed through to the llvm_ExternalProject_Add and the rest will be dropped hence this error we're seeing (because none of the other runtimes was built).

[plotfi]

Nice! Thanks @phosek. I’m fine with a revert. Currently away from keyboard.

In D67195#1665516, @phosek wrote:

I think I see the issue, if you look at LLVMExternalProjectUtils.cmake, it uses the following logic for PASSTHROUGH_PREFIXES:

list(APPEND ARG_PASSTHROUGH_PREFIXES ${nameCanon})
foreach(prefix ${ARG_PASSTHROUGH_PREFIXES})
  foreach(variableName ${variableNames})
    if(variableName MATCHES "^${prefix}")
      string(REPLACE ";" "|" value "${${variableName}}")
      list(APPEND PASSTHROUGH_VARIABLES
        -D${variableName}=${value})
    endif()
  endforeach()
endforeach()

We then set LIST_SEPARATOR | when invoking ExternalProject_Add.

Changing the list separator is really critical when passing through lists, as is the case for LLVM_ENABLE_RUNTIMES where the value would typically be -DLLVM_ENABLE_RUNTIMES=compiler-rt;libcxx;libcxxabi;libunwind. With your change, only the first value (i.e. compiler-rt) would be passed through to the llvm_ExternalProject_Add and the rest will be dropped hence this error we're seeing (because none of the other runtimes was built).

[plotfi]

s/;/|/g

runtimes/CMakeLists.txt

       endif()
     endforeach()
 
+    if(NOT RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES)
+      string(REPLACE ";" "|" LLVM_ENABLE_RUNTIMES_PASSTHROUGH "${LLVM_ENABLE_RUNTIMES}")
+      list(APPEND ${name}_extra_args -DLLVM_ENABLE_RUNTIMES=${LLVM_ENABLE_RUNTIMES_PASSTHROUGH})
+    endif()
+
     if(target IN_LIST LLVM_RUNTIME_BUILD_ID_LINK_TARGETS)
       list(APPEND EXTRA_ARGS STRIP_TOOL ${CMAKE_CURRENT_BINARY_DIR}/llvm-strip-link)
     endif()

                                         -DCOMPILER_RT_DEFAULT_TARGET_ONLY=ON
                                         -DLLVM_RUNTIMES_TARGET=${name}
                                         ${${name}_extra_args}
-                             PASSTHROUGH_PREFIXES LLVM_ENABLE_RUNTIMES
                              TOOLCHAIN_TOOLS clang lld llvm-ar llvm-ranlib llvm-nm llvm-objcopy llvm-objdump llvm-strip
                              EXTRA_TARGETS ${${name}_extra_targets}
                                            ${${name}_test_targets}