[PATCH] D33299: [Polly][CMake] Use the CMake Package instead of llvm-config in out-of-tree builds
Philip Pfaffe via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun May 21 13:15:27 PDT 2017
philip.pfaffe marked an inline comment as done.
philip.pfaffe added inline comments.
================
Comment at: CMakeLists.txt:24
+ if (LLVM_BUILD_MAIN_SRC_DIR)
+ set(LLVM_SOURCE_ROOT ${MAIN_SRC_DIR} CACHE PATH "Path to LLVM source tree")
+ else()
----------------
Meinersbur wrote:
> This is the same line in both cases?
The former is a typo and should use LLVM_BUILD_MAIN_SRC_DIR.
================
Comment at: CMakeLists.txt:26
+ else()
+ execute_process(COMMAND "${LLVM_INSTALL_ROOT}/bin/llvm-config" --src-root
+ OUTPUT_VARIABLE MAIN_SRC_DIR
----------------
Meinersbur wrote:
> Still using `llvm-config`? If there is no `LLVM_BUILD_MAIN_SRC_DIR`, shouldn't the llvm-config also fail?
At this point, llvm-config is purely used to provide a default for the setting. I expect the result to be useful when you're building against the install tree of a local build. It won't be helpful if you're using, e.g., a binary distribution for instance.
================
Comment at: CMakeLists.txt:35
if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h)
- add_library(gtest
- ${UNITTEST_DIR}/googletest/src/gtest-all.cc
- ${UNITTEST_DIR}/googlemock/src/gmock-all.cc
- )
- target_include_directories(gtest
- PUBLIC
- "${UNITTEST_DIR}/googletest/include"
- "${UNITTEST_DIR}/googlemock/include"
-
- PRIVATE
- "${UNITTEST_DIR}/googletest"
- "${UNITTEST_DIR}/googlemock"
- )
- target_link_libraries(gtest -lpthread)
-
- add_library(gtest_main ${UNITTEST_DIR}/UnitTestMain/TestMain.cpp)
- target_link_libraries(gtest_main gtest)
-
- set(POLLY_GTEST_AVAIL 1)
+ if (TARGET gtest)
+ set_target_properties(gtest
----------------
Meinersbur wrote:
> Under which conditions does LLVM not export a gtest target?
The install tree doesn't contain gtest.
================
Comment at: lib/CMakeLists.txt:109
+ LLVMPasses
+ ${nvptx_libs}
+ # The libraries below are required for darwin: http://PR26392
----------------
Meinersbur wrote:
> Has this order change a reason?
>
> You seem to have added `LLVMPasses` to your local changes.
Yes, this one slipped through.
https://reviews.llvm.org/D33299
More information about the llvm-commits
mailing list