Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Windows pass plugin support #101

Open
3 tasks
jvstech opened this issue Oct 14, 2023 · 4 comments
Open
3 tasks

Add Windows pass plugin support #101

jvstech opened this issue Oct 14, 2023 · 4 comments

Comments

@jvstech
Copy link
Collaborator

jvstech commented Oct 14, 2023

Windows pass plugins are supported in LLVM 14+, and llvm-tutor is set up in such a way that enabling them would be a fairly simple process.

Requirements:

  • Export each llvmGetPassPluginInfo function
    • Use a module definitions file (PassName.def) (preferred: requires one file for each plugin and changes to each CMake target definition)
      - [ ] Export every symbol automatically via CMake (not preferred: least amount of work, but pollutes the export table)
      - [ ] Decorate each llvmGetPassPluginInfo with __declspec(dllexport) (not preferred: requires the use of a #define macro to support cross-compilation)
  • Add Windows CI pipelines

@banach-space: What do you think for the symbol exporting options? I prefer module definition files, but I leave that decision up to you. We can discuss it in further detail what would be needed for each option.

@banach-space
Copy link
Owner

Thank you so much for looking into this!

You are the expert here and I am happy to follow your lead/recommendation. I would only suggest that we go for the most canonical approach :)

@OwenTrokeBillard
Copy link

Hi @banach-space @jvstech. Thank you for your work on this tutor.

Enzyme is an established open source tool gaining prominence in the scientific computing community. It is an LLVM plugin that uses an out-of-tree pass to automatically differentiate arbitrary functions.

We are trying to get it to compile on Windows with Clang + MSVC but there are some stubborn linker errors. See EnzymeAD/Enzyme#1607. The errors look related to the current issue and this discussion.

We would be grateful if either of you could peek at the error and see if the cause is obvious given your expertise.

@jvstech
Copy link
Collaborator Author

jvstech commented Jan 28, 2024

Hi @banach-space @jvstech. Thank you for your work on this tutor.

Enzyme is an established open source tool gaining prominence in the scientific computing community. It is an LLVM plugin that uses an out-of-tree pass to automatically differentiate arbitrary functions.

We are trying to get it to compile on Windows with Clang + MSVC but there are some stubborn linker errors. See EnzymeAD/Enzyme#1607. The errors look related to the current issue and this discussion.

We would be grateful if either of you could peek at the error and see if the cause is obvious given your expertise.

Just wanted to point out that the problem was identified, and I believe there is a way forward.

@jvstech
Copy link
Collaborator Author

jvstech commented Feb 3, 2024

I've had some more time to work on this, and I've run into an issue that's been discussed before elsewhere.

Each pass is its own library. The transforms need to link against their related analysis passes in order to use them. That doesn't sound like that big of a deal, except that the pass manager and pass CRTP mixin classes are completely implemented as templates, giving them inline linkage. If the linker doesn't see the symbols being used, it drops them in a cascading manner, and then there aren't any symbols to export for other pass libraries to link against -- even if they're manually decorated with __declspec(dllexport) or listed in a module definitions file.

For example:

  • DuplicateBB uses the analysis results of RIV.
  • RIV defines static llvm::AnalysisKey Key (as do all analysis passes).
  • llvm::AnalysisInfoMixin<DerivedT> defines static llvm::AnalysisKey *ID().
  • llvm::AnalysisManager<...>::registerPass is called by getRIVPluginInfo (which is called by llvmGetPassPluginInfo, which is marked as a DLL export).
  • llvm::AnalysisManager<...>::registerPass runs the lambda passed to it in getRIVPluginInfo, which returns an instance of RIV.
  • llvm::AnalysisManager<...>::registerPass calls ID on the lambda-returned instance of RIV to get a pointer used for registering the pass.

Here's where the problem manifests in this scenario:

  • RIV itself never uses its static Key member, nor does it ever refer to itself via the llvm::AnalysisManager<..>::getResult<PassT>() function (which does call PassT::ID())
  • Because it's not used, RIV::ID() is never created -- even if it's explicitly decorated with __declspec(dllexport).
  • DuplicateBB calls llvm::AnalysisManager<...>::getResult<RIV>() -- in its own library -- which causes RIV::ID() to be marked as an external symbol.
  • The linker sees the symbol use in DuplicateBB and tries to link to it, but because it's not exported (nor does it even exist in the import library -- the static library that contains "thunks" to the real symbols in the DLL), it gives up with an "unresolved symbol" error.

I haven't run into this issue before because every pass library I've ever written to this point has had all its passes -- both analysis and transform -- contained within that library, and they don't call out to other libraries. To be honest, I'm not sure why this works on ELF or Mach-O builds given what I've seen in LLVM's pass manager code, but that's probably due to a lack of knowledge on my part regarding how linking is handled for those formats.

To get everything building and linking properly, I believe we're going to have to change how passes are defined.

  1. We're probably going to have to build every library as an object library and a shared library -- with the shared libraries "linking" against the object libraries (via target_link_libraries, like target_link_libraries(RIV PUBLIC obj.RIV), for example). This ensures that even symbols the linker doesn't think are used will still be added to the library.
  2. We're also going to have to explicitly instantiate at least the ID() function for each of the analysis passes:
extern template static llvm::AnalysisKey *llvm::AnalysisInfoMixin<RIV>::ID();

I think, perhaps the cleanest way to do this is via X-macros, sort of how LLVM uses Passes.def.

Let me know what you think. This is a pretty big change and I don't want to tear up existing code without a consensus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants