[PATCH] D99621: [CMake][Compiler-rt] Make it possible to configure standalone compiler-rt without `LLVMConfig.cmake`.

[Petr Hosek via Phabricator via llvm-commits]

phosek added a comment.

In D99621#2659869 <https://reviews.llvm.org/D99621#2659869>, @delcypher wrote:

> In D99621#2659844 <https://reviews.llvm.org/D99621#2659844>, @phosek wrote:
>
>> Do you know what we need `LLVMConfig.cmake` for? Could we remove that dependency altogether? It's not used by any other runtime as far as I'm aware.
>
> I wouldn't want to remove the dependency at this stage as not enough of this new code has been tested (I only tested on macOS). However, I think this patch could used as a stepping stone towards completely removing the dependency on `LLVMConfig.cmake`. If we landed this and did the necessary work to make this new code path work on all the host platforms we care about then I think we could remove the dependency on `LLVMConfig.cmake`. I'd like to see that happen because the requirement makes testing harder and seems unnecessary.

SGTM

> The actions that the new `compiler_rt_mock_llvm_cmake_config` macro take do what I found to be necessary to get the CMake configure to work without having `LLVMConfig.cmake` present with on a macOS host. Basically this seems to be
>
> - Setting `TARGET_TRIPLE`. The build system fails in weird and wonderful ways when this is an empty string. **SHAKES FIST AT CMAKE**.

I see only two dependencies on this variable in https://github.com/llvm/llvm-project/blob/a360a9786f5f82f4beff6fdcec12b40ee392db7a/compiler-rt/cmake/Modules/CompilerRTUtils.cmake#L317 and https://github.com/llvm/llvm-project/blob/a360a9786f5f82f4beff6fdcec12b40ee392db7a/compiler-rt/cmake/Modules/AddCompilerRT.cmake#L131.

I could see how the build starts misbehaving if that variable isn't set, but it should be also feasible to address that issue given that it's only two references.

> - Including `AddLLVM.cmake` from the LLVM source tree. It seems we have some CMake code that depends on code in this file.
> - Add LLVM's CMake module directory into the CMake module search path.  This necessary for the include of `AddLLVM.cmake` to work.

This should become a non-issue if we assume the monorepo layout.

> It may turn out we need to do more than this to support other platforms.



================
Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:320
+      include("${LLVM_CMAKE_PATH}/LLVMConfig.cmake")
+    else()
+      message(WARNING "LLVM CMake path (${LLVM_CMAKE_PATH}) reported by llvm-config does not exist")
----------------
Would it be possible to leave a comment to clarify that this configuration is likely insufficient for a full build and is only here to allow testing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99621