cmake: Remove config.guess

[tstellar]

Summary

We are unable to update config.guess to a newer version since its
license is different than the license we use for llvm. Therefore, any
new host triple detection code needs to be done using CMake. Rather than have
some triple detection in CMake and some in config.guess, I think it
would be better to move all triple detection code into CMake.

This patch replaces config.guess with some logic that uses the host
compiler to determine the host's triple.

In the case where the CMake detection fails, the user can still manually
specify the host triple using -DLLVM_HOST_TRIPLE.

When using gcc or clang as the host compiler, this patch should make
host triple detection more accurate, since config.guess was returning
the wrong value on some systems.

[MaskRay]

llvm/CMakeLists.txt

479

Remove trailing . for the help message to be consistent with other options.

llvm/cmake/modules/GetHostTriple.cmake

70

Mention -DLLVM_HOST_TRIPLE=?

Seems that LLVM_HOST_TRIPLE is used to initialize LLVM_DEFAULT_TARGET_TRIPLE (llvm/CMakeLists.txt), so setting LLVM_HOST_TRIPLE is sufficient even in the absence of config.guess

[hvdijk]

Thanks for suggesting this approach, I was not looking forward to having a massive CMake recreation of config.guess, this is a nice way to avoid duplicating all the detection.

llvm/cmake/modules/GetHostTriple.cmake

57

CMake if commands are special, see the warning at https://cmake.org/cmake/help/latest/command/if.html#variable-expansion.

58

When CMAKE_CXX_COMPILER is set using the CXX environment variable, and CXX contains multiple words, the remaining words are provided in CMAKE_CXX_COMPILER_ARG1 and should also be included. (Yes, despite the name, all words other than the first are stored in a single variable.) CMAKE_CXX_FLAGS may have been set from the CXXFLAGS environment variable.

Including these options can be useful when setting CXX="clang++ -m32" or CXX=clang++ CXXFLAGS=-m32 or such.

That said, this only helps when the host compiler is clang, as gcc -v's printed target is always the default target and does not get adjusted based on any -m options.

[MaskRay]

% gcc -dumpmachine
x86_64-linux-gnu
% clang -dumpmachine
x86_64-unknown-linux-gnu

llvm/cmake/modules/GetHostTriple.cmake

58

% gcc -dumpmachine
x86_64-linux-gnu
% gcc -dumpmachine -m32
x86_64-linux-gnu
% clang -dumpmachine
x86_64-unknown-linux-gnu
% clang -dumpmachine -m32
i386-unknown-linux-gnu

is probably better. If the user wants to control x86_64-linux-gnu, x86_64-unknown-linux-gnu, x86_64-pc-linux-gnu, the should specify LLVM_HOST_TRIPLE explicitly.

[MaskRay]

I have thought again: we don't even need LLVM_USE_CONFIG_GUESS. The warning can just ask the user to run sh llvm/cmake/config.guess manually and re-run cmake with -DLLVM_HOST_TRIPLE=, with a caution that the config.guess value may not be correct.

[tstellar]

Use -dumpmachine instead of -v.
Pass CXX_FLAGS to compiler along with -dumpmachine.

[tstellar]

llvm/cmake/modules/GetHostTriple.cmake

70

Ok, I think this means I need to change this from a FATAL_ERROR to a WARNING, since get_host_triple gets called even if LLVM_HOST_TRIPLE is set.

[tstellar]

In D109837#3002372, @MaskRay wrote:

I have thought again: we don't even need LLVM_USE_CONFIG_GUESS. The warning can just ask the user to run sh llvm/cmake/config.guess manually and re-run cmake with -DLLVM_HOST_TRIPLE=, with a caution that the config.guess value may not be correct.

I like this approach, but what if we took it step further and asked them to download config.guess from https://raw.githubusercontent.com/gcc-mirror/gcc/master/config.guess so that we could remove it from the llvm tree entirely?

[MaskRay]

In D109837#3002538, @tstellar wrote:

In D109837#3002372, @MaskRay wrote:

I have thought again: we don't even need LLVM_USE_CONFIG_GUESS. The warning can just ask the user to run sh llvm/cmake/config.guess manually and re-run cmake with -DLLVM_HOST_TRIPLE=, with a caution that the config.guess value may not be correct.

