Generalize the pass registration mechanism used by Polly to any third-party tool

[serge-sans-paille]

Summary

There's quite a lot of references to Polly in the LLVM CMake codebase. However the registration pattern used by Polly could be useful to other external projects: thanks to that mechanism it would be possible to develop LLVM extension without touching the LLVM code base.

This an attempt at doing so, using Polly as reference.

[mehdi_amini]

CMakeLists.txt

1125

Where is this property populated?

cmake/modules/AddLLVM.cmake

811

If the target has to exist here and since you're using this to make it link a library, why are you going through properties in the first place? Why can't you do here: `target_link_libraries(${tool} PRIVATE ${name})

[Meinersbur]

In D61446#1493738, @serge-sans-paille wrote:

@Meinersbur, I've tested and reviewed this patch, I'm curious about Polly's team opinion on the changes in current state?

I will have to try out the patch, e.g. on Windows. Please ping me if it takes to long.

cmake/modules/AddLLVM.cmake

808

According to a recent mailing list posting, lld might need to have such tools linked-in as well for LTO.

tools/polly/CMakeLists.txt

212

This was likely added as one of my comments: Polly-ACC requires the NVPTX target to be present in the executable it is linked to. bugpoint by default does not depend on LLVMTarget, i.e. does not contain the NVPTX target.

[mehdi_amini]

tools/polly/CMakeLists.txt

212

Yes, I saw the code removed in CMakeLists.txt after writing this comment but forgot to delete it before sending, sorry.

[serge-sans-paille]

I will have to try out the patch, e.g. on Windows. Please ping me if it takes to long.

Thanks @Meinersbur

CMakeLists.txt

1124

I was thinking « X Macro » :-) https://en.wikipedia.org/wiki/X_Macro

1125

Fixed in register_llvm_extension

cmake/modules/AddLLVM.cmake

811

Because it's illegal in cmake to call target_link_libraries in a different file, see https://cmake.org/cmake/help/v3.0/command/target_link_libraries.html (this has been changed in recent version of cmake though)

[serge-sans-paille]

@Meinersbur I'll be happy to read your feedback on windows compilation!

[Meinersbur]

This didn't work for me, neither on Windows nor on Linux.

Linux error:

[138/138] Linking CXX executable bin/opt
/usr/bin/ld: cannot find -lpolly
collect2: error: ld returned 1 exit status

Windows error:

[1654/1715] Linking CXX executable bin\opt.exe
LINK : fatal error LNK1181: cannot open input file 'polly.lib'

This might indicate that "Polly" is not a recognized target during target_link_libraries. If it did, it would add lib/libPolly.a instead of -lPolly to the linker command line. My Polly is in ${LLVM_ROOT}/tools/Polly. I can see it has been picked up since ninja Polly works and "-- Registering polly as a compiler extension" is printed as well. My Linux cmake-line is:

cmake "${SRCPATH}" \
  -GNinja \
  -DCMAKE_C_COMPILER=/soft/compilers/gcc/8.2.0/linux-rhel7-x86_64/bin/gcc \
  -DCMAKE_CXX_COMPILER=/soft/compilers/gcc/8.2.0/linux-rhel7-x86_64/bin/g++ \
  -DCMAKE_INSTALL_PREFIX="${INSTALLPATH}" \
  -DCLANG_ENABLE_STATIC_ANALYZER=OFF \
  -DLLVM_PARALLEL_LINK_JOBS=2 \
  -DCLANG_ENABLE_ARCMT=OFF \
  -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
  '-DLLVM_TARGETS_TO_BUILD=X86;NVPTX' \
  -DLLVM_ENABLE_ASSERTIONS=ON \
  -DCMAKE_BUILD_TYPE=Release \
  -DPOLLY_ENABLE_GPGPU_CODEGEN=ON \
  -DLLVM_TEMPORARILY_ALLOW_OLD_TOOLCHAIN=ON \
  -DCUDA_TOOLKIT_ROOT_DIR=/soft/compilers/cuda/cuda-10.0.130/

My Extension.def contains:

//extension handlers
HANDLE_EXTENSION(polly, Polly)
#undef HANDLE_EXTENSION

[serge-sans-paille]

@Meinersbur thanks a lot for the feedback. I was only testing with the new project layout, so it's nice you tested with the legacy layout.
I've updated the diff accordingly, it now compiles fine on my Linux box with both old-style and new-style layout.

Can you confirm it's ok on your side? I don't have access to a Windows box.

[serge-sans-paille]

@Meinersbur up!

[philip.pfaffe]

Unfortunately this will stop working as soon as we switch on the new pass manager. With that, there is no canonical initialize*Passes that you can call. Do you have a plan to support the new pass manager as well?

[Meinersbur]

Fail the regression tests under Linux:

$ ninja check-polly
[...]
: 'RUN: at line 1';   opt -load /home/meinersbur/build/llvm/release/lib/LLVMPolly.so -load-pass-plugin /home/meinersbur/build/llvm/release/lib/LLVMPolly.so [...]
--
Exit Code: 2

Command Output (stderr):
--
opt: CommandLine Error: Option 'polly-dependences-computeout' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options
[...]

This error typically appears when polly is statically linked into tools (LINK_POLLY_INTO_TOOLS=ON), but the shared library is loaded as well at run-time (-load /home/meinersbur/build/llvm/release/lib/LLVMPolly.so). You probably will have to adjust polly/test/lit.site.cfg.in:

if config.link_polly_into_tools == '' or \
   config.link_polly_into_tools.lower() == '0' or \
   config.link_polly_into_tools.lower() == 'n' or \
   config.link_polly_into_tools.lower() == 'no' or \
   config.link_polly_into_tools.lower() == 'off' or \
   config.link_polly_into_tools.lower() == 'false' or \
   config.link_polly_into_tools.lower() == 'notfound' or \
   config.link_polly_into_tools.lower() == 'link_polly_into_tools-notfound':

To these long turnarounds, you could test these configurations yourself. Some time ago I made an overview on which combinations of dynamic/static linking should work: https://groups.google.com/d/msg/polly-dev/vxumPMhrSEs/E50kboqlAQAJ

[Meinersbur]

For LTO, the llvm_extensions might need to be linked into lld as well. The recent polly-dev thread: https://groups.google.com/d/msg/polly-dev/BeC_v_oJIVM/LAA7vVT9AwAJ

However, since we don't even have this for Polly, I would leave it up to you whether to include this in this patch review.

[serge-sans-paille]

• provide minimal documentation
• fix testing configuration
• validate locally with both shared and static libraries, using monorepo or tool layout
• update sample https://sergesanspaille.fedorapeople.org/bye.tgz

@Meinersbur thanks a lot for the previous review! lld integration, as well as new PM integration, will be for another commit, in order to prevent this review from growing too large

[philip.pfaffe]

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 -load or -load-plugin, right? So can we use that interface directly? Effectively, can we have statically built-in plugins, that prepopulate the plug-in loader mechanisms?

If the patch becomes too large then it should be split into separate functional units, e.g., one adding the cmake functionality, one updating the plugins and the tools.

You could also ask @beanz for a review of the cmake change.

[Meinersbur]

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 the CMakeCache.txt and be available during the next run, it actually works after running the cmake configure step a second time. We should not expect users to do so. Because of this, any option(...) or set(... CACHED) should be done at the beginning of the file.

llvm/CMakeLists.txt

497 ↗

(On Diff #202795)

Note that there is LLVM_POLLY_LINK_INTO_TOOLS and LINK_POLLY_INTO_TOOLS. The former is the user-configurable option, the latter is for internal querying. At least, this is what is was meant for.

928 ↗

(On Diff #202795)

Polly is included here. LINK_POLLY_INTO_TOOLS will not be visible in there if set only later.

llvm/cmake/modules/AddLLVM.cmake

808 ↗

(On Diff #202795)

[remark] Use LLVM_ prefix for LLVM-level options.

[beanz]

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_<X>_INTO_TOOLS variables can be generated by LLVM's configuration instead of relying on Polly's to run. If you do that LLVM_LINK_POLLY_INTO_TOOLS could be set to On even if Polly isn't enabled to build, but that actually will be fine if you change the calls to target_link_libraries in the tools to use a generator expression something like: $<$<TARGET_EXISTS:Polly>,Polly>.

These changes should eliminate the order-dependence on the tools and allow the first configuration run to be accurate.

[serge-sans-paille]

• 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

[serge-sans-paille]

@philip.pfaffe : NewPM integeration actually works thanks to `Register${Name}Passes(llvm::PassBuilder &PB)`. It's already used by Polly for NewPM integration and we mimic that.

[serge-sans-paille]

@beanz turns out the depdendency problem was just a matter of moving the register_llvm_extension call at the top of the file.

[serge-sans-paille]

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 (new PM)
• do not clutter the required API with extra namespaces

I've updated https://sergesanspaille.fedorapeople.org/bye.tgz to showcase a minimal example.

[serge-sans-paille]

• 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.

[serge-sans-paille]

• get Rid of the constraint on initializePasses : it can be registered suing a static constructor, no need to clutter the API with it

[Meinersbur]

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  -polly-process-unprofitable  -polly-remarks-minimal  -polly-use-llvm-names  -polly-import-jscop-dir=/home/meinersbur/src/llvm/tools/polly/test/Simplify  -polly-codegen-verify  -polly-import-jscop    -polly-import-jscop-postfix=transformed -polly-simplify -analyze < /home/meinersbur/src/llvm/tools/polly/test/Simplify/gemm.ll    | FileCheck /home/meinersbur/src/llvm/tools/polly/test/Simplify/gemm.ll
--
Exit Code: 2

Command Output (stderr):
--
opt: for the   -o option: may not occur within a group!
opt: Unknown command line argument '-polly-import-jscop'.  Try: 'opt --help'
opt: Did you mean '  -o'?
opt: for the   -p option: may only occur zero or one times!
opt: for the   -o option: may not occur within a group!
opt: Unknown command line argument '-polly-simplify'.  Try: 'opt --help'
opt: Did you mean '  -o'?
FileCheck error: '-' is empty.
FileCheck command line:  FileCheck /home/meinersbur/src/llvm/tools/polly/test/Simplify/gemm.ll

I think this means that the Polly passes have not been registered (initializePollyPasses must be called someway in opt/clang_cc1/bugpoint). Linking from static libraries will NOT include Polly.o (and run its static initializers) unless it is needed to resolve at least one symbol.

PLEASE run make/ninja check-polly before uploading a patch.

llvm/tools/opt/opt.cpp

533 ↗

(On Diff #203086)

Where is the equivalent for this in your change? I see it's been done for clang_cc1, but not for opt/bugpoint.

polly/CMakeLists.txt

210 ↗

(On Diff #203086)

[nit] Whitespace change

polly/lib/CMakeLists.txt

27 ↗

(On Diff #203086)

Why this change? Polly.cpp should only be necessary for the loadable module, but not for LLVM_LINK_POLLY_INTO_TOOLS.

[Meinersbur]

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.

[beanz]

llvm/cmake/modules/AddLLVM.cmake

820 ↗

(On Diff #203086)

I don't think we should hard code this list. It would be much better if we add an option to llvm_add_executable so that extension targets denote themselves.

822 ↗

(On Diff #203086)

if(TARGET ...) is order dependent. That's why you need to change tools/CMakeLists.txt, which you won't need to do if you change this to work how I suggested in my earlier comment.

[serge-sans-paille]

@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 -DCMAKE_BUILD_TYPE=Release  -DLLVM_TARGETS_TO_BUILD=X86 -DLLVM_POLLY_LINK_INTO_TOOLS=OFF

with old and mono repo layout. I'll test with BUILD_SHARED_LIBS=OFF too.

[Meinersbur]

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.

[serge-sans-paille]

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 would guarantee that its static initializer is called without dynamic library.

That's what I tried to do when always adding Polly.cpp to PollyCore, but it doesn't seem to be enough, I still need to invstigate why (it is actually enough when using BUILD_SHARED_LIBS=ON)

[serge-sans-paille]

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 ON/OFF and with llvm-project / legacy configuration, but *not* on Windows

As suggested by @Meinersbur, use static constructor for initialization step with legacy PM, for both shared and static libraries.

The patch looks much less intrusive now, thanks a lot to all reviewers for the provided guidance.

[beanz]

llvm/cmake/modules/AddLLVM.cmake

812 ↗

(On Diff #203697)

Change this to llvm-plugins to match our convention and wrap it in if (NOT TARGET...) so it doesn't error if AddLLVM is included twice.

[Meinersbur]

In D61446#1533076, @serge-sans-paille wrote:

That's what I tried to do when always adding Polly.cpp to PollyCore, but it doesn't seem to be enough, I still need to invstigate why (it is actually enough when using BUILD_SHARED_LIBS=ON)

With static libraries, you had the following structure:

PollyCore.a:
  RegisterPasses.o
  Polly.o <-- static initializer here, but never used

LLVMPolly.so (for use with -load mechanism)
  RegisterPasses.o
  Polly.o <-- static initializer here, executed when loading the .so

opt:
  main.o <-- call to initializePollyPass removed

With static linking, there is no reason for the linker to add Polly.o to the opt executable because no symbol from Polly.o (or any of PollyCore) was required to resolve any symbol in opt. Building a shared library using the object library files will include all object files, since it does not know which symbols will be used at runtime. I recommend reading http://www.lurklurk.org/linkers/linkers.html#staticlibs and https://www.bfilipek.com/2018/02/static-vars-static-lib.html.

What I proposed was

PollyCore.a:
  RegisterPasses.o

LLVMPolly.so (for use with -load mechanism)
  RegisterPasses.o
  Polly.o

opt: // or any ENABLE_PLUGINS tool
  main.o
  Polly.o // via add_executable(opt Polly.cpp) or target_sources

Adding the object file explicitly to the add_executable will add it unconditionally, even if no none of its symbol is required, like for LLVMPolly.so.

In the latest update you changed the location of the static initalizer to RegisterPasses.o, which contains the symbol llvmGetPassPluginInfo which is pulled-in by the new pass manager plugin system. This makes an unfortunate dependence of the static initializer on the llvmGetPassPluginInfo mechanism (which I think only works for opt in the current patch).

There is a reason why pass loading in Polly is organized as it is.

llvm/cmake/modules/AddLLVM.cmake

812 ↗

(On Diff #203697)

[serious] I get multiple of these errors by cmake:

CMake Error at cmake/modules/AddLLVM.cmake:812 (add_custom_target):
  add_custom_target cannot create target "LLVM_PLUGINS" because another
  target with the same name already exists.  The existing target is a custom
  target created in source directory "/home/meinersbur/src/llvm".  See
  documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
  projects/compiler-rt/unittests/CMakeLists.txt:2 (include)

llvm/tools/opt/NewPMDriver.cpp

292 ↗

(On Diff #203697)

[serious] This will pull-in the symbol llvmGetPassPluginInfo from the plugin for opt, but what about the other tools (bugpoint/clang)?

polly/lib/Support/RegisterPasses.cpp

727 ↗

(On Diff #203697)

[serious] Unfortunately, the new pass manager's plugin system relies on the function name to be llvmGetPassPluginInfo in each plugin. This works with multiple dynamic libraries all declaring the same name using the PassPlugin::Load mechanism, but linking them all statically will violate the one-definition-rule.

IMHO, Polly.cpp would have been a better place for this function.

[serge-sans-paille]

@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

@Meinersbur : thanks a lot for the polly layout explanation. I've slighlty updated your suggection like this:

PollyCore.a:
  RegisterPasses.o
  polly_static_entry_point.cpp

LLVMPolly.so (for use with -load mechanism)
  RegisterPasses.o
  polly_plugin_entry_point.cpp

initialization is done in both cases through a static initializer in RegisterPasses.cpp. llvmGetPassPluginInfo is only available in polly_plugin_entry_point.cpp.

polly/lib/Support/RegisterPasses.cpp

727 ↗

(On Diff #203697)

but linking them all statically will violate the one-definition-rule.

They are unused when liked statically, and flagged as weak to avoid link-time conflict.

IMHO, Polly.cpp would have been a better place for this function.

I still agree it's more explicit if linked conditionaly.

[serge-sans-paille]

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.

[Meinersbur]

llvm/cmake/modules/AddLLVM.cmake

808 ↗

(On Diff #205298)

[serious] I get the following error:

CMake Error at cmake/modules/AddLLVM.cmake:813 (add_custom_target):
  add_custom_target cannot create target "llvm-pass-plugins" because another
  target with the same name already exists.  The existing target is a custom
  target created in source directory "/home/meinersbur/src/llvm".  See
  documentation for policy CMP0002 for more details.
Call Stack (most recent call first):
  projects/compiler-rt/test/CMakeLists.txt:2 (include)

What you meant to use is

if (NOT TARGET llvm-pass-plugins)

See https://cmake.org/cmake/help/latest/command/if.html

851 ↗

(On Diff #205298)

[serious] Under Windows, I get the following error:

CMake Error at cmake/modules/AddLLVM.cmake:853 (target_sources):
  target_sources called with non-compilable target type
Call Stack (most recent call first):
  tools/polly/lib/CMakeLists.txt:164 (register_llvm_pass_plugin)

This is because "LLVMPolly" is not a library, but a dummy "add_custom_target" since loadable modules are not supported under Windows.

llvm/docs/WritingAnLLVMPass.rst

1222 ↗

(On Diff #205298)

"out-of-tree" is the wrong term. This registration only works if the plugin if configured in the same cmake-run. "out-of-tree" would describe a processes with a separate cmake source- and build-tree using LLVM_MAIN_SRC_DIR or https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project

llvm/tools/CMakeLists.txt

45 ↗

(On Diff #205298)

What is the reason to have this after add_llvm_implicit_projects in contrast to the other LLVM subprojects?

polly/lib/Support/RegisterPasses.cpp

727 ↗

(On Diff #203697)

You seem to have removed the weak attribute. Did you mean to put it into the polly namespace to avoid name clashing with other plugins?

[serge-sans-paille]

@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 lacking to polly previously. I've tested it a bit and it builds fine with static setup, still need to test it with more config.

[serge-sans-paille]

llvm/cmake/modules/AddLLVM.cmake

808 ↗

(On Diff #205298)

I'm no longer using that mechanism, it was showing some limits wrt. cmake generator-expression and export set.

851 ↗

(On Diff #205298)

I'm no longer using target_sources, it should be fine now.

llvm/docs/WritingAnLLVMPass.rst

1222 ↗

(On Diff #205298)

I'm not super happy with the new title, but I gave it a try.

llvm/tools/CMakeLists.txt

45 ↗

(On Diff #205298)

polly used to be included before the other external projects, so that clang/opt could depend on it. We are now injecting dependencies a posteriori, so polly needs to be included after opt/clang/bugpoint. I've put it right before the loop on add_llvm_external_project because they share the same function call name.

polly/lib/Support/RegisterPasses.cpp

727 ↗

(On Diff #203697)

There are two entry points: llvmGetPassPluginInfo for new PM (marked as weak) and get##name##PluginInfo for legacy PM (name is supposed to be unique, no need for weak linkage)

[Meinersbur]

Windows seems to work. Good job!

Linux works with static libraries, but not with BUILD_SHARED_LIBS=ON:

$ bin/opt
: CommandLine Error: Option 'polly-dependences-computeout' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options

This error is typical for having translation units (in this case: Polly's DependenceInfo.cpp) multiple times in the address space.

See https://groups.google.com/forum/#!topic/polly-dev/vxumPMhrSEs for the configurations I intend to test and which currently work.

llvm/cmake/modules/AddLLVM.cmake

815 ↗

(On Diff #206648)

Why use cmake_parse_arguments if there are no options to parse?

825 ↗

(On Diff #206648)

[concern] This requires that all plugin-able tool have been registered before. It might be confusing that not all plugins will be loaded into every tool if the relative order is not <all add_llvm_executable(... ENABLE_PLUGINS)> <all add_llvm_pass_plugin(...)>.

Is it possible to error-out if a ENABLE_PLUGINS happens after a plugin has been added via add_llvm_pass_plugin?

827 ↗

(On Diff #206648)

This injects all plugin sources directly into tool executable (in addition to loading them as a library with LINK_LIBRARIES), probably the reason for the error I see with BUILD_SHARED_LIBS.

It ignores the library separation, which is not a nice solution for the same reasons why LLVM does not simply add all object files from all its libraries to each of the add_llvm_executables, but instead uses target_link_libraries.

llvm/cmake/modules/LLVMProcessSources.cmake

112 ↗

(On Diff #206648)

[nit] unrelated?

llvm/docs/WritingAnLLVMPass.rst

1240 ↗

(On Diff #206648)

"Out-of-tree" still mentioned here.

polly/lib/Support/RegisterPasses.cpp

727 ↗

(On Diff #203697)

Unfortunately, the Windows platform has no concept of weak symbols.

get##name##PluginInfo is not related to the legacy pass manager. The legacy passe manager uses llvm::PassRegistry and llvm::RegisterStandardPasses. polly::RegisterPollyPasses is only used by the new pass manager.

Could you create a second plugin using add_llvm_pass_plugin, for instance convert LLVMHello? Then we could check whether this works.

[serge-sans-paille]

Added a Bye project in examples/
Fixed linking of plugins into core tools
Fixed dependency issue

[Meinersbur]

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 (LLVM_BYE_LINK_INTO_TOOLS=ON and LLVM_POLLY_LINK_INTO_TOOLS=ON), the following regression tests fail (ninja check-llvm):

Failing Tests (8):
    LLVM :: DebugInfo/debugify-each.ll
    LLVM :: Other/new-pm-defaults.ll
    LLVM :: Other/new-pm-thinlto-defaults.ll
    LLVM :: Other/opt-O0-pipeline.ll
    LLVM :: Other/opt-O2-pipeline.ll
    LLVM :: Other/opt-O3-pipeline.ll
    LLVM :: Other/opt-Os-pipeline.ll
    LLVM :: Transforms/LoopVectorize/X86/reg-usage.ll

The pass output such as

Bye: foo
Bye: goo
Bye: bar
Bye: hoo

seem to interfere with the CHECK lines in the test cases.

---

Using the configuration LLVM_LINK_LLVM_DYLIB=1 and LLVM_POLLY_LINK_INTO_TOOLS=ON, the following tests fail:

Failing Tests (10):
    LLVM :: BugPoint/compile-custom.ll
    LLVM :: BugPoint/crash-narrowfunctiontest.ll
    LLVM :: BugPoint/func-attrs-keyval.ll
    LLVM :: BugPoint/func-attrs.ll
    LLVM :: BugPoint/invalid-debuginfo.ll
    LLVM :: BugPoint/metadata.ll
    LLVM :: BugPoint/named-md.ll
    LLVM :: BugPoint/remove_arguments_test.ll
    LLVM :: BugPoint/replace-funcs-with-null.ll
    LLVM :: BugPoint/unsymbolized.ll

The error output is:

: CommandLine Error: Option 'disable-basicaa' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options"

---

As expected, on Windows, I get the following linker error with both LLVM_BYE_LINK_INTO_TOOLS=ON and LLVM_POLLY_LINK_INTO_TOOLS=ON:

[1/1] Linking CXX executable bin\clang.exe
FAILED: bin/clang.exe
cmd.exe /C "cd . && "C:\Program Files\CMake\bin\cmake.exe" -E vs_link_exe --intdir=tools\clang\tools\driver\CMakeFiles\clang.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100177~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100177~1.0\x64\mt.exe --manifests  -- C:\PROGRA~2\MICROS~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\clang.rsp  /out:bin\clang.exe /implib:lib\clang.lib /pdb:bin\clang.pdb /version:0.0  /machine:x64 /STACK:10000000 /INCREMENTAL:NO /subsystem:console  && cmd.exe /C "cd /D C:\Users\meinersbur\build\llvm\release\tools\clang\tools\driver && "C:\Program Files\CMake\bin\cmake.exe" -E copy C:/Users/meinersbur/build/llvm/release/bin/clang.exe C:/Users/meinersbur/build/llvm/release/./bin/clang++.exe && cd /D C:\Users\meinersbur\build\llvm\release\tools\clang\tools\driver && "C:\Program Files\CMake\bin\cmake.exe" -E copy C:/Users/meinersbur/build/llvm/release/bin/clang.exe C:/Users/meinersbur/build/llvm/release/./bin/clang-cl.exe && cd /D C:\Users\meinersbur\build\llvm\release\tools\clang\tools\driver && "C:\Program Files\CMake\bin\cmake.exe" -E copy C:/Users/meinersbur/build/llvm/release/bin/clang.exe C:/Users/meinersbur/build/llvm/release/./bin/clang-cpp.exe""
LINK: command "C:\PROGRA~2\MICROS~1\2017\COMMUN~1\VC\Tools\MSVC\1416~1.270\bin\Hostx64\x64\link.exe /nologo @CMakeFiles\clang.rsp /out:bin\clang.exe /implib:lib\clang.lib /pdb:bin\clang.pdb /version:0.0 /machine:x64 /STACK:10000000 /INCREMENTAL:NO /subsystem:console /MANIFEST /MANIFESTFILE:bin\clang.exe.manifest" failed (exit code 1169) with the following output:
Bye.lib(Bye.cpp.obj) : error LNK2005: llvmGetPassPluginInfo already defined in Polly.lib(RegisterPasses.cpp.obj)
bin\clang.exe : fatal error LNK1169: one or more multiply defined symbols found
ninja: build stopped: subcommand failed.

llvm/cmake/modules/AddLLVM.cmake

856–858 ↗

(On Diff #210273)

[nit] Please remove commented code entirely

llvm/examples/Bye/Bye.cpp

14 ↗

(On Diff #210273)

Could you add a test that verifies that the Bye test has been loaded and is working?

[serge-sans-paille]

• Test extension point registration when extension is built-in
• Correctly handle -DLLVM_LINK_LLVM_DYLIB=0
• Fix a few details

[serge-sans-paille]

@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.

[serge-sans-paille]

Make validation more resilient depending on shared/static build.

[Meinersbur]

Did you resolve the conflicting llvmGetPassPluginInfo symbols for windows?

[serge-sans-paille]

@Meinersbur nop, forgot that one, I'll have a look, thanks for pointing that out.

[serge-sans-paille]

Also fix symbol redefinition during static builds on Windows

[serge-sans-paille]

@Meinersbur up!

[Meinersbur]

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 linked library is the cleaner solution than relying on linker cleverness. If unused by design, there is no reason why the symbol should be even there. Can you convince me otherwise?

llvm/include/llvm/Support/Compiler.h

162 ↗

(On Diff #213176)

With __declspec(selectany), the FIXME should be removed.

However, I think this does not exactly match __attribute__((__weak__)): Weak symbols can still be undefined whereas selectany, if I understand MS's docs correctly, requires one valid symbol during the linking phase (Well, we currently don't support LLVM dlls anyway). This might be what you want for this patch, but classically I'd expect a weak symbol as a symbol that can resolve to NULL.

Also, I am not sure mingw/cygwin support it (I'd try out once I can apply the patch again).

[serge-sans-paille]

@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 LLVM_${name_upper}_LINK_INTO_TOOLS)

is used to only provide the symbol if we're in shared library mode.

[serge-sans-paille]

Up :-)

[serge-sans-paille]

@Meinersbur any feedback on this update?

[andwar]

llvm/examples/Bye/Bye.cpp

53 ↗

(On Diff #217604)

[nit] Empty line

llvm/examples/Bye/CMakeLists.txt

9 ↗

(On Diff #217604)

Are all these libraries required? I tried building on Darwin and for me LLVMSupport, LLVMCore and LLVMipo were sufficient.

28 ↗

(On Diff #217604)

[nit] Empty line

[serge-sans-paille]

Updates:

• fix typo in documentation
• take into account @andwar advices
• improve shared/static build automation

[Meinersbur]

llvm/test/Feature/load_extension.ll

1 ↗

(On Diff #221321)

It would be preferable to have a "REQUIRES" line that checks that the Bye pass has been linked-in dynamically.

Alternatively, use the lit.cfg to expend to the required -load= argument if required, so this test checks with and without LLVM_BYE_LOAD_INTO_TOOLS

2 ↗

(On Diff #221321)

[serious] Use FileCheck

llvm/test/lit.cfg.py

193–196 ↗

(On Diff #221321)

[serious] This kind of shell expansion does not work on Windows:

$ "c:\users\meinersbur\build\llvm\release\bin\filecheck.exe" "C:\Users\meinersbur\src\llvm\test\Other\new-pm-thinlto-defaults.ll" "--check-prefixes=CHECK-O,CHECK-O1,CHECK-POSTLINK-O,${LLVM_CHECK_EXT},CHECK-POSTLINK-O1"
# command stderr:
Supplied check-prefix is invalid! Prefixes must be unique and start with a letter and contain only alphanumeric characters, hyphens and underscores

error: command failed with exit status: 2

Polly 'solves' this by adding itself to the pass manager only when another command line flag is added (-polly) which would be less intrusive.

llvm/test/lit.site.cfg.py.in

49 ↗

(On Diff #221321)

See https://cmake.org/cmake/help/latest/command/if.html for which values cmake considers true-ish.

polly/lib/Support/RegisterPasses.cpp

726 ↗

(On Diff #221321)

[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

[Meinersbur]

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).

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

[Meinersbur]

polly/lib/Support/RegisterPasses.cpp

726 ↗

(On Diff #221321)

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.

[serge-sans-paille]

First rework pass, sevral comments have been taken into account but not all of them.

[serge-sans-paille]

Take into account most of the reviews.

[serge-sans-paille]

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 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.

polly/lib/Support/RegisterPasses.cpp

726 ↗

(On Diff #221321)

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.

[serge-sans-paille]

@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.

[Meinersbur]

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 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.

llvm/docs/WritingAnLLVMPass.rst

1200 ↗

(On Diff #230415)

[typo] when sets to

llvm/test/Feature/load_extension.ll

1–2 ↗

(On Diff #230415)

[suggestion] The established pattern is to pipe the output to FileCheck without using a temporary file.

llvm/tools/bugpoint/bugpoint.cpp

230–232 ↗

(On Diff #230415)

Nice!

polly/include/polly/RegisterPasses.h

26 ↗

(On Diff #230415)

[style] Start function names with lower case letters

polly/lib/Plugin/Polly.cpp

17–30 ↗

(On Diff #230415)

[serious] Since opt, ... etc. don't call initializePollyPasses directly anymore, I think the static initializer has to moved to RegisterPasses.cpp.

[serge-sans-paille]

Handle all @Meinersbur comment, rebuild on github with extra flags pending.

[Alouest]

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 included the pass, the error logs and some files to reproduce the bug (hello2.bc is supposed to be placed in the directory where you start your code).
It seems that the bug is coming from StringMap.cpp, it looks like NumBuckets is not always initialized when accessed, raising a SegFault.
In my example, this function is called by Module::getFunction (StringRef).

I'm a beginner in LLVM so I might just be doing dumb things.

test-df7efc.c151 KBDownload

Bye.cpp5 KBDownload

test.c125 BDownload

test-df7efc.sh1 KBDownload

hello2.bc4 KBDownload

test.ll2 KBDownload

log6 KBDownload

[serge-sans-paille]

@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.

The registration point you're using for the legacy passmanager is specific to Function passes, the one you should be using is EP_ModuleOptimizerEarly

static llvm::RegisterStandardPasses RegisterBye(
    llvm::PassManagerBuilder::EP_ModuleOptimizerEarly,
    [](const llvm::PassManagerBuilder &Builder,
       llvm::legacy::PassManagerBase &PM) {
    PM.add(new LegacyBye());
    });

[serge-sans-paille]

@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?

[Meinersbur]

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
Any hint?

I mentioned this already here: https://groups.google.com/d/msg/polly-dev/vxumPMhrSEs/uE7OfPojCwAJ

Polly uses llvm::Linker, which is not used in opt and hence thrown out when statically linking the opt executable. Try -DLLVM_EXPORT_SYMBOLS_FOR_PLUGINS=ON.

Note this never worked before, so you don't need to bother for this patch. It's what I think is a flaw in the plugin system with statically linked executables.

[serge-sans-paille]

@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  -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_POLLY_LINK_INTO_TOOLS=ON

and also on dynamic builds, with the following config, except on windows:

-DLLVM_ENABLE_PROJECTS='polly;clang' -DPOLLY_ENABLE_GPGPU_CODEGEN=OFF -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_POLLY_LINK_INTO_TOOLS=OFF

Is there any other setup you'd like to be added to the github action validation?

[Meinersbur]

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 IMHO this is good to go.

[Meinersbur]

Btw, this patch did not apply cleanly on trunk. I instead test the version from your repository.

[serge-sans-paille]

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!

[thakis]

llvm/test/lit.site.cfg.py.in

6 ↗

(On Diff #235879)

FYI, this is a now-discouraged pattern. It's better to use llvm_canonicalize_cmake_booleans in the cmake file instead. See https://reviews.llvm.org/D56912 for an example.

[serge-sans-paille]

This comment has been deleted.

llvm/test/lit.site.cfg.py.in

6 ↗

(On Diff #235879)

Sure! Thanks for pointing at this, I'll fix that once the builds are back to normal.

[sdmitriev]

llvm/test/Other/opt-O2-pipeline.ll

1 ↗

(On Diff #235879)

Looks like there is a redundant '-' at the end of the check prefix that need to be removed (CHECK- => CHECK). And opt-O3-pipeline.ll and opt-Os-pipeline.ll tests also have this issue.

[fhahn]

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.

[svenvh]

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"

  but this file does not exist.  Possible reasons include:

  * The file was deleted, renamed, or moved to another location.

  * An install or uninstall procedure did not complete successfully.

  * The installation package was faulty and contained

     "/usr/lib/llvm-10/lib/cmake/llvm/LLVMExports.cmake"

  but not all the files it references.

Call Stack (most recent call first):

  /usr/lib/llvm-10/cmake/LLVMConfig.cmake:260 (include)

  CMakeLists.txt:34 (find_package)

-- Configuring incomplete, errors occurred!

For reference: we saw a similar issue with D69416.

[serge-sans-paille]

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 (?)

[efriedma]

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?

CMakeLists.txt

set(LLVM_USE_SANITIZER "" CACHE STRING
   "Define the sanitizer used to build binaries and tests.")
 option(LLVM_OPTIMIZE_SANITIZED_BUILDS "Pass -O1 on debug sanitizer builds" ON)
 set(LLVM_LIB_FUZZING_ENGINE "" CACHE PATH
   "Path to fuzzing library for linking with fuzz targets")
 
 option(LLVM_USE_SPLIT_DWARF
   "Use -gsplit-dwarf when compiling llvm." OFF)
 
-option(LLVM_POLLY_LINK_INTO_TOOLS "Statically link Polly into tools (if available)" ON)
-option(LLVM_POLLY_BUILD "Build LLVM with Polly" ON)
-
-if (EXISTS ${LLVM_MAIN_SRC_DIR}/tools/polly/CMakeLists.txt)
-  set(POLLY_IN_TREE TRUE)
-elseif(LLVM_EXTERNAL_POLLY_SOURCE_DIR)
-  set(POLLY_IN_TREE TRUE)
-else()
-  set(POLLY_IN_TREE FALSE)
-endif()
-
-if (LLVM_POLLY_BUILD AND POLLY_IN_TREE)
-  set(WITH_POLLY ON)
-else()
-  set(WITH_POLLY OFF)
-endif()
-
-if (LLVM_POLLY_LINK_INTO_TOOLS AND WITH_POLLY)
-  set(LINK_POLLY_INTO_TOOLS ON)
-else()
-  set(LINK_POLLY_INTO_TOOLS OFF)
-endif()
-
 # Define an option controlling whether we should build for 32-bit on 64-bit
 # platforms, where supported.
 if( CMAKE_SIZEOF_VOID_P EQUAL 8 AND NOT WIN32 )
   # TODO: support other platforms and toolchains.
   option(LLVM_BUILD_32_BITS "Build 32 bits executables and libraries." OFF)
 endif()
 
 # Define the default arguments to use with 'lit', and an option for the user to
 # override.
 set(LIT_ARGS_DEFAULT "-sv")
 if (MSVC_IDE OR XCODE)
   set(LIT_ARGS_DEFAULT "${LIT_ARGS_DEFAULT} --no-progress-bar")
 endif()
 set(LLVM_LIT_ARGS "${LIT_ARGS_DEFAULT}" CACHE STRING "Default options for lit")
if (LLVM_INCLUDE_BENCHMARKS)
   set(BENCHMARK_DOWNLOAD_DEPENDENCIES OFF CACHE BOOL "Don't download dependencies" FORCE)
   set(BENCHMARK_ENABLE_GTEST_TESTS OFF CACHE BOOL "Disable Google Test in benchmark" FORCE)
   # Since LLVM requires C++11 it is safe to assume that std::regex is available.
   set(HAVE_STD_REGEX ON CACHE BOOL "OK" FORCE)
 
   add_subdirectory(utils/benchmark)
   add_subdirectory(benchmarks)
 endif()
+
+# generate X Macro file for extension handling. It provides a
+# HANDLE_EXTENSION(extension_namespace, ExtensionProject) call for each extension
+# allowing client code to define HANDLE_EXTENSION to have a specific code be run for
+# each extension.
+get_property(LLVM_EXTENSIONS GLOBAL PROPERTY LLVM_EXTENSIONS)
+file(WRITE "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def.tmp" "//extension handlers\n")
+foreach(llvm_extension ${LLVM_EXTENSIONS})
+    string(TOLOWER ${llvm_extension} llvm_extension_namespace)
+
+    string(TOUPPER ${llvm_extension} llvm_extension_upper)
+    string(SUBSTRING ${llvm_extension_upper} 0 1 llvm_extension_upper_first)
+    string(SUBSTRING ${llvm_extension_namespace} 1 -1 llvm_extension_lower_tail)
+    string(CONCAT llvm_extension_project ${llvm_extension_upper_first} ${llvm_extension_lower_tail})
+
+    file(APPEND "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def.tmp" "HANDLE_EXTENSION(${llvm_extension_namespace}, ${llvm_extension_project})\n")
+endforeach()
+file(APPEND "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def.tmp" "#undef HANDLE_EXTENSION\n")
+
+# only replace if there's an actual change
+execute_process(COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def.tmp" "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def")
+file(REMOVE "${CMAKE_BINARY_DIR}/include/llvm/Support/Extension.def.tmp")
+
+

cmake/modules/AddLLVM.cmake

if (LLVM_PTHREAD_LIB)
     # executable must be linked with it in order to provide consistent
     # API for all shared libaries loaded by this executable.
     target_link_libraries(${name} PRIVATE ${LLVM_PTHREAD_LIB})
   endif()
 
   llvm_codesign(${name} ENTITLEMENTS ${ARG_ENTITLEMENTS})
 endmacro(add_llvm_executable name)
 
+# register_llvm_extension(name)
+#   Register ${name} as an llvm compiler extension.
+#   A compiler extension must define three functions in namespace ${name}
+#   - void initialize${Name}Passes(llvm::PassRegistry &Registry) that calls the initializer of dependant passes/ analyse
+#   - void register${Name}Passes(llvm::legacy::PassManagerBase &PM) that registers all the extension passes to `opt`, legacy PM
+#   - void Register${Name}Passes(llvm::PassBuilder &PB) that registers all the extension passes to `opt`, new PM
+
+function(register_llvm_extension name)
+    message(STATUS "Registering ${name} as a compiler extension")
+    set(LLVM_ALL_EXTENSION_TARGETS clang;bugpoint;opt)
+
+    set_property(GLOBAL APPEND PROPERTY LLVM_EXTENSIONS ${name})
+    foreach(tool ${LLVM_ALL_EXTENSION_TARGETS})
+        if(TARGET ${tool})
+            set_property(TARGET ${tool} APPEND PROPERTY LLVM_COMPILER_EXTENSIONS ${name})
+        endif()
+    endforeach()
+endfunction(register_llvm_extension)
+
+
 function(export_executable_symbols target)
   if (LLVM_EXPORTED_SYMBOL_FILE)
     # The symbol file should contain the symbols we want the executable to
     # export
     set_target_properties(${target} PROPERTIES ENABLE_EXPORTS 1)
   elseif (LLVM_EXPORT_SYMBOLS_FOR_PLUGINS)
     # Extract the symbols to export from the static libraries that the
     # executable links against.

include/llvm/Config/llvm-config.h.cmake

/* This file enumerates variables from the LLVM configuration so that they
    This is a C header that can be included in the llvm-c headers. */
 
 #ifndef LLVM_CONFIG_H
 #define LLVM_CONFIG_H
 
 /* Define if LLVM_ENABLE_DUMP is enabled */
 #cmakedefine LLVM_ENABLE_DUMP
 
-/* Define if we link Polly to the tools */
-#cmakedefine LINK_POLLY_INTO_TOOLS
-
 /* Target triple LLVM will generate code for by default */
 #cmakedefine LLVM_DEFAULT_TARGET_TRIPLE "${LLVM_DEFAULT_TARGET_TRIPLE}"
 
 /* Define if threads enabled */
 #cmakedefine01 LLVM_ENABLE_THREADS
 
 /* Has gcc/MSVC atomic intrinsics */
 #cmakedefine01 LLVM_HAS_ATOMICS

tools/CMakeLists.txt

 # This file will recurse into all subdirectories that contain CMakeLists.txt
 # Setting variables that match the pattern LLVM_TOOL_{NAME}_BUILD to Off will
 # prevent traversing into a directory.
 #
 # The only tools that need to be explicitly added are ones that have explicit
 # ordering requirements.
 
 # Iterates all the subdirectories to create CMake options to enable/disable
 # traversing each directory.
 create_llvm_tool_options()
 
-# Build polly before the tools: the tools link against polly when
-# LINK_POLLY_INTO_TOOLS is set.
-if(WITH_POLLY)
-  add_llvm_external_project(polly)
-else()
-  set(LLVM_TOOL_POLLY_BUILD Off)
-endif()
-
 if(NOT LLVM_BUILD_LLVM_DYLIB AND NOT LLVM_BUILD_LLVM_C_DYLIB)
   set(LLVM_TOOL_LLVM_SHLIB_BUILD Off)
 endif()
 
 if(NOT LLVM_USE_INTEL_JITEVENTS )
   set(LLVM_TOOL_LLVM_JITLISTENER_BUILD Off)
 endif()
 
 if(CYGWIN OR NOT LLVM_ENABLE_PIC)

 
 # Projects supported via LLVM_EXTERNAL_*_SOURCE_DIR need to be explicitly
 # specified.
 add_llvm_external_project(clang)
 add_llvm_external_project(llgo)
 add_llvm_external_project(lld)
 add_llvm_external_project(lldb)
 
+# projects sensible to extension property (namely clang, opt and bugpoint)
+# must be declared before extensions
+add_llvm_external_project(opt)
+add_llvm_external_project(bugpoint)
+
 # Automatically add remaining sub-directories containing a 'CMakeLists.txt'
 # file as external projects.
 add_llvm_implicit_projects()
 
 # Add subprojects specified using LLVM_EXTERNAL_PROJECTS
 foreach(p ${LLVM_EXTERNAL_PROJECTS})
   add_llvm_external_project(${p})
 endforeach(p)
 
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS} PARENT_SCOPE)

tools/bugpoint/CMakeLists.txt

add_llvm_tool(bugpoint
   OptimizerDriver.cpp
   ToolRunner.cpp
   bugpoint.cpp
 
   DEPENDS
   intrinsics_gen
   )
 export_executable_symbols(bugpoint)
-
-if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
-  target_link_libraries(bugpoint PRIVATE Polly)
-  # Ensure LLVMTarget can resolve dependences in Polly.
-  target_link_libraries(bugpoint PRIVATE LLVMTarget)
-endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
+target_link_libraries(bugpoint PRIVATE $<TARGET_PROPERTY:LLVM_COMPILER_EXTENSIONS>)

tools/bugpoint/bugpoint.cpp

public:
   void add(Pass *P) override {
     const void *ID = P->getPassID();
     const PassInfo *PI = PassRegistry::getPassRegistry()->getPassInfo(ID);
     D.addPass(PI->getPassArgument());
   }
 };
 }
 
-#ifdef LINK_POLLY_INTO_TOOLS
-namespace polly {
-void initializePollyPasses(llvm::PassRegistry &Registry);
-}
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) namespace Ns { void initialize##Ext##Passes(llvm::PassRegistry &Registry); }
+#include "llvm/Support/Extension.def"
 
 int main(int argc, char **argv) {
 #ifndef DEBUG_BUGPOINT
   InitLLVM X(argc, argv);
 #endif
 
   // Initialize passes
   PassRegistry &Registry = *PassRegistry::getPassRegistry();
   initializeCore(Registry);
   initializeScalarOpts(Registry);
   initializeObjCARCOpts(Registry);
   initializeVectorization(Registry);
   initializeIPO(Registry);
   initializeAnalysis(Registry);
   initializeTransformUtils(Registry);
   initializeInstCombine(Registry);
   initializeAggressiveInstCombine(Registry);
   initializeInstrumentation(Registry);
   initializeTarget(Registry);
 
-#ifdef LINK_POLLY_INTO_TOOLS
-  polly::initializePollyPasses(Registry);
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) Ns::initialize##Ext##Passes(Registry);
+#include "llvm/Support/Extension.def"
 
   if (std::getenv("bar") == (char*) -1) {
     InitializeAllTargets();
     InitializeAllTargetMCs();
     InitializeAllAsmPrinters();
     InitializeAllAsmParsers();
   }
 

tools/clang/tools/driver/CMakeLists.txt

if(CLANG_ORDER_FILE AND
   if("${ORDER_FILE}" STREQUAL "\n")
     set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${CLANG_ORDER_FILE})
   elseif(LINKER_ORDER_FILE_WORKS)
     target_link_libraries(clang PRIVATE ${LINKER_ORDER_FILE_OPTION})
     set_target_properties(clang PROPERTIES LINK_DEPENDS ${CLANG_ORDER_FILE})
   endif()
 endif()
 
-if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
-  target_link_libraries(clang PRIVATE Polly)
-endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
+target_link_libraries(clang PRIVATE $<TARGET_PROPERTY:LLVM_COMPILER_EXTENSIONS>)

tools/clang/tools/driver/cc1_main.cpp

static void LLVMErrorHandler(void *UserData, const std::string &Message,
   llvm::sys::RunInterruptHandlers();
 
   // We cannot recover from llvm errors.  When reporting a fatal error, exit
   // with status 70 to generate crash diagnostics.  For BSD systems this is
   // defined as an internal software error.  Otherwise, exit with status 1.
   exit(GenCrashDiag ? 70 : 1);
 }
 
-#ifdef LINK_POLLY_INTO_TOOLS
-namespace polly {
-void initializePollyPasses(llvm::PassRegistry &Registry);
-}
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) namespace Ns { void initialize##Ext##Passes(llvm::PassRegistry &Registry); }
+#include "llvm/Support/Extension.def"
 
 #ifdef CLANG_HAVE_RLIMITS
 #if defined(__linux__) && defined(__PIE__)
 static size_t getCurrentStackAllocation() {
   // If we can't compute the current stack usage, allow for 512K of command
   // line arguments and environment.
   size_t Usage = 512 * 1024;
   if (FILE *StatFile = fopen("/proc/self/stat", "r")) {
int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
   PCHOps->registerReader(llvm::make_unique<ObjectFilePCHContainerReader>());
 
   // Initialize targets first, so that --version shows registered targets.
   llvm::InitializeAllTargets();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmPrinters();
   llvm::InitializeAllAsmParsers();
 
-#ifdef LINK_POLLY_INTO_TOOLS
-  llvm::PassRegistry &Registry = *llvm::PassRegistry::getPassRegistry();
-  polly::initializePollyPasses(Registry);
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) { llvm::PassRegistry &Registry = *llvm::PassRegistry::getPassRegistry(); Ns::initialize##Ext##Passes(Registry); }
+#include "llvm/Support/Extension.def"
 
   // Buffer diagnostics from argument parsing so that we can output them using a
   // well formed diagnostic object.
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts = new DiagnosticOptions();
   TextDiagnosticBuffer *DiagsBuffer = new TextDiagnosticBuffer;
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
   bool Success = CompilerInvocation::CreateFromArgs(
       Clang->getInvocation(), Argv.begin(), Argv.end(), Diags);

tools/opt/CMakeLists.txt

add_llvm_tool(opt
   PassPrinters.cpp
   PrintSCC.cpp
   opt.cpp
 
   DEPENDS
   intrinsics_gen
   )
 export_executable_symbols(opt)
-
-if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
-  target_link_libraries(opt PRIVATE Polly)
-endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
+target_link_libraries(opt PRIVATE $<TARGET_PROPERTY:LLVM_COMPILER_EXTENSIONS>)

tools/opt/NewPMDriver.cpp

PB.registerOptimizerLastEPCallback(
         [&PB, VerifyEachPass, DebugLogging](FunctionPassManager &PM,
                                             PassBuilder::OptimizationLevel) {
           ExitOnError Err("Unable to parse OptimizerLastEP pipeline: ");
           Err(PB.parsePassPipeline(PM, OptimizerLastEPPipeline, VerifyEachPass,
                                    DebugLogging));
         });
 }
 
-#ifdef LINK_POLLY_INTO_TOOLS
-namespace polly {
-void RegisterPollyPasses(PassBuilder &);
-}
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) namespace Ns { void Register##Ext##Passes(PassBuilder &); }
+#include "llvm/Support/Extension.def"
 
 bool llvm::runPassPipeline(StringRef Arg0, Module &M, TargetMachine *TM,
                            ToolOutputFile *Out, ToolOutputFile *ThinLTOLinkOut,
                            ToolOutputFile *OptRemarkFile,
                            StringRef PassPipeline, OutputKind OK,
                            VerifierKind VK,
                            bool ShouldPreserveAssemblyUseListOrder,
                            bool ShouldPreserveBitcodeUseListOrder,
PB.registerPipelineParsingCallback(
           return true;
         } else if (Name == "check-debugify") {
           MPM.addPass(NewPMCheckDebugifyPass());
           return true;
         }
         return false;
       });
 
-#ifdef LINK_POLLY_INTO_TOOLS
-  polly::RegisterPollyPasses(PB);
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) Ns::Register##Ext##Passes(PB);
+#include "llvm/Support/Extension.def"
 
   // Specially handle the alias analysis manager so that we can register
   // a custom pipeline of AA passes with it.
   AAManager AA;
   if (auto Err = PB.parseAAPipeline(AA, AAPipeline)) {
     errs() << Arg0 << ": " << toString(std::move(Err)) << "\n";
     return false;
   }

tools/opt/opt.cpp

if (!TheTarget) {
     return nullptr;
   }
 
   return TheTarget->createTargetMachine(TheTriple.getTriple(), CPUStr,
                                         FeaturesStr, Options, getRelocModel(),
                                         getCodeModel(), GetCodeGenOptLevel());
 }
 
-#ifdef LINK_POLLY_INTO_TOOLS
-namespace polly {
-void initializePollyPasses(llvm::PassRegistry &Registry);
-}
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) namespace Ns { void initialize##Ext##Passes(llvm::PassRegistry &Registry); }
+#include "llvm/Support/Extension.def"
 
 //===----------------------------------------------------------------------===//
 // main for opt
 //
 int main(int argc, char **argv) {
   InitLLVM X(argc, argv);
 
   // Enable debug stream buffering.
int main(int argc, char **argv) {
   initializeInterleavedAccessPass(Registry);
   initializeEntryExitInstrumenterPass(Registry);
   initializePostInlineEntryExitInstrumenterPass(Registry);
   initializeUnreachableBlockElimLegacyPassPass(Registry);
   initializeExpandReductionsPass(Registry);
   initializeWasmEHPreparePass(Registry);
   initializeWriteBitcodePassPass(Registry);
 
-#ifdef LINK_POLLY_INTO_TOOLS
-  polly::initializePollyPasses(Registry);
-#endif
+#define HANDLE_EXTENSION(Ns, Ext) Ns::initialize##Ext##Passes(Registry);
+#include "llvm/Support/Extension.def"
 
   cl::ParseCommandLineOptions(argc, argv,
     "llvm .bc -> .bc modular optimizer and analysis printer\n");
 
   if (AnalyzeOnly && NoOutput) {
     errs() << argv[0] << ": analyze mode conflicts with no-output mode.\n";
     return 1;
   }

tools/polly/CMakeLists.txt

 endforeach ()
 
 add_custom_target(polly-check-format DEPENDS ${check_format_depends})
 set_target_properties(polly-check-format PROPERTIES FOLDER "Polly")
 
 add_custom_target(polly-update-format DEPENDS ${update_format_depends})
 set_target_properties(polly-update-format PROPERTIES FOLDER "Polly")
 
+register_llvm_extension(polly)
+
+# Polly-ACC requires the NVPTX target to be present in the executable it is linked to
+set_property(TARGET bugpoint APPEND PROPERTY LLVM_COMPILER_EXTENSIONS LLVMTarget)

utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn

write_cmake_config("config") {
   }
 }
 
 write_cmake_config("llvm-config") {
   input = "llvm-config.h.cmake"
   output = "$target_gen_dir/llvm-config.h"
   values = [
     "LLVM_ENABLE_DUMP=",
-    "LINK_POLLY_INTO_TOOLS=",
     "LLVM_DEFAULT_TARGET_TRIPLE=$llvm_target_triple",
     "LLVM_HAS_ATOMICS=1",
     "LLVM_HOST_TRIPLE=$llvm_current_triple",
     "LLVM_NATIVE_ARCH=$native_target",
     "LLVM_NATIVE_ASMPARSER=1",
     "LLVM_NATIVE_ASMPRINTER=1",
     "LLVM_NATIVE_DISASSEMBLER=1",
     "LLVM_NATIVE_TARGET=1",