Respect LLVM_HOST_TRIPLE when manually specified

[ldrumm]

Summary

If the user specifies -DLLVM_HOST_TRIPLE=... this should override the guess made by CMake

This appears necessary when cross-compiling Android lldb-server on win32 where the host triple guessed by cmake interferes with the target triple

[beanz]

Can you explain why this is necessary when cross-targeting? I kinda feel that the problem is more likely that the LLVM_HOST_TRIPLE is being used in places where a *target* triple should be used.

[ldrumm]

In D22370#484620, @beanz wrote:

Can you explain why this is necessary when cross-targeting? I kinda feel that the problem is more likely that the LLVM_HOST_TRIPLE is being used in places where a *target* triple should be used.

The issue here is that on a win32 machine with Visual Studio installed, MSVC is set to true because CMake picks up the MSVC installation and immediately enter the if (MSVC) block on line 5. However, the LLVM_HOST_TRIPLE triple should be the native triple of the cross-compiler. For example I might be building an lldb-server to run on an arm android device, and use the gcc4.9 cross compiler for as part of the android native development kit which requires the following invocation for cmake

cmake.exe %LLVM_SRC_PATH% -GNinja -DCMAKE_TOOLCHAIN_FILE=%LLVM_SRC_PATH\tools\lldb\cmake\platforms\Android.cmake -DANDROID_TOOLCHAIN_DIR=%CROSS_COMPILER_PATH% -DANDROID_ABI=armeabi -DCMAKE_CXX_COMPILER_VERSION=4.9 -DLLVM_TARGETS_TO_BUILD=ARM -DLLVM_HOST_TRIPLE=arm-unknown-linux-android -DLLVM_TABLEGEN=host-tblgen\\bin\\llvm-tblgen -DCLANG_TABLEGEN=host-tblgen\\bin\\clang-tblgen

However, the configure fails without this patch:

CMake Error at cmake/modules/GetHostTriple.cmake:24 (message):

Failed to execute C:/Users/luke/jenkins-workspace/llvm/cmake/config.guess

Call Stack (most recent call first):

cmake/config-ix.cmake:352 (get_host_triple)
CMakeLists.txt:460 (include)

In any case, I think being able to manually specify the LLVM_HOST_TRIPLE on the CMake invocation step to be useful to those that are dealing with more complex cross-compilation setups, and for those that don't care, the behaviour won't change because this configure step will simply fall through to the original host triple step.

[beanz]

I very much believe you are doing something wrong.

Your description of the problem doesn't match the error message, and I can tell you that the solution *isn't* to make get_host_triple return anything that isn't actually the host triple.

This is the error you posted:

CMake Error at cmake/modules/GetHostTriple.cmake:24 (message):

That code is only hit if MSVC is false. You're in the else block of an if(MSVC). The problem is actually that if(MSVC) isn't really the right check. Can you give this patch a try?

diff --git a/cmake/modules/GetHostTriple.cmake b/cmake/modules/GetHostTriple.cmake
index 5de710c..e3ecc5d 100644
--- a/cmake/modules/GetHostTriple.cmake
+++ b/cmake/modules/GetHostTriple.cmake
@@ -2,18 +2,18 @@
 # Invokes config.guess
 
 function( get_host_triple var )
-  if( MSVC )
-    if( CMAKE_CL_64 )
-      set( value "x86_64-pc-win32" )
-    else()
-      set( value "i686-pc-win32" )
-    endif()
-  elseif( MINGW AND NOT MSYS )
+  if( MINGW AND NOT MSYS )
     if( CMAKE_SIZEOF_VOID_P EQUAL 8 )
       set( value "x86_64-w64-mingw32" )
     else()
       set( value "i686-pc-mingw32" )
     endif()