I like this approach, but what if we took it step further and asked them to download config.guess from https://raw.githubusercontent.com/gcc-mirror/gcc/master/config.guess so that we could remove it from the llvm tree entirely?

Sounds good!

[phosek]

In D109837#3002540, @MaskRay wrote:

In D109837#3002538, @tstellar wrote:

In D109837#3002372, @MaskRay wrote:

I have thought again: we don't even need LLVM_USE_CONFIG_GUESS. The warning can just ask the user to run sh llvm/cmake/config.guess manually and re-run cmake with -DLLVM_HOST_TRIPLE=, with a caution that the config.guess value may not be correct.

I like this approach, but what if we took it step further and asked them to download config.guess from https://raw.githubusercontent.com/gcc-mirror/gcc/master/config.guess so that we could remove it from the llvm tree entirely?

Sounds good!

We could make LLVM_USE_CONFIG_GUESS a string flag rather than a boolean option where the value would be the path to config.guess.

[hvdijk]

In D109837#3002538, @tstellar wrote:

I like this approach, but what if we took it step further and asked them to download config.guess from https://raw.githubusercontent.com/gcc-mirror/gcc/master/config.guess so that we could remove it from the llvm tree entirely?

The idea sounds good to me, but the upstream link is https://git.savannah.gnu.org/cgit/config.git/plain/config.guess. GCC updates its copy when needed for GCC, not whenever a change is made, so it can get outdated.

[tstellar]

Completely remove config.guess and add an error message with instructions
for how to download the latest config.guess and use it to generate the
default target.

[echristo]

I think I'd rather not tell people to download config.guess. We can still modify the one we have, have there been serious problems doing so?

[tstellar]

In D109837#3005394, @echristo wrote:

I think I'd rather not tell people to download config.guess. We can still modify the one we have, have there been serious problems doing so?

We could modify the one we have, but personally, I would rather see people put the fixes into CMake rather than trying to keep using this very old version of config.guess.

Do you think we should keep it as a fallback or would you be OK with removing it if we also removed the part of the error message telling people to download it?

[MaskRay]

GettingStarted mentions the supported compilers are:

Clang 3.5
Apple Clang 6.0
GCC 5.1
Visual Studio 2017

Note that the patch doesn't touch the MSVC code path. -dumpmachine works for GCC, Clang and Intel C++ Compiler, so really the downloading config.guess message will only apply to IBM XL C/C++.
Even for that, looking at powerpc64-ibm-aix above, the XL C/C++ code paths are covered.

So perhaps we can just replace elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU") with unconditional -dumpmachine call and ask the user to specify LLVM_HOST_TRIPLE explicitly if -dumpmachine is somehow not available.
User not covered by -dumpmachine are expected to be able to come up the triplet by themselves.

If mentioning config.guess URL is a problem, just omit it. I don't think there will be a large loss.

---

If we want to be conservative, mention that llvm/cmake/config.guess exists; as a follow-up, delete config.guess.

Otherwise, just delete config.guess in this patch.

[tstellar]

• Remove suggestion to download config.guess
• Failur to detect triple is a warning not instead of an error.

[MaskRay]

LGTM.

[tstellar]

Make failure to determine the host via -dumpmachine a fatal error.
Empty string is not a valid value for LLVM_HOST_TRIPLE.

[tstellar]

Does anyone else have comments for this before I commit?

[echristo]

This systems seems somewhat more fragile than config.guess? I guess I'm just not convinced that doing this versus the straightforward changes to config.guess are worth our effort? Can we get an idea of what changes you don't want to make to the local copy of config.guess?

Thanks!

[tstellar]

In D109837#3016845, @echristo wrote:

This systems seems somewhat more fragile than config.guess? I guess I'm just not convinced that doing this versus the straightforward changes to config.guess are worth our effort? Can we get an idea of what changes you don't want to make to the local copy of config.guess?

Thanks!

The main feature that is missing from config.guess is the vendor detection (e.g. RedHat, OpenSuse, Ubuntu etc.), so the changes needed in config.guess to support this would be similar to what we have in clang now: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Distro.cpp#L97 There may be other features missing too, but this is just the one I'm interested in.

I also think that in general it would be nice to remove config.guess, because its license has caused some confusion with users and contributors.

[MaskRay]

Perhaps the description can mention the cost we'd incur if we took the "fixing config.guess" approach:

