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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini 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()
----------------
serge-sans-paille wrote:
> 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`
Will this work when building LLVM with `cmake -DLLVM_ENABLE_PROJECTS=polly`? It isn't clear to me how the LLVM_EXTENSIONS is set?


================
Comment at: CMakeLists.txt:521
+endforeach()
+file(APPEND "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def" "#undef HANDLE_EXTENSION\n")
 
----------------
serge-sans-paille wrote:
> 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?
I'm not concerned about using a hash, but I'm wonder if there isn't a primitive "copy_if_changed" or similar in CMake that would just diff and copy?
(your solution is fine but less boilerplate/complexity is always welcome)


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

https://reviews.llvm.org/D61446