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

efriedma added a comment. http://lab.llvm.org:8011/builders/aosp-O3-polly-before-vectorizer-unprofitable is currently broken, I think due to this patch. It looks like polly isn't getting linked into clang by default anymore? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

serge-sans-paille added a comment. In D61446#1805413 , @svenvh wrote: > This change seems to be causing a problem with the nightly packages from > apt.llvm.org. https://reviews.llvm.org/D72255 should do the trick (?) Repository: rG LLVM Github

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

svenvh added a comment. This change seems to be causing a problem with the nightly packages from apt.llvm.org. CMake Error at /usr/lib/llvm-10/lib/cmake/llvm/LLVMExports.cmake:1357 (message): The imported target "Bye" references the file "/usr/lib/llvm-10/lib/libBye.a"

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

fhahn added a comment. There seems to be a problem with this patch on macOS. I've XFAIL'd it in rGdb82fc5dd80ff14798e7f1c35dd7e593f6409ba3 . This should unblock the macOS builders, but please take a look at the problem.

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

sdmitriev added inline comments. Comment at: llvm/test/Other/opt-O2-pipeline.ll:1 -; RUN: opt -mtriple=x86_64-- -O2 -debug-pass=Structure < %s -o /dev/null 2>&1 | FileCheck %s +; RUN: opt -mtriple=x86_64-- -O2 -debug-pass=Structure < %s -o /dev/null 2>&1 | FileCheck

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

serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. serge-sans-paille added inline comments. Comment at: llvm/test/lit.site.cfg.py.in:6 +def cmake_bool(val): +return val.lower() in (1, "on", "yes", "true", "y",) + thakis

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

thakis added inline comments. Comment at: llvm/test/lit.site.cfg.py.in:6 +def cmake_bool(val): +return val.lower() in (1, "on", "yes", "true", "y",) + FYI, this is a now-discouraged pattern. It's better to use llvm_canonicalize_cmake_booleans in the cmake

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG24ab9b537e61: Generalize the pass registration mechanism used by Polly to any third-party tool (authored by serge-sans-paille). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

serge-sans-paille updated this revision to Diff 234501. serge-sans-paille added a comment. Patch rebased, validation ongoing. I'll merge that once I'm back from holidays, don't feel like commiting that 3h before holidays. Thanks **a lot** @Meinersbur for all the reviews! Repository: rG LLVM

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

Meinersbur added a comment. Btw, this patch did not apply cleanly on trunk. I instead test the version from your repository . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. I tested the configurations mentioned at https://groups.google.com/d/topic/polly-dev/vxumPMhrSEs/discussion again and did not find any issues that were not there before. Therefore

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

serge-sans-paille updated this revision to Diff 232690. serge-sans-paille added a comment. @Meinersbur : with this version, validation passes using static build on three platforms, with the following config: -DLLVM_ENABLE_PROJECTS='polly;clang' -DPOLLY_ENABLE_GPGPU_CODEGEN=ON

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

Meinersbur added a comment. In D61446#1763278 , @serge-sans-paille wrote: > @Meinersbur I have a strange symbol issue when activating the GPu part: > https://github.com/serge-sans-paille/llvm-project/pull/2/checks?check_run_id=324901896#step:5:4888 >

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

serge-sans-paille added a comment. @Meinersbur I have a strange symbol issue when activating the GPu part: https://github.com/serge-sans-paille/llvm-project/pull/2/checks?check_run_id=324901896#step:5:4888 Any hint? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

serge-sans-paille added a comment. @Alouest thanks for the reproducer. I confirm your issue, and also confirm that if you pass `-fexperimental-new-pass-manager -O1` to clang (i.e. use the new pass manager) it works fine. As far as I can tell, the segfault happens because `RegisterPass` also

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

Alouest added a comment. Greetings, I stumbled upon a bug while toying with your example Bye.cpp. I modified your pass to make a ModulePass and while it works with /build/bin/opt -goodbye -wave-goodbye test.ll It's making clang crash with the command /build/bin/clang test.c I've

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

serge-sans-paille updated this revision to Diff 231092. serge-sans-paille added a comment. Handle all @Meinersbur comment, rebuild on github with extra flags pending. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/

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

Meinersbur added a comment. I still have to try out the patch. In D61446#1756211 , @serge-sans-paille wrote: > @Meinersbur to make your reviewer job easier, I've setup validation for that > patch, see

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

serge-sans-paille added a comment. @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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

serge-sans-paille added a comment. In 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

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

serge-sans-paille updated this revision to Diff 230415. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. Take into account most of the reviews. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/

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

serge-sans-paille updated this revision to Diff 229127. serge-sans-paille added a comment. First rework pass, sevral comments have been taken into account but not all of them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/

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

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

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