linux-musl, suse-linux, redhat-linux, and every Linux distro which customizes the 'vendor' part need to be fixed. The approach would involve poking around /etc/os-release and perhaps other random files.
As concrete examples: these are all incorrect x86_64-alpine-linux-musl, aarch64-suse-linux-gnu, powerpc64le-redhat-linux-gnu, i586-pc-haiku

The diff with the upstream version has 2000 lines FWIW :)

[echristo]

In D109837#3016868, @tstellar wrote:

In D109837#3016845, @echristo wrote:

This systems seems somewhat more fragile than config.guess? I guess I'm just not convinced that doing this versus the straightforward changes to config.guess are worth our effort? Can we get an idea of what changes you don't want to make to the local copy of config.guess?

Thanks!

The main feature that is missing from config.guess is the vendor detection (e.g. RedHat, OpenSuse, Ubuntu etc.), so the changes needed in config.guess to support this would be similar to what we have in clang now: https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/Distro.cpp#L97 There may be other features missing too, but this is just the one I'm interested in.

I also think that in general it would be nice to remove config.guess, because its license has caused some confusion with users and contributors.

That seems reasonable (and Fangrui has given a lot of context behind the scenes as well). It was mostly the long term maintainability of this versus other methods I was concerned about. icc and gxlc have compatibility modes here, though we might want to take a look and make sure that it works in general.

Otherwise, LGTM and thanks :)

-eric

llvm/cmake/modules/GetHostTriple.cmake

54

For debugging purposes it might be good to give the entire command line that you're trying versus just that the compiler failed?

[tstellar]

Put full -dumpmachine command in the error message.

[echristo]

Thanks! No need to wait on future approval with that change :)

-eric

[tstellar]

I've been doing a lot of testing with this patch, and one problem I've run into is that config.guess would include -gnu in the environment component of the triple, where gcc -dumpmachine does not (at least for the redhat triples e.g. x86_64-redhat-linux). There are a few places in gcc and clang that behave differently when the gnu environment is part of the triple.

I wonder if we should normalize some of the gcc style triples to include the gnu environment by default?

[tstellar]

In D109837#3034457, @tstellar wrote:

I've been doing a lot of testing with this patch, and one problem I've run into is that config.guess would include -gnu in the environment component of the triple, where gcc -dumpmachine does not (at least for the redhat triples e.g. x86_64-redhat-linux). There are a few places in gcc and clang that behave differently when the gnu environment is part of the triple.

I wonder if we should normalize some of the gcc style triples to include the gnu environment by default?

I tested this and it doesn't work. If we normalize x86_64-redhat-linux to x86_64-redhat-linux-gnu, then we don't end up picking the gcc install at /usr/lib/gcc/x86_64-redhat-linux/.

[aaronpuchert]

In D109837#3034457, @tstellar wrote:

I've been doing a lot of testing with this patch, and one problem I've run into is that config.guess would include -gnu in the environment component of the triple, where gcc -dumpmachine does not (at least for the redhat triples e.g. x86_64-redhat-linux). There are a few places in gcc and clang that behave differently when the gnu environment is part of the triple.

Same problem for SUSE. Maybe we could let Triple::Triple(const Twine &) "guess" the environment as it does for some MIPS triples? So it would take x86_64-redhat-linux but then based on the OS vendor would set Environment = Triple::GNU; at least in the case of RedHat and SUSE, and maybe some other OS vendors.

[tstellar]

In D109837#3038321, @aaronpuchert wrote:

In D109837#3034457, @tstellar wrote:

I've been doing a lot of testing with this patch, and one problem I've run into is that config.guess would include -gnu in the environment component of the triple, where gcc -dumpmachine does not (at least for the redhat triples e.g. x86_64-redhat-linux). There are a few places in gcc and clang that behave differently when the gnu environment is part of the triple.

Same problem for SUSE. Maybe we could let Triple::Triple(const Twine &) "guess" the environment as it does for some MIPS triples? So it would take x86_64-redhat-linux but then based on the OS vendor would set Environment = Triple::GNU; at least in the case of RedHat and SUSE, and maybe some other OS vendors.

Here is my proposed solution for this: D110900

[hvdijk]

In D109837#3035161, @tstellar wrote:

I tested this and it doesn't work. If we normalize x86_64-redhat-linux to x86_64-redhat-linux-gnu, then we don't end up picking the gcc install at /usr/lib/gcc/x86_64-redhat-linux/.

