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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: polly/lib/Support/RegisterPasses.cpp:726
+
+#ifndef LLVM_POLLY_LINK_INTO_TOOLS
+extern "C" LLVM_ATTRIBUTE_WEAK ::llvm::PassPluginLibraryInfo
----------------
Meinersbur wrote:
> [serious] `LLVM_POLLY_LINK_INTO_TOOLS` is a cmake configuration parameter, but not a preprocessor symbol. Hence, `LLVM_POLLY_LINK_INTO_TOOLS` is never defined.
> 
> Error on Windows:
> ```
> Bye.lib(Bye.cpp.obj) : error LNK2005: llvmGetPassPluginInfo already defined in Polly.lib(RegisterPasses.cpp.obj)
> bin\opt.exe : fatal error LNK1169: one or more multiply defined symbols found
> ```
Before you try to fix this preprocessor symbol, consider that Polly compiles a loadable module (to be used with `-load`) __and__ a library (static or dynamic depending on `BUILD_SHARED_LIBS`) __independent__ of `LLVM_POLLY_LINK_INTO_TOOLS`. That is, the loadable module must contain `llvmGetPassPluginInfo`, but not the library. That is, a preprocessor symbol that is the same for both will not work.

PLEASE, just keep the Polly.cpp (and move `llvmGetPassPluginInfo` in there; the static initializer indeed has to stay here as it will be used by both), it will make things easier as it already has been shown to work already. Do the same for Bye. 

If you really insist on removing the `Polly.cpp`, do so in a follow-up patch. In that case you will have to rework the CMakeLists.txt to only build one, either the loadable module or the library.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446