[PATCH] D62640: [CMake] Use find_package(LLVM) instead of LLVMConfig

[Petr Hosek via Phabricator via llvm-commits]

phosek added inline comments.


================
Comment at: llvm/runtimes/CMakeLists.txt:62
 
+  find_package(LLVM)
+
----------------
smeenai wrote:
> phosek wrote:
> > smeenai wrote:
> > > `CMAKE_PREFIX_PATH` won't work correctly when `CMAKE_FIND_ROOT_PATH` is used and `CMAKE_FIND_ROOT_PATH_MODE_PACKAGE` is set to `ONLY`, which is a fairly common configuration for cross-compilation. Setting `LLVM_DIR` explicitly to `${LLVM_BINARY_DIR}/lib/cmake/llvm` should always work. You could combine that with `NO_DEFAULT_PATH` to ensure you only search in the specified directory, unless you want to be able to find some other LLVM package that's installed on the system?
> > I've used `find_package(LLVM PATHS ...)`, is that what you had in mind?
> I believe `PATHS` is also affected by `CMAKE_FIND_ROOT_PATH`, though you could explicitly add `NO_CMAKE_FIND_ROOT_PATH` to avoid that. What I was thinking of was setting `LLVM_DIR` instead of `CMAKE_PREFIX_PATH` in your previous patch which was doing the explicit `-DCMAKE_PREFIX_PATH=...` (the `<package>_DIR` variable is used by CMake automatically to search for `<package>`), but I think `PATHS` + `NO_CMAKE_FIND_ROOT_PATH` should be equivalent and keeps the path in one place.
> 
> Does just using `${LLVM_BINARY_DIR}` as the `PATHS` argument do the right thing, i.e. search inside `lib/cmake/llvm` itself? If not, you could use `${LLVM_LIBRARY_DIR}` as part of the path specification here.
I tested it and just `${LLVM_BINARY_DIR}` seems to be sufficient so that's what I went with.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62640