+  elseif( CMAKE_HOST_WIN32 )
+    if( CMAKE_CL_64 )
+      set( value "x86_64-pc-win32" )
+    else()
+      set( value "i686-pc-win32" )
+    endif()
   else( MSVC )
     set(config_guess ${LLVM_MAIN_SRC_DIR}/cmake/config.guess)
     execute_process(COMMAND sh ${config_guess}

-Chris

[ldrumm]

In D22370#485442, @beanz wrote:

I very much believe you are doing something wrong.

Your description of the problem doesn't match the error message, and I can tell you that the solution *isn't* to make get_host_triple return anything that isn't actually the host triple.

The fundamental use case I'm trying to enable is enabling specifying the host triple. If the user is in a position to manually specify the triple, I think we should allow that. I think that while my assumptions regarding the root cause of the failure are incorrect (see below), this is still valid, and of general utility.

This is the error you posted:

CMake Error at cmake/modules/GetHostTriple.cmake:24 (message):

That code is only hit if MSVC is false. You're in the else block of an if(MSVC). The problem is actually that if(MSVC) isn't really the right check. Can you give this patch a try?

You're absolutely right. I didn't read that error message correctly, and misdiagnosed the entry of the if ( MSVC ) block. My apologies.

diff --git a/cmake/modules/GetHostTriple.cmake b/cmake/modules/GetHostTriple.cmake
index 5de710c..e3ecc5d 100644
--- a/cmake/modules/GetHostTriple.cmake
+++ b/cmake/modules/GetHostTriple.cmake
@@ -2,18 +2,18 @@
 # Invokes config.guess
 
 function( get_host_triple var )
-  if( MSVC )
-    if( CMAKE_CL_64 )
-      set( value "x86_64-pc-win32" )
-    else()
-      set( value "i686-pc-win32" )
-    endif()
-  elseif( MINGW AND NOT MSYS )
+  if( MINGW AND NOT MSYS )
     if( CMAKE_SIZEOF_VOID_P EQUAL 8 )
       set( value "x86_64-w64-mingw32" )
     else()
       set( value "i686-pc-mingw32" )
     endif()
+  elseif( CMAKE_HOST_WIN32 )
+    if( CMAKE_CL_64 )
+      set( value "x86_64-pc-win32" )
+    else()
+      set( value "i686-pc-win32" )
+    endif()
   else( MSVC )
     set(config_guess ${LLVM_MAIN_SRC_DIR}/cmake/config.guess)
     execute_process(COMMAND sh ${config_guess}

I think this is still going to set the host triple to an x86 win32 triple as the following conditions will always be true: CMAKE_HOST_WIN32, not using msys, and as you've shown, CMAKE_CL_64 won't be set, because MSVC is not set. I may be misunderstanding the root of the problem (my CMake shops are pretty woeful, admittedly), but this looks to me like it doesn't actually solve the case of specifying the triple manually. Having said that I'll definitely give this a shot when I've access to a win32 machine on Monday, and see if it does solve the specific lldb-server case I've been experiencing.

L

[beanz]

Supporting specifying the triple manually is a completely separate issue from the bug you encountered. I'd prefer to make target detection for windows work correctly and reliably so that we don't need to support setting LLVM_HOST_TRIPLE manually. Other than the bugs in get_host_triple, is there a reason you feel the need to set the host triple manually?

You are right about using CMAKE_CL_64, we should instead use CMAKE_SIZEOF_VOID_P. So something more like:

diff --git a/cmake/modules/GetHostTriple.cmake b/cmake/modules/GetHostTriple.cmake
index 5de710c..f8c7bf1 100644
--- a/cmake/modules/GetHostTriple.cmake
+++ b/cmake/modules/GetHostTriple.cmake
@@ -2,18 +2,18 @@
 # Invokes config.guess
 
 function( get_host_triple var )
-  if( MSVC )
-    if( CMAKE_CL_64 )
-      set( value "x86_64-pc-win32" )
-    else()
-      set( value "i686-pc-win32" )
-    endif()
-  elseif( MINGW AND NOT MSYS )
+  if( MINGW AND NOT MSYS )
     if( CMAKE_SIZEOF_VOID_P EQUAL 8 )
       set( value "x86_64-w64-mingw32" )
     else()
       set( value "i686-pc-mingw32" )
     endif()
+  elseif( CMAKE_HOST_WIN32 )
+    if( CMAKE_SIZEOF_VOID_P EQUAL 8 )
+      set( value "x86_64-pc-win32" )
+    else()
+      set( value "i686-pc-win32" )
+    endif()
   else( MSVC )
     set(config_guess ${LLVM_MAIN_SRC_DIR}/cmake/config.guess)
     execute_process(COMMAND sh ${config_guess}

-Chris

[tberghammer]

We are currently specifying LLVM_HOST_TRIPLE explicitly when we cross compile lldb-server from Linux to Android (all architecture) to match the triple of the host the compiled lldb-server will run on because we call llvm::sys::getProcessTriple() to determine the triple of the current system what is depending on LLVM_HOST_TRIPLE.

My understanding is that LLVM_HOST_TRIPLE should be the triple of the system you are planning to run LLVM on so it have to be specified explicitly (unless we figure it out based on the cross compiler specified)

[ldrumm]

In D22370#485676, @beanz wrote:

Supporting specifying the triple manually is a completely separate issue from the bug you encountered. I'd prefer to make target detection for windows work correctly and reliably so that we don't need to support setting LLVM_HOST_TRIPLE manually. Other than the bugs in get_host_triple, is there a reason you feel the need to set the host triple manually?

They are separate issues, I agree, but this kills two birds with one stone, as it fixes the lldb-server cross-compilation issue on win32 *and* enables people to specify the triple manually in the event it is required, while AFAICS not introducing any semantic changes unless the triple is manually specified.

You are right about using CMAKE_CL_64, we should instead use CMAKE_SIZEOF_VOID_P. So something more like:

diff --git a/cmake/modules/GetHostTriple.cmake b/cmake/modules/GetHostTriple.cmake
index 5de710c..f8c7bf1 100644
--- a/cmake/modules/GetHostTriple.cmake
+++ b/cmake/modules/GetHostTriple.cmake
@@ -2,18 +2,18 @@
 # Invokes config.guess
 
 function( get_host_triple var )
-  if( MSVC )
-    if( CMAKE_CL_64 )
-      set( value "x86_64-pc-win32" )
-    else()
-      set( value "i686-pc-win32" )
-    endif()
-  elseif( MINGW AND NOT MSYS )
+  if( MINGW AND NOT MSYS )
     if( CMAKE_SIZEOF_VOID_P EQUAL 8 )
       set( value "x86_64-w64-mingw32" )
     else()
       set( value "i686-pc-mingw32" )
     endif()
+  elseif( CMAKE_HOST_WIN32 )
+    if( CMAKE_SIZEOF_VOID_P EQUAL 8 )
+      set( value "x86_64-pc-win32" )
+    else()
+      set( value "i686-pc-win32" )
+    endif()
   else( MSVC )
     set(config_guess ${LLVM_MAIN_SRC_DIR}/cmake/config.guess)
     execute_process(COMMAND sh ${config_guess}

I'm afraid this won't work. As Tamas mentioned, we're not trying to detect windows; we're trying to detect the triple of the host the lldb-server is going to run on. The triple will need to be arm32-linux-unknown-android or similar. This does is the exact opposite of what we're trying to achieve, and unconditionally sets a win32 triple regardless of the cross compiler.

[beanz]

LLVM_HOST_TRIPLE is not intended to match the target, and it shouldn't be used in situations where it must match the target. I'm not familiar with how lldb builds the remote server in CMake, but using LLVM_HOST_TRIPLE is not the right answer.

In general when cross-compiling with CMake you'll want to specify all the target flags in a CMake toolchain file, and have the CMake configuration do the right thing.

[beanz]

From config-ix.cmake:

get_host_triple(LLVM_INFERRED_HOST_TRIPLE)

set(LLVM_HOST_TRIPLE "${LLVM_INFERRED_HOST_TRIPLE}" CACHE STRING
    "Host on which LLVM binaries will run")

If LLVM_HOST_TRIPLE is set manually on the command line the value read from get_host_triple will not overwrite it because the FORCE option isn't specified on the set.

Meaning, this change isn't actually needed to do what you're trying to do anyways. What is needed, is fixing the bugs in get_host_triple, so that it doesn't call config.guess on a Windows system.

[ldrumm]

In D22370#486859, @beanz wrote:

LLVM_HOST_TRIPLE is not intended to match the target, and it shouldn't be used in situations where it must match the target. I'm not familiar with how lldb builds the remote server in CMake, but using LLVM_HOST_TRIPLE is not the right answer.

From the documentation I can see, that's not the case. Indeed sys::getProcessTriple is documented here as follows:

Return an appropriate target triple for generating code to be loaded into the current process, e.g.
when using the JIT.

Its implementation is a simple wrapper around the LLVM_HOST_TRIPLE macro

std::string sys::getProcessTriple() {
  Triple PT(Triple::normalize(LLVM_HOST_TRIPLE));

  if (sizeof(void *) == 8 && PT.isArch32Bit())
    PT = PT.get64BitArchVariant();
  if (sizeof(void *) == 4 && PT.isArch64Bit())
    PT = PT.get32BitArchVariant();

  return PT.str();
}

getProcessTriple is then used in lldb and lldb-server to compute options at startup

Whether this meets the /intended/ use here is a philosophical question (albeit one that's answered here). Right now, we need this to match the triple to the cross-compiled binary, and forcing this to be win32 is incorrect in a cross-compilation situation. If this is a bug in the documentation for getProcessTriple, I'm happy to fix it, but this looks like lldb-server *is* using getProcessTriple in the intended way according to the commit message for it's initial implementation, as well as the Doxygen docs, so there shouldn't anything wrong with setting this.

[beanz]

I did not realize that LLVM_HOST_TRIPLE fed into getProcessTriple. In that case you are correct that you would need to manually override it. However your patch is still not the correct fix.

The problem you're encountering is not that manually specifying doesn't work, but rather that due to a bug in get_host_triple, your configuration is failing. Please see my other comment about config-ix.cmake.

[ldrumm]

Sorry Chris, I didn't see this one. It appeared while I was posting my last message

In D22370#486861, @beanz wrote:

From config-ix.cmake:

get_host_triple(LLVM_INFERRED_HOST_TRIPLE)

set(LLVM_HOST_TRIPLE "${LLVM_INFERRED_HOST_TRIPLE}" CACHE STRING
    "Host on which LLVM binaries will run")

If LLVM_HOST_TRIPLE is set manually on the command line the value read from get_host_triple will not overwrite it because the FORCE option isn't specified on the set.

Meaning, this change isn't actually needed to do what you're trying to do anyways. What is needed, is fixing the bugs in get_host_triple, so that it doesn't call config.guess on a Windows system.

So that's the root cause. Okay. That makes sense. I'm not running with a POSIX shell, so it's failing to run that script which is /bin/sh/. I'll take a look at that.

Thanks for helping me track down the root cause of this.

[beanz]

There is a patch that I put in an earlier comment which may do the trick for you. I'd start with that and see if it works.

[clayborg]

I don't know enough about cmake to be helpful in reviewing this patch.

cmake/modules/GetHostTriple.cmake

 # Returns the host triple.
 # Invokes config.guess
 
 function( get_host_triple var )
+  if( LLVM_HOST_TRIPLE )
+  set( value ${LLVM_HOST_TRIPLE} )
+  return()
+  endif()
   if( MSVC )
     if( CMAKE_CL_64 )
       set( value "x86_64-pc-win32" )
     else()
       set( value "i686-pc-win32" )
     endif()
   elseif( MINGW AND NOT MSYS )
     if( CMAKE_SIZEOF_VOID_P EQUAL 8 )