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

[Michael Kruse via Phabricator via cfe-commits]

Meinersbur added a comment.

I still have to try out the patch.

In D61446#1756211 <https://reviews.llvm.org/D61446#1756211>, @serge-sans-paille wrote:

> @Meinersbur to make your reviewer job easier, I've setup validation for that patch, see https://github.com/serge-sans-paille/llvm-project/pull/2/checks
>  It build and validates fine.


Thanks!

[suggestion] Also add `-DPOLLY_ENABLE_GPGPU_CODEGEN=ON` and `-DLLVM_ENABLE_ASSERTIONS=ON`, and run `ninja check-polly`. It currently does not run any polly code, just compiles it.



================
Comment at: llvm/docs/WritingAnLLVMPass.rst:1200
+
+- ``LLVM_${NAME}_LINK_INTO_TOOLS``, when sets to ``ON``, turns the project into
+  a statically linked extension
----------------
[typo] when set~~s~~ to


================
Comment at: llvm/test/Feature/load_extension.ll:1-2
+; RUN: opt %s %loadbye -goodbye -wave-goodbye -disable-output > %t.log
+; RUN: FileCheck %s < %t.log
+; REQUIRES: plugins, examples
----------------
[suggestion] The established pattern is to pipe the output to FileCheck without using a temporary file.


================
Comment at: llvm/tools/bugpoint/bugpoint.cpp:230-232
+// Needed to pull in symbols from statically linked extensions, including static
+// registration. It is unused otherwise because bugpoint has no support for
+// NewPM.
----------------
Nice!


================
Comment at: polly/include/polly/RegisterPasses.h:26
 void initializePollyPasses(llvm::PassRegistry &Registry);
-void registerPollyPasses(llvm::legacy::PassManagerBase &PM);
+void RegisterPollyPasses(llvm::PassBuilder &PB);
 } // namespace polly
----------------
[style] [[ https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly | Start function names with lower case letters ]]


================
Comment at: polly/lib/Plugin/Polly.cpp:17-30
+/// Initialize Polly passes when library is loaded.
+///
+/// We use the constructor of a statically declared object to initialize the
+/// different Polly passes right after the Polly library is loaded. This ensures
+/// that the Polly passes are available e.g. in the 'opt' tool.
+class StaticInitializer {
+public:
----------------
[serious] Since `opt`, ... etc. don't call `initializePollyPasses` directly anymore, I think the static initializer has to moved to `RegisterPasses.cpp`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446