[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