Setting the LLVM_TARGET_TRIPLE macro based on the LLVM_DEFAULT_TARGET_TRIPLE

[nicolerabjohn]

Summary

Due to https://reviews.llvm.org/D137870, LLVM_TARGET_TRIPLE is no longer defined on the runtime path into compiler-rt. This patch creates a common block of code to set LLVM_TARGET_TRIPLE equal to the default for both the llvm- and runtime- paths to compiler-rt.

[nicolerabjohn]

This comment was removed by nicolerabjohn.

[nicolerabjohn]

Removing comment to allow for more generic use.

[mgorny]

llvm/cmake/modules/SetTargetTriple.cmake

2

Stray empty line here ;-).

[phosek]

What's the issue you're trying to address? I just landed D137451, does it address your issue?

[daltenty]

In D138864#3956057, @phosek wrote:

What's the issue you're trying to address? I just landed D137451, does it address your issue?

compiler-rt doesn't configure anymore for us on AIX, due to an empty variable expansion (which is derived from LLVM_TARGET_TRIPLE):

$cmake -DLLVM_ENABLE_RUNTIMES=compiler-rt  ../llvm-project/runtimes
...
CMake Error at ../llvm-project/compiler-rt/cmake/Modules/CompilerRTUtils.cmake:367 (string):
  string sub-command REPLACE requires at least four arguments.
Call Stack (most recent call first):
  ../llvm-project/compiler-rt/CMakeLists.txt:120 (construct_compiler_rt_default_triple)

where the offending block is:

    set(COMPILER_RT_DEFAULT_TARGET_TRIPLE ${LLVM_TARGET_TRIPLE} CACHE STRING
          "Default triple for which compiler-rt runtimes will be built.")
...
  string(REPLACE "-" ";" LLVM_TARGET_TRIPLE_LIST ${COMPILER_RT_DEFAULT_TARGET_TRIPLE})

[nicolerabjohn]

Removed extra empty line.

[daltenty]

LGTM. Some paths in compiler-rt still require LLVM_TARGET_TRIPLE, so commoning the setup makes sense

[probinson]

Sorry, what is the semantic difference between LLVM_DEFAULT_TARGET_TRIPLE and LLVM_TARGET_TRIPLE?

[daltenty]

This comment was removed by daltenty.

[daltenty]

I believe @phosek documents the intended semantic here in https://reviews.llvm.org/D137451:

LLVM_DEFAULT_TARGET_TRIPLE is the default value that's used by tools like clang or llc when you don't explicitly specify --target or --triple. LLVM_TARGET_TRIPLE is the target triple we're building for. LLVM_DEFAULT_TARGET_TRIPLE is used as a the default value for LLVM_TARGET_TRIPLE but they're independent.

[probinson]

In D138864#3989941, @daltenty wrote:

I believe @phosek documents the intended semantic here in https://reviews.llvm.org/D137451:

LLVM_DEFAULT_TARGET_TRIPLE is the default value that's used by tools like clang or llc when you don't explicitly specify --target or --triple. LLVM_TARGET_TRIPLE is the target triple we're building for. LLVM_DEFAULT_TARGET_TRIPLE is used as a the default value for LLVM_TARGET_TRIPLE but they're independent.

If I rephrase it this way, does it mean the same thing?
LLVM_DEFAULT_TARGET_TRIPLE is the default for --target or --triple-like options in the tools we are building.
LLVM_TARGET_TRIPLE describes the environment where we expect those tools to run (and could be passed to the compiler being used to build those tools).

The lit tests, which I'm dealing with a great deal these days, don't seem to look at those parameters the same way. Each projects's lit.site.cfg.py.in file sets config.target_triple = "@LLVM_TARGET_TRIPLE@" (except for a couple of outliers that don't set target_triple at all, and maybe the runtimes which hide their lit config files in unobvious places). Now, I'm willing to believe that the CMake configuration parameters have evolved without lit following along, but if we're trying to come to some conclusion about What It All Means, then we need to make sure that lit keeps up.

[probinson]

