Page MenuHomePhabricator

CMake: Replace open-coded find_package
ClosedPublic

Authored by tstellar on Apr 26 2017, 7:38 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellar created this revision.Apr 26 2017, 7:38 PM

I suggest this may be an options when LLVM_CONFIG is not specified.

CMakeLists.txt
36 ↗(On Diff #96859)

Could we move find_package(LLVM) into here?

I have investigated what LLVMConfig.cmake actually provides.

With the installed tree, SRC_ROOT (LLVM_BUILD_MAIN_SRC_DIR) is not provided.
It shouldn't be included in installed LLVMConfig.cmake. Just "llvm-config --src-root" knows. And LLVM_MAIN_SRC_DIR should be CACHE, optional to be specified manually.

I suggest;

  1. Invoke "llvm-config --cmakedir --src-root" if LLVM_CONFIG is specified. (may be removed in future) Do not override LLVM_MAIN_SRC_DIR if it is defined.
  2. find_package(LLVM REQUIRED HINTS ${LLVM_CMAKE_PATH}). Without LLVM_CONFIG, CMake will seek LLVMConfig.cmake along CMake's search paths like -DCMAKE_PREFIX_PATH.
  3. If LLVM_MAIN_SRC_DIR is unknown, invoke "llvm-config --src-root"

LLVM_MAIN_SRC_DIR is just required to build test tools in llvm/utils.

I suggest this may be an options when LLVM_CONFIG is not specified.

This is my eventual goal, but for now this is just a NFC clean up to make future changes more simple.

tstellar updated this revision to Diff 141793.Apr 9 2018, 11:15 PM

Rebased this patch on ToT.

tstellar updated this revision to Diff 164141.Sep 5 2018, 8:09 PM

Rebase on trunk.

mgorny accepted this revision.Nov 10 2018, 1:32 AM

LGTM

This revision is now accepted and ready to land.Nov 10 2018, 1:32 AM
This revision was automatically updated to reflect the committed changes.