[PATCH] D41660: [cmake] Add new linux toolchain file

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In https://reviews.llvm.org/D41660#965877, @beanz wrote:

> You should split the CMake cache file you created into two files, (1) a CMake Cache to manage the build configuration and (2) a tool chain file for targeting Linux. As @semeenai pointed out we absolutly want the behavior of the toolchain file being loaded multiple times. That is the correct way this build should work.


I really like keeping this in a single file, but will break it up if necessary.

The `if` part of the if/else is used in stage1 as a cache file, and the `else` part used in stage2 (and as you said, is loaded many times).  Splitting this into two files won't make much difference in that regard.

> For bootstrap builds where you want the stage1 to run on your build host, you should be able to set `BOOTSTRAP_CMAKE_TOOLCHAIN_FILE` in the first stage build, to signal to the first stage build that the second stage will be cross-compiled, and we can customize the multi-stage dependencies correctly based on that. That avoids the need for the `ADDITIONAL_CLANG_BOOTSTRAP_DEPS` variable, which feels a bit hacky to me.

Unless there's another way to do it, It's not hacky.  I believe the term is `escape hatch`.

While I'm happy to use `BOOTSTRAP_CMAKE_TOOLCHAIN_FILE` instead of passing `-DCMAKE_TOOLCHAIN_FILE=${CMAKE_CURRENT_LIST_FILE}`, I do not see how it helps with this problem.  When running `ninja stage2`, I need to insure that the dependancies where built.  `BOOTSTRAP_LLVM_ENABLE_LLD` can be used to add `lld` to the dependency list, but since I'm setting `CLANG_DEFAULT_LINKER=llb`, I don't want clang adding `-fuse-ld`.

If I run this on Linux for both stages, it doesn't matter, because clang/CMakeLists.txt add llvm-ar and llvm-ranlib automatically, but since I'm on APPLE (see clang/CMakeLists.txt:559), they don't get added.

So, how else would I add them?



================
Comment at: cmake/caches/linux-toolchain.cmake:2
+# This file is intended to support cross compiling a linux toolchain
+# on any host system, includind Darwin.
+#
----------------
smeenai wrote:
> Cross-compilation terminology is kinda weird, and traditionally, the "host" is actually the system the built binaries will be run on (Linux in this case), whereas the build machine is the "build" (but of course that word is super ambiguous). I think LLVM generally sticks to that terminology though, e.g. `LLVM_HOST_TRIPLE`.
I'll work on cleaning up this comment, but the idea is that we cross compile on any host system, e.g., Linux, Darwin, Windows, etc., and target Linux.


================
Comment at: cmake/caches/linux-toolchain.cmake:84
+  else()
+    set(BOOTSTRAP_STAGE2_PROJECTS "clang;libcxx;libcxxabi;libunwind" CACHE STRING "" FORCE)
+  endif()
----------------
smeenai wrote:
> Nit: write this out as a list instead of a string with semicolons? (I know they're equivalent, but the list reads nicer IMO.)
Perhaps, but this is the style used throughout the clang+llvm cmake files.


================
Comment at: cmake/caches/linux-toolchain.cmake:88
+  # Required on non-elf hosts.
+  set(ADDITIONAL_CLANG_BOOTSTRAP_DEPS "lld;llvm-ar;llvm-ranlib" CACHE STRING "")
+
----------------
smeenai wrote:
> Not exactly related, but I wonder why the LLVM build needs ranlib (rather than just invoking ar appropriately).
Darwin version of ranlib doesn't like elf binaries, so we need the one we build in stage1.


================
Comment at: cmake/caches/linux-toolchain.cmake:102
+else()
+  set(CMAKE_SYSTEM_NAME Linux CACHE STRING "" FORCE)
+
----------------
smeenai wrote:
> The CMake documentation for CMAKE_SYSTEM_NAME says CMAKE_SYSTEM_VERSION should also be set when cross-compiling (though I haven't seen any ill effects from not doing so). Setting CMAKE_SYSTEM_PROCESSOR probably doesn't hurt either.
These can be passed to stage1 as BOOTSTRAP_CMAKE_SYSTEM_VERSION, etc., allowing the user full control.  I'll add a note to the comments up top.


Repository:
  rC Clang

https://reviews.llvm.org/D41660