cmake: Prefix Polly options with LLVM_ to avoid variable shadowing

[grosser]

Summary

Before this change certain Polly variables have been used both as a user-facing
CACHED cmake variables as well as a uncached internal variables. Even though
this seems to have worked OK in practice, the behavior only worked due to
one variable shadowing the other. This behavior has been found confusing.
To make the use of cmake variables more clear we now prefix the cached, user
facing variables with LLVM_ as it is common habit for LLVM options. AS a result,
Polly is now enabled with LLVM_BUILD_POLLY instead of BUILD_POLLY and the
linking behavior of Polly is controlled with LLVM_LINK_POLLY_INTO_TOOLS instead
of LINK_POLLY_INTO_TOOLS.

[Meinersbur]

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.

CMakeLists.txt

349 ↗

(On Diff #56106)

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.

[grosser]

In D19907#420937, @Meinersbur wrote:

This Differential does a bit more than renaming the cmake variables, which I think it should be reflected in the commit message/title.

As discussed in our phone call, I added another sentence to the commit message to make this clear.

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.

I used the names you suggested.

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.

This clearly is worth improving, but as this is more involved, it is probably better to leave this for another patch.

llvm/trunk/CMakeLists.txt

 endif( LLVM_USE_OPROFILE )
 
 set(LLVM_USE_SANITIZER "" CACHE STRING
   "Define the sanitizer used to build binaries and tests.")
 
 option(LLVM_USE_SPLIT_DWARF
   "Use -gsplit-dwarf when compiling llvm." OFF)
 
-option(WITH_POLLY "Build LLVM with Polly" ON)
-option(LINK_POLLY_INTO_TOOLS "Static link Polly into tools" ON)
+option(LLVM_POLLY_LINK_INTO_TOOLS "Statically link Polly into tools (if available)" ON)
+option(LLVM_POLLY_BUILD "Build LLVM with Polly" ON)
+
+if (EXISTS ${LLVM_MAIN_SRC_DIR}/tools/polly/CMakeLists.txt)
+  set(POLLY_IN_TREE TRUE)
+else()
+  set(POLLY_IN_TREE FALSE)
+endif()
+
+if (LLVM_POLLY_BUILD AND POLLY_IN_TREE)
+  set(WITH_POLLY ON)
+else()
+  set(WITH_POLLY OFF)
+endif()
+
+if (LLVM_POLLY_LINK_INTO_TOOLS AND WITH_POLLY)
+  set(LINK_POLLY_INTO_TOOLS ON)
+else()
+  set(LINK_POLLY_INTO_TOOLS OFF)
+endif()
 
 # Define an option controlling whether we should build for 32-bit on 64-bit
 # platforms, where supported.
 if( CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32 )
   # TODO: support other platforms and toolchains.
   option(LLVM_BUILD_32_BITS "Build 32 bits executables and libraries." OFF)
 endif()
 

 option (LLVM_BUILD_DOCS "Build the llvm documentation." OFF)
 option (LLVM_INCLUDE_DOCS "Generate build targets for llvm documentation." ON)
 option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm API documentation." OFF)
 option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
 
 option (LLVM_BUILD_EXTERNAL_COMPILER_RT
   "Build compiler-rt as an external project." OFF)
 
-if(WITH_POLLY)
-  if(NOT EXISTS ${LLVM_MAIN_SRC_DIR}/tools/polly/CMakeLists.txt)
-    set(WITH_POLLY OFF)
-  endif()
-endif(WITH_POLLY)
-
-if (NOT WITH_POLLY)
- set(LINK_POLLY_INTO_TOOLS OFF)
-endif (NOT WITH_POLLY)
-
 # You can configure which libraries from LLVM you want to include in the
 # shared library by setting LLVM_DYLIB_COMPONENTS to a semi-colon delimited
 # list of LLVM components. All component names handled by llvm-config are valid.
 if(NOT DEFINED LLVM_DYLIB_COMPONENTS)
   set(LLVM_DYLIB_COMPONENTS "all" CACHE STRING
     "Semicolon-separated list of components to include in libLLVM, or \"all\".")
 endif()
 option(LLVM_LINK_LLVM_DYLIB "Link tools against the libllvm dynamic library" OFF)