[PATCH] D137024: [compiler-rt] Switch from llvm-config to find_package(LLVM)

Dan Liew via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 16 11:30:48 PST 2022


delcypher added a comment.

In D137024#3931167 <https://reviews.llvm.org/D137024#3931167>, @mgorny wrote:

> In D137024#3931126 <https://reviews.llvm.org/D137024#3931126>, @thetruestblue wrote:
>
>> This patch removes the logic that sets the binary tools dir using llvm-config. We don't have llvmConfig.cmake in our toolchain or our build tree and relied on llvm-config to 
>> set(LLVM_TOOLS_BINARY_DIR "${LLVM_TOOLS_BINARY_DIR}"directory.
>
> While I sympathize with you, I don't think this is valid reason to maintain full compatibility with `llvm-config`. Unless i'm mistaken, its use for building other LLVM projects standalone was deprecated for a few years already, and compiler-rt was the last project to carry the compatibility code. Furthermore, skipping cmake files from LLVM seems wrong.

@mgorny To add a bit of context to this discussion. We are not asking for compatibility with the `llvm-config` binary. I'm fine if that goes way, which it already has.

What we want to do is make sure is that our use case still works. I know that "skipping cmake files from LLVM seems wrong" but there is a very good reason for this. We have a testing configuration where we consume pre-built toolchains and run the compiler-rt tests against these. We use the standalone CMake configure of compiler-rt to generate the test suites, nothing actually gets built.
In this scenario the toolchain does not include LLVM CMake files because we do not ship them to customers. Making it a hard requirement to have those CMake files would prevent us from doing that kind of testing.

I suggest that @thetruestblue and I draft a patch so our intentions our clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137024



More information about the cfe-commits mailing list