[runtimes] Don't override LLVM_ENABLE_PER_TARGET_RUNTIME_DIR set from llvm

[DavidSpickett]

Summary

LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is set in llvm/CMakeLists.txt
and in llvm/runtimes/CMakeLists.txt.

This meant that anything you passed down, or any platform not using
this layout yet would have it enabled despite it being OFF earlier.

To fix this, check if we have already defined the variable
and if so, use that value.

bultin_register_target I don't fully understand the purpose of.
So for now I have left it setting the value to ON. The rest will
respect what was previously set.

[DavidSpickett]

Background is that I'm using the release build script to test the new library layout for armhf, and first step to that was getting a working build without any changes.

I have left the functions that do =ON alone, though maybe they need this check too. I didn't hit them in the builds I've tried.

The other thing I don't know is if we should be putting the same opt out/ins in llvm and runtimes for this setting. llvm turns it ON for BSD or Linux, except when it's on Arm. runtimes disables it for AIX. Perhaps they could be shared?

Problem with that is that I think you can build without using only the runtimes cmake, is that the case? If so, you'd need to have the opt in/outs in both places.

[DavidSpickett]

+ @daltenty on the off chance it makes sense to move the AIX check around. There is currently no AIX check in llvm/CMakeLists.txt

[DavidSpickett]

llvm/runtimes/CMakeLists.txt

78

I don't know why the _default variable was used here, I don't think it matters too much.

With this change, if you add -DLLVM_ENABLE_PER... to your cmake command line, it will be passed down to this function as expected.

[DavidSpickett]

• Move code to a macro
• Use it in more functions that I hit testing the release build

[MaskRay]

I do not fully understand the logic but the diff seems reasonable.

[DavidSpickett]

Thanks for the review! I'll also wait to see what @phosek says.

The thing I'm not sure about is what level should set the default for this option.

[DavidSpickett]

ping @phosek Does the approach to this look good to you?

Given that this is a global setting, handling it like you suggested in https://reviews.llvm.org/D140005 does not seem the way to go.

[DavidSpickett]

Rebase onto main.

[DavidSpickett]

Rebase

llvm/runtimes/CMakeLists.txt

 ${error} Set RUNTIMES_BUILD_ALLOW_DARWIN to allow a single darwin triple.")
       endif()
     elseif(component_lower MATCHES "^ios|^macos|^tvos|^watchos")
       message(FATAL_ERROR "${error}")
     endif()
   endforeach()
 endfunction()
 
-function(builtin_default_target compiler_rt_path)
-  cmake_parse_arguments(ARG "" "" "DEPENDS" ${ARGN})
-
-  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)
+macro(set_enable_per_target_runtime_dir)
+  # May have been set by llvm/CMakeLists.txt.
+  if (NOT DEFINED LLVM_ENABLE_PER_TARGET_RUNTIME_DIR)
     # AIX should fold 32-bit & 64-bit arch libraries into a single archive.
     if (LLVM_TARGET_TRIPLE MATCHES "aix")
-    set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default OFF)
+      set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR OFF)
+    else()
+      set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON)
     endif()