I have to say, I'm not really convinced that LLVM_TARGET_TRIPLE really means what's mentioned above. It's still about the target, not about the host. CMake conjures up an LLVM_HOST_TRIPLE which the lit config files can capture, and that's the running environment for the built tools.

The discussion about the meanings of these things should be taken to Discourse, not buried in a couple of patch reviews, in any case.

[daltenty]

The discussion about the meanings of these things should be taken to Discourse, not buried in a couple of patch reviews, in any case.

100% agree with you. I think we can straighten things out there, since this is well out of scope of this review.

The matter of fixing the definitions of these macros and long term refactoring aside, we plan to proceed with this patch if there are no objections as it's fixing a build break introduce by D137870 which is still very much a present issue for us.

llvm/CMakeLists.txt

if("${LLVM_HOST_TRIPLE}" MATCHES "^powerpc64-ibm-aix")
   string(REGEX REPLACE "^powerpc64" "powerpc" LLVM_DEFAULT_TARGET_TRIPLE_default "${LLVM_HOST_TRIPLE}")
 else()
   # Only set default triple when native target is enabled.
   if (LLVM_NATIVE_TARGET)
     set(LLVM_DEFAULT_TARGET_TRIPLE_default "${LLVM_HOST_TRIPLE}")
   endif()
 endif()
 
-set(LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE_default}" CACHE STRING
-  "Default target for which LLVM will generate code." )
-if (TARGET_TRIPLE)
-  message(WARNING "TARGET_TRIPLE is deprecated and will be removed in a future release. "
-                  "Please use LLVM_DEFAULT_TARGET_TRIPLE instead.")
-  set(LLVM_TARGET_TRIPLE "${TARGET_TRIPLE}")
-else()
-  set(LLVM_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}")
-endif()
-message(STATUS "LLVM host triple: ${LLVM_HOST_TRIPLE}")
-message(STATUS "LLVM default target triple: ${LLVM_DEFAULT_TARGET_TRIPLE}")
+include(SetTargetTriple)
+set_llvm_target_triple()
 
 if(WIN32 OR CYGWIN)
   if(BUILD_SHARED_LIBS OR LLVM_BUILD_LLVM_DYLIB)
     set(LLVM_ENABLE_PLUGINS_default ON)
   else()
     set(LLVM_ENABLE_PLUGINS_default OFF)
   endif()
 else()

llvm/cmake/modules/SetTargetTriple.cmake

+
+# Common code between the runtimes and LLVM entry point when building libatomic
+# To set LLVM_TARGET_TRIPLE based on the default
+
+macro(set_llvm_target_triple)
+  set(LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE_default}" CACHE STRING
+  "Default target for which LLVM will generate code." )
+  if (TARGET_TRIPLE)
+    message(WARNING "TARGET_TRIPLE is deprecated and will be removed in a future release. "
+    "Please use LLVM_DEFAULT_TARGET_TRIPLE instead.")
+    set(LLVM_TARGET_TRIPLE "${TARGET_TRIPLE}")
+  else()
+    set(LLVM_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}")
+  endif()
+  message(STATUS "LLVM host triple: ${LLVM_HOST_TRIPLE}")
+  message(STATUS "LLVM default target triple: ${LLVM_DEFAULT_TARGET_TRIPLE}")
+endmacro()

runtimes/CMakeLists.txt

 # Use libtool instead of ar if you are both on an Apple host, and targeting Apple.
 if(CMAKE_HOST_APPLE AND APPLE)
   include(UseLibtool)
 endif()
 
 # This can be used to detect whether we're in the runtimes build.
 set(LLVM_RUNTIMES_BUILD ON)
 
+include(SetTargetTriple)
+set_llvm_target_triple()
+
 foreach(entry ${runtimes})
   get_filename_component(projName ${entry} NAME)
 
   # TODO: Clean this up as part of an interface standardization
   string(REPLACE "-" "_" canon_name ${projName})
   string(TOUPPER ${canon_name} canon_name)
 
   # TODO: compiler-rt has to use standalone build for now. We tried to remove