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

[serge via Phabricator via llvm-commits]

serge-sans-paille added a comment.

In D61446#1681841 <https://reviews.llvm.org/D61446#1681841>, @Meinersbur wrote:

> Keep in mind that for static linking you will need something that pulls-in a symbol from the pass static library. In this patch, `NewPMDriver.cpp` does it for `opt` by calling `get##Ext##PluginInfo()`. In clang, it is `BackendUtil.cpp`. But nothing for `bugpoint` hence it doesn't contain either Polly not the Goodbye pass (However, llvm-reduce is in the works, we might consider bugpoint deprecated).


Done, thanks.

> Could you add documentation for how to write a tool that uses loadable plugins to document the way it should be done?

It's natural if  we use the NewPM, so I've added some doc in that direction.



================
Comment at: polly/lib/Support/RegisterPasses.cpp:726
+
+#ifndef LLVM_POLLY_LINK_INTO_TOOLS
+extern "C" LLVM_ATTRIBUTE_WEAK ::llvm::PassPluginLibraryInfo
----------------
Meinersbur wrote:
> 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.
In `add_llvm_pass_plugin` there's 

```
  if(LLVM_${name_upper}_LINK_INTO_TOOLS)
      target_compile_definitions(${name} PRIVATE LLVM_${name_upper}_LINK_INTO_TOOLS)
      set_property(TARGET ${name} APPEND PROPERTY COMPILE_DEFINITIONS LLVM_LINK_INTO_TOOLS)
      if (TARGET intrinsics_gen)
        add_dependencies(obj.${name} intrinsics_gen)
      endif()
  endif()
```

So the macro definition should be there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61446