[PATCH] D19907: cmake: Prefix Polly options with LLVM_ to avoid variable shadowing

[Justin Bogner via llvm-commits]

Tobias Grosser <tobias at grosser.es> writes:
> grosser created this revision.
> grosser added reviewers: bogner, Meinersbur.
> grosser added subscribers: llvm-commits, pollydev.
>
> 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.

This is great. LGTM.

> http://reviews.llvm.org/D19907
>
> Files:
>   CMakeLists.txt
>
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt
> +++ CMakeLists.txt
> @@ -343,8 +343,26 @@
>  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_LINK_POLLY_INTO_TOOLS ON "Static link Polly into tools")
> +option(LLVM_WITH_POLLY ON "Build LLVM with Polly")

Maybe this should be "Build LLVM with Polly (if available)", just to be
ridiculously clear?

> +
> +if (EXISTS ${LLVM_MAIN_SRC_DIR}/tools/polly/CMakeLists.txt)
> +  set(POLLY_IN_TREE TRUE)
> +else()
> +  set(POLLY_IN_TREE FALSE)
> +endif()
> +
> +if (LLVM_WITH_POLLY AND POLLY_IN_TREE)
> +  set(WITH_POLLY ON)
> +else()
> +  set(WITH_POLLY OFF)
> +endif()
> +
> +if (LLVM_LINK_POLLY_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.
> @@ -395,16 +413,6 @@
>  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.
>