[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur requested changes to this revision.
Meinersbur added a comment.
This revision now requires changes to proceed.

It still fails with the same error. `LINK_POLLY_INTO_TOOLS` is only set after the polly subdirectory is processed. Because it is an `option`, the ON value will be stored in the `CMakeCache.txt` and be available during the next run, it actually works after running the cmake configure step a second time. We should not expect users to do so. Because of this, any `option(...)` or `set(... CACHED)` should be done at the beginning of the file.



================
Comment at: llvm/CMakeLists.txt:497
 
-option(LLVM_POLLY_LINK_INTO_TOOLS "Statically link Polly into tools (if available)" ON)
-option(LLVM_POLLY_BUILD "Build LLVM with Polly" ON)
----------------
Note that there is `LLVM_POLLY_LINK_INTO_TOOLS` and `LINK_POLLY_INTO_TOOLS`. The former is the user-configurable option, the latter is for internal querying. At least, this is what is was meant for.


================
Comment at: llvm/CMakeLists.txt:928
 if( LLVM_INCLUDE_TOOLS )
   add_subdirectory(tools)
 endif()
----------------
Polly is included here. `LINK_POLLY_INTO_TOOLS` will not be visible in there if set only later.


================
Comment at: llvm/cmake/modules/AddLLVM.cmake:808
+
+    option(LINK_${llvm_extension_upper}_INTO_TOOLS "Statically link ${llvm_extension_project} into tools (if available)" ON)
+    option(LLVM_${llvm_extension_upper}_BUILD "Build LLVM with ${llvm_extension_project}" ON)
----------------
[remark] Use `LLVM_` prefix for LLVM-level options. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61446/new/

https://reviews.llvm.org/D61446