[PATCH] D19907: cmake: Prefix Polly options with LLVM_ to avoid variable shadowing
Michael Kruse via llvm-commits
llvm-commits at lists.llvm.org
Wed May 4 05:25:40 PDT 2016
Meinersbur added a comment.
This Differential does a bit more than renaming the cmake variables, which I think it should be reflected in the commit message/title.
Each of the LLVM projects usually has its own prefix: "CLANG_", "LLD_", ... . However, this is LLVM's CMakeLists.txt. I therefore suggest to use "LLVM_POLLY_LINK_INTO_TOOLS" and "LLVM_POLLY_BUILD" such that they are grouped together in ccmake/cmake-gui.
Instead of "LLVM_WITH_POLLY" I'd use the already existing "LLVM_TOOL_POLLY_BUILD", or maybe there is an intended difference?
When LLVM_EXTERNAL_*BUILD was renamed to LLVM_TOOL_*_BUILD, the old _BUILD property was still respected in order to allow to switch between older and newer revisions (eg. for bisect). We could do the same here, but personally I don't think it is necessary.
================
Comment at: CMakeLists.txt:349
@@ +348,3 @@
+
+if (EXISTS ${LLVM_MAIN_SRC_DIR}/tools/polly/CMakeLists.txt)
+ set(POLLY_IN_TREE TRUE)
----------------
It is documented that the Polly directory can be changed using LLVM_EXTERNAL_POLLY_SOURCE_DIR at
http://llvm.org/docs/CMake.html but I think it is currently impossible it is automatically deactivated by
```
if(WITH_POLLY)
add_llvm_external_project(polly)
else()
set(LLVM_TOOL_POLLY_BUILD Off)
endif()
```
We should either change the documentation or (my preference) actually allow a different Polly source directory.
http://reviews.llvm.org/D19907
More information about the llvm-commits
mailing list