[all-commits] [llvm/llvm-project] 1e03c3: Prepare Compiler-RT for GnuInstallDirs, matching l...

John Ericson via All-commits all-commits at lists.llvm.org
Tue Jul 13 08:21:55 PDT 2021


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 1e03c37b97b6176a60404d84665c40321f4e33a4
      https://github.com/llvm/llvm-project/commit/1e03c37b97b6176a60404d84665c40321f4e33a4
  Author: John Ericson <John.Ericson at Obsidian.Systems>
  Date:   2021-07-13 (Tue, 13 Jul 2021)

  Changed paths:
    M clang/runtime/CMakeLists.txt
    M compiler-rt/cmake/Modules/AddCompilerRT.cmake
    M compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
    M compiler-rt/cmake/Modules/CompilerRTUtils.cmake
    M compiler-rt/cmake/base-config-ix.cmake
    A compiler-rt/docs/BuildingCompilerRT.rst
    M compiler-rt/include/CMakeLists.txt
    M compiler-rt/lib/dfsan/CMakeLists.txt
    M libcxx/CMakeLists.txt
    M libcxx/docs/BuildingLibcxx.rst
    M libcxxabi/CMakeLists.txt
    M libunwind/CMakeLists.txt
    M libunwind/docs/BuildingLibunwind.rst

  Log Message:
  -----------
  Prepare Compiler-RT for GnuInstallDirs, matching libcxx, document all

This is a second attempt at D101497, which landed as
9a9bc76c0eb72f0f2732c729a460abbd5239c2e3 but had to be reverted in
8cf7ddbdd4e5af966a369e170c73250f2e3920e7.

This issue was that in the case that `COMPILER_RT_INSTALL_PATH` is
empty, expressions like "${COMPILER_RT_INSTALL_PATH}/bin" evaluated to
"/bin" not "bin" as intended and as was originally.

One solution is to make `COMPILER_RT_INSTALL_PATH` always non-empty,
defaulting it to `CMAKE_INSTALL_PREFIX`. D99636 adopted that approach.
But, I think it is more ergonomic to allow those project-specific paths
to be relative the global ones. Also, making install paths absolute by
default inhibits the proper behavior of functions like
`GNUInstallDirs_get_absolute_install_dir` which make relative install
paths absolute in a more complicated way.

Given all this, I will define a function like the one asked for in
https://gitlab.kitware.com/cmake/cmake/-/issues/19568 (and needed for a
similar use-case).

---

Original message:

Instead of using `COMPILER_RT_INSTALL_PATH` through the CMake for
complier-rt, just use it to define variables for the subdirs which
themselves are used.

This preserves compatibility, but later on we might consider getting rid
of `COMPILER_RT_INSTALL_PATH` and just changing the defaults for the
subdir variables directly.

---

There was a seaming bug where the (non-Apple) per-target libdir was
`${target}` not `lib/${target}`. I suspect that has to do with the docs
on `COMPILER_RT_INSTALL_PATH` saying was the library dir when that's no
longer true, so I just went ahead and fixed it, allowing me to define
fewer and more sensible variables.

That last part should be the only behavior changes; everything else
should be a pure refactoring.

---

I added some documentation of these variables too. In particular, I
wanted to highlight the gotcha where `-DSomeCachePath=...` without the
`:PATH` will lead CMake to make the path absolute. See [1] for
discussion of the problem, and [2] for the brief official documentation
they added as a result.

[1]: https://cmake.org/pipermail/cmake/2015-March/060204.html

[2]: https://cmake.org/cmake/help/latest/manual/cmake.1.html#options

In 38b2dec37ee735d5409148e71ecba278caf0f969 the problem was somewhat
misidentified and so `:STRING` was used, but `:PATH` is better as it
sets the correct type from the get-go.

---

D99484 is the main thrust of the `GnuInstallDirs` work. Once this lands,
it should be feasible to follow both of these up with a simple patch for
compiler-rt analogous to the one for libcxx.

Reviewed By: phosek, #libc_abi, #libunwind

Differential Revision: https://reviews.llvm.org/D105765




More information about the All-commits mailing list