Meinersbur added a comment. 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`

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

Meinersbur requested changes to this revision. Meinersbur added inline comments. This revision now requires changes to proceed. Comment at: llvm/test/Feature/load_extension.ll:1 +; RUN: not test -f %llvmshlibdir/libBye%shlibext || opt %s -load=%llvmshlibdir/libBye%shlibext

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

serge-sans-paille updated this revision to Diff 221321. serge-sans-paille added a comment. Updates: - fix typo in documentation - take into account @andwar advices - improve shared/static build automation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

andwar added inline comments. Comment at: llvm/examples/Bye/Bye.cpp:53 +/* New PM Registration */ + +llvm::PassPluginLibraryInfo getByePluginInfo() { [nit] Empty line Comment at: llvm/examples/Bye/CMakeLists.txt:9 +else() +

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

serge-sans-paille added a comment. @Meinersbur any feedback on this update? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list

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

serge-sans-paille added a comment. Up :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

serge-sans-paille updated this revision to Diff 217604. serge-sans-paille added a comment. @Meinersbur patch rebased. I removed the linker trick (which only work for global variables, not function, anyway), as it's no longer needed: target_compile_definitions(${name} PRIVATE

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

Meinersbur added a comment. Sorry for the break. Unfortunately the patch does not apply cleanly anymore. Can you rebase to latest trunk? I surprisingly did not know about `__declspec(selectany)`. I still think having the symbol only defined in that loadable module, but not in the statically

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

serge-sans-paille added a comment. @Meinersbur up! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

serge-sans-paille updated this revision to Diff 213176. serge-sans-paille added a comment. Also fix symbol redefinition during static builds on Windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446

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

serge-sans-paille added a comment. @Meinersbur nop, forgot that one, I'll have a look, thanks for pointing that out. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list

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

Meinersbur added a comment. Did you resolve the conflicting `llvmGetPassPluginInfo` symbols for windows? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list

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

serge-sans-paille updated this revision to Diff 212567. serge-sans-paille added a comment. Make validation more resilient depending on shared/static build. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 Files:

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

serge-sans-paille added a comment. @Meinersbur I've updated the test case to test extension point if the extension is linked in, this is not so intrusive, I'm happy with the solution. Also fixed some linkage options when using llvm dynlib, validation looks nice on my side. Repository: rG

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

serge-sans-paille updated this revision to Diff 212509. serge-sans-paille added a comment. Herald added subscribers: dexonsmith, steven_wu. - Test extension point registration when extension is built-in - Correctly handle -DLLVM_LINK_LLVM_DYLIB=0 - Fix a few details Repository: rG LLVM Github

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

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. Thank you for adding the Bye pass. It is really useful! Is there a specific reason to not modify the Hello pass? --- If I enable both passes statically

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

serge-sans-paille updated this revision to Diff 210273. serge-sans-paille marked 5 inline comments as done and 2 inline comments as done. serge-sans-paille added a comment. Added a `Bye` project in examples/ Fixed linking of plugins into core tools Fixed dependency issue Repository: rG LLVM

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

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. Windows seems to work. Good job! Linux works with static libraries, but not with `BUILD_SHARED_LIBS=ON`: $ bin/opt : CommandLine Error: Option

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

serge-sans-paille marked 10 inline comments as done. serge-sans-paille added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:808 +if(NOT llvm-pass-plugins) +# Target used to hold global properties referencable from generator-expression

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

serge-sans-paille updated this revision to Diff 206648. serge-sans-paille marked an inline comment as done. serge-sans-paille added a comment. @Meinersbur your comment and my devs crossed, but this should be fine. This update enables new PM static plugin support for clang, something that was

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

Meinersbur added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:808 +if(NOT llvm-pass-plugins) +# Target used to hold global properties referencable from generator-expression [serious] I get the following error: ``` CMake Error at

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

serge-sans-paille updated this revision to Diff 205298. serge-sans-paille added a comment. Mostly documentation update + helper function renaming. @meinersbur, I've tested the setup with static and dynamic builds on Linux, please let me know your thoughts on this. CHANGES SINCE LAST ACTION

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

serge-sans-paille added inline comments. Comment at: polly/lib/Support/RegisterPasses.cpp:727 +extern "C" ::llvm::PassPluginLibraryInfo LLVM_ATTRIBUTE_WEAK +llvmGetPassPluginInfo() { + return getPassPluginInfo(); Meinersbur wrote: > [serious] Unfortunately, the

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

serge-sans-paille updated this revision to Diff 204073. serge-sans-paille marked 3 inline comments as done. serge-sans-paille added a comment. @beanz : In addition to your suggested changes, I've renamed the cmake utility into `add_llvm_pass_plugin` to match other functions from `AddLLVM.cmake`

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

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. In D61446#1533076 , @serge-sans-paille wrote: > That's what I tried to do when always adding Polly.cpp to PollyCore, but it >

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

beanz added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:812 +# llvm::PassPluginLibraryInfo ${entry_point}(); +add_custom_target(LLVM_PLUGINS) # target used to hold global properties referencable from generator-expression

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

serge-sans-paille updated this revision to Diff 203697. serge-sans-paille added a comment. Using @beanz idea, provide an option to `add_llvm_executable` to declare it as receiving plugins, which provides lower coupling with actual plugins Tested with four configurations: `BUILD_SHARED_LIBS`

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

serge-sans-paille added a comment. In D61446#1532848 , @Meinersbur wrote: > Just an idea: We could avoid the explicit calls to 'initializeXYZPass' in > opt/bugpoint/clang by adding Polly.cpp as a source file or object library to > the executables. This

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

Meinersbur added a comment. Just an idea: We could avoid the explicit calls to 'initializeXYZPass' in opt/bugpoint/clang by adding Polly.cpp as a source file or object library to the executables. This would guarantee that its static initializer is called without dynamic library. Repository:

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

serge-sans-paille added a comment. @Meinersbur Itested with the following configuration cmake3 ../llvm -DLLVM_EXTERNAL_PROJECTS=bye -DBUILD_SHARED_LIBS=ON -DCMAKE_BUILD_TYPE=Release -DLLVM_TARGETS_TO_BUILD=X86 cmake3 ../llvm -DLLVM_EXTERNAL_PROJECTS=bye -DBUILD_SHARED_LIBS=ON

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

beanz added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:820 + +set(LLVM_ALL_EXTENSION_TARGETS clang;bugpoint;opt) +foreach(tool ${LLVM_ALL_EXTENSION_TARGETS}) I don't think we should hard code this list. It would be much

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

Meinersbur added a comment. Sorry, you might have tested it with `LLVM_LINK_POLLY_INTO_TOOLS=OFF` and/or `BUILD_SHARED_LIBS`/`LLVM_LINK_LLVM_DYLIV` where it might work, but unfortunately not with the default configuration using static component libraries. Repository: rG LLVM Github

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

Meinersbur added a comment. This fails with Polly/Linux regression tests: FAIL: Polly :: Simplify/gemm.ll (1148 of 1149) TEST 'Polly :: Simplify/gemm.ll' FAILED Script: -- : 'RUN: at line 1'; opt

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

serge-sans-paille updated this revision to Diff 203086. serge-sans-paille added a comment. - get Rid of the constraint on initializePasses : it can be registered suing a static constructor, no need to clutter the API with it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

serge-sans-paille updated this revision to Diff 203029. serge-sans-paille added a comment. - make it possible for client code to speicify the registration function for new PM, thanks @philip.pfaffe for the hint. @Meinersbur this should be ok now. CHANGES SINCE LAST ACTION

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

serge-sans-paille updated this revision to Diff 202950. serge-sans-paille added a comment. Take into account @philip.pfaffe remarks and improve support for new PM. - harmonize static/dynamic loading, although two functions are still needed, one for dynamic loading and one for static loading

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

serge-sans-paille added a comment. @beanz turns out the depdendency problem was just a matter of moving the `register_llvm_extension` call at the top of the file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/

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

serge-sans-paille added a comment. @philip.pfaffe : NewPM integeration actually works thanks to ``Register${Name}Passes(llvm::PassBuilder )``. It's already used by Polly for NewPM integration and we mimic that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

serge-sans-paille updated this revision to Diff 202908. serge-sans-paille added a comment. - reorder registration to make sure options are correctly taken into account - make LINK_POLLY_INTO_TOOLS obsolete in favor of just using LLVM_LINK_POLLY_INTO_TOOLS Repository: rG LLVM Github Monorepo

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

beanz added a comment. I have a thought for how we can make the CMake interface for this better. Instead of `LLVM_EXTENSIONS` being a global property if it were a pre-defined list like `LLVM_ALL_PROJECTS`, then the `LLVM_LINK__INTO_TOOLS` variables can be generated by LLVM's configuration

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

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. It still fails with the same error. `LINK_POLLY_INTO_TOOLS` is only set after the polly subdirectory is processed. Because it is an `option`, the ON value will be stored in

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

philip.pfaffe added a subscriber: beanz. philip.pfaffe added a comment. I fear you'll need to plan at least for new pm right now. Otherwise your change will be obsolete in half a year. What is it really that LINK_X_INTO_TOOLS is supposed to do? Effectively bake in something that's consumable by

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

serge-sans-paille updated this revision to Diff 202795. serge-sans-paille added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. - provide minimal documentation - fix testing configuration - validate locally with both shared and static libraries, using monorepo or