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

[serge via Phabricator via llvm-commits]

serge-sans-paille marked 5 inline comments as done.
serge-sans-paille added inline comments.


================
Comment at: CMakeLists.txt:512
+    string(TOLOWER ${llvm_extension_lib} llvm_extension_dir)
+    include(${LLVM_MAIN_SRC_DIR}/tools/${llvm_extension_dir}/cmake/extension.cmake OPTIONAL)
+endforeach()
----------------
mehdi_amini wrote:
> We shouldn't hardcode anything with respect to the "tools" folder in llvm. I see that your patch is setup like if clang is in llvm/tools, but the current recommended setup for laying out the directories for subproject is "side by side" in the monorepo: https://github.com/llvm/llvm-project/
I've tried to make it compatible with llvm-project while maintaining current status wrt. `tools`


================
Comment at: CMakeLists.txt:521
+endforeach()
+file(APPEND "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def" "#undef HANDLE_EXTENSION\n")
 
----------------
mehdi_amini wrote:
> Is this file gonna be re-written for each re-execution of CMake?
> Will this trigger more recompilation than needed?
> If so what can be done about this? Can we generate a copy and replace the final one only if it differs in content? Can we make re-writing the file a rule based on actual dependencies (a target depending on the content of `LLVM_EXTENSION_LIBS` changing)?
Using as shA256 is not 100% bullet proof but should be ok, right?


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

https://reviews.llvm.org/D61446