+  endif()
+endmacro()
+
+function(builtin_default_target compiler_rt_path)
+  cmake_parse_arguments(ARG "" "" "DEPENDS" ${ARGN})
+
+  set_enable_per_target_runtime_dir()
 
   llvm_ExternalProject_Add(builtins
                            ${compiler_rt_path}/lib/builtins
                            DEPENDS ${ARG_DEPENDS}
                            CMAKE_ARGS -DLLVM_LIBRARY_OUTPUT_INTDIR=${LLVM_LIBRARY_DIR}
                                       -DLLVM_RUNTIME_OUTPUT_INTDIR=${LLVM_TOOLS_BINARY_DIR}
                                       -DLLVM_DEFAULT_TARGET_TRIPLE=${LLVM_TARGET_TRIPLE}
-                                      -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=${LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default}
+                                      -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=${LLVM_ENABLE_PER_TARGET_RUNTIME_DIR}
                                       -DCMAKE_C_COMPILER_WORKS=ON
                                       -DCMAKE_ASM_COMPILER_WORKS=ON
                                       ${COMMON_CMAKE_ARGS}
                                       ${BUILTINS_CMAKE_ARGS}
                            PASSTHROUGH_PREFIXES COMPILER_RT
                                                 DARWIN
                                                 SANITIZER
                            USE_TOOLCHAIN
foreach(component ${LLVM_RUNTIME_DISTRIBUTION_COMPONENTS})
     endif()
   endforeach()
 
   if(LLVM_INCLUDE_TESTS)
     set_property(GLOBAL APPEND PROPERTY LLVM_ALL_LIT_TESTSUITES "@${LLVM_BINARY_DIR}/runtimes/runtimes-bins/lit.tests")
     list(APPEND test_targets runtimes-test-depends check-runtimes)
   endif()
 
-  set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default ON)
-  # AIX should fold 32-bit & 64-bit arch libraries into a single archive.
-  if (LLVM_TARGET_TRIPLE MATCHES "aix")
-    set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default OFF)
-  endif()
+  set_enable_per_target_runtime_dir()
 
   llvm_ExternalProject_Add(runtimes
                            ${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
                            DEPENDS ${ARG_DEPENDS}
                            # Builtins were built separately above
                            CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
                                       -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
                                       -DLLVM_DEFAULT_TARGET_TRIPLE=${LLVM_TARGET_TRIPLE}
                                       -DLLVM_ENABLE_PROJECTS_USED=${LLVM_ENABLE_PROJECTS_USED}
-                                      -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=${LLVM_ENABLE_PER_TARGET_RUNTIME_DIR_default}
+                                      -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=${LLVM_ENABLE_PER_TARGET_RUNTIME_DIR}
                                       -DLLVM_BUILD_TOOLS=${LLVM_BUILD_TOOLS}
                                       -DCMAKE_C_COMPILER_WORKS=ON
                                       -DCMAKE_CXX_COMPILER_WORKS=ON
                                       -DCMAKE_ASM_COMPILER_WORKS=ON
                                       ${COMMON_CMAKE_ARGS}
                                       ${RUNTIMES_CMAKE_ARGS}
                            PASSTHROUGH_PREFIXES LLVM_ENABLE_RUNTIMES
                                                 LLVM_USE_LINKER
if(NOT RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES AND NOT RUNTIMES_${target}_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(NOT RUNTIMES_${name}_LLVM_USE_LINKER AND NOT RUNTIMES_${target}_LLVM_USE_LINKER)
     list(APPEND ${name}_extra_args -DLLVM_USE_LINKER=${LLVM_USE_LINKER})
   endif()
 
+  set_enable_per_target_runtime_dir()
+
   llvm_ExternalProject_Add(runtimes-${name}
                            ${CMAKE_CURRENT_SOURCE_DIR}/../../runtimes
                            DEPENDS ${${name}_deps}
                            # Builtins were built separately above
                            CMAKE_ARGS -DCOMPILER_RT_BUILD_BUILTINS=Off
                                       -DLLVM_INCLUDE_TESTS=${LLVM_INCLUDE_TESTS}
                                       -DLLVM_DEFAULT_TARGET_TRIPLE=${target}
                                       -DLLVM_ENABLE_PROJECTS_USED=${LLVM_ENABLE_PROJECTS_USED}
-                                      -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=ON
+                                      -DLLVM_ENABLE_PER_TARGET_RUNTIME_DIR=${LLVM_ENABLE_PER_TARGET_RUNTIME_DIR}
                                       -DCMAKE_C_COMPILER_WORKS=ON
                                       -DCMAKE_CXX_COMPILER_WORKS=ON
                                       -DCMAKE_ASM_COMPILER_WORKS=ON
                                       -DCOMPILER_RT_DEFAULT_TARGET_ONLY=ON
                                       -DLLVM_RUNTIMES_TARGET=${name}
                                       ${COMMON_CMAKE_ARGS}
                                       ${${name}_extra_args}
                            EXTRA_TARGETS ${${name}_extra_targets}