[PATCH] D109837: cmake: Remove config.guess

[Eric Christopher via Phabricator via llvm-commits]

echristo accepted this revision.
echristo added a comment.

In D109837#3016868 <https://reviews.llvm.org/D109837#3016868>, @tstellar wrote:

> In D109837#3016845 <https://reviews.llvm.org/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



================
Comment at: llvm/cmake/modules/GetHostTriple.cmake:53
       if( NOT TT_RV EQUAL 0 )
-        message(FATAL_ERROR "Failed to execute ${config_guess}")
+        message(FATAL_ERROR "Failed to execute ${CMAKE_CXX_COMPILER} for triple detection."
+                        " Use -DLLVM_HOST_TRIPLE= if you want to define the host triple.")
----------------
For debugging purposes it might be good to give the entire command line that you're trying versus just that the compiler failed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109837/new/

https://reviews.llvm.org/D109837