We actually already don't. Red Hat have a RH-specific patch for this that lets it work, it's not related to the triple handling, it's that RH prefer GCC installations that contain libgcc_s, which RH's cross-compilers don't: https://src.fedoraproject.org/rpms/clang/blob/rawhide/f/0004-PATCH-clang-Prefer-gcc-toolchains-with-libgcc_s.so-w.patch And this is suppressed under -static -static-libgcc, so to confirm that this is what causes the GCC installation to be picked that you want: after installing gcc-x86_64-linux-gnu and clang on a Fedora system, clang -v prints "Selected GCC installation: /usr/bin/../lib/gcc/x86_64-redhat-linux/11", clang -static -static-libgcc -v prints "Selected GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/11".

So, this patch, if we modify it to normalize x86_64-redhat-linux to x86_64-redhat-linux-gnu, does not make things any worse. I am not trying to dismiss your other patches, they should still be considered, just probably not as blockers for this one.

llvm/CMakeLists.txt

option(LLVM_USE_OPROFILE
   "Use opagent JIT interface to inform OProfile about JIT code" OFF)
 
 option(LLVM_EXTERNALIZE_DEBUGINFO
   "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
 
 set(LLVM_CODESIGNING_IDENTITY "" CACHE STRING
   "Sign executables and dylibs with the given identity or skip if empty (Darwin Only)")
 
+option(LLVM_USE_CONFIG_GUESS
+   "(deprecated) Use config.guess to determine host architecture.  This option will be removed in the future." OFF)
+
 # If enabled, verify we are on a platform that supports oprofile.
 if( LLVM_USE_OPROFILE )
   if( NOT CMAKE_SYSTEM_NAME MATCHES "Linux" )
     message(FATAL_ERROR "OProfile support is available on Linux only.")
   endif( NOT CMAKE_SYSTEM_NAME MATCHES "Linux" )
 endif( LLVM_USE_OPROFILE )
 
 option(LLVM_USE_PERF

llvm/cmake/modules/GetHostTriple.cmake

elseif( CMAKE_SYSTEM_NAME STREQUAL AIX )
     if( CMAKE_SIZEOF_VOID_P EQUAL 8 )
       set( value "powerpc64-ibm-aix" )
     else()
       set( value "powerpc-ibm-aix" )
     endif()
   else( MSVC )
     if(CMAKE_HOST_SYSTEM_NAME STREQUAL Windows AND NOT MSYS)
       message(WARNING "unable to determine host target triple")
-    else()
+    elseif(LLVM_USE_CONFIG_GUESS)
       set(config_guess ${LLVM_MAIN_SRC_DIR}/cmake/config.guess)
       execute_process(COMMAND sh ${config_guess}
         RESULT_VARIABLE TT_RV
         OUTPUT_VARIABLE TT_OUT
         OUTPUT_STRIP_TRAILING_WHITESPACE)
       if( NOT TT_RV EQUAL 0 )
         message(FATAL_ERROR "Failed to execute ${config_guess}")
       endif( NOT TT_RV EQUAL 0 )
       set( value ${TT_OUT} )
+    elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "Clang" OR ${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU")
+      execute_process(COMMAND ${CMAKE_CXX_COMPILER} -v
+        RESULT_VARIABLE TT_RV
+        ERROR_VARIABLE TT_OUT
+        ERROR_STRIP_TRAILING_WHITESPACE)
+      if( NOT TT_RV EQUAL 0 )
+        message(FATAL_ERROR "Failed to execute ${CMAKE_CXX_COMPILER} for triple detection")
+      endif( NOT TT_RV EQUAL 0 )
+      string(REGEX MATCH "Target: ([^\r\n]+)[\r\n]" value "${TT_OUT}")
+      if (NOT "${CMAKE_MATCH_1}" STREQUAL "")
+        set(value ${CMAKE_MATCH_1})
+      else()
+        message(FATAL_ERROR "Unable to determine host triple.  Please file a bug.  "
+                             "You may work around this for now configuring with "
+                             "-DLLVM_USE_CONFIG_GUESS=ON, however, this option "
+                             "will be removed in the future")
+      endif()
     endif()
   endif( MSVC )
   set( ${var} ${value} PARENT_SCOPE )
 endfunction( get_host_triple var )