- Update documentation now that the move to monorepo has been made
- Fix validation issue when building examples
- Add NO_MODULE option to match Polly specific requirements (i.e. building the module *and* linking it statically)
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I've tested that with LLVM_BYE_LINK_INTO_TOOLS ON/OFF, LLVM_BUILD_EXAMPLES ON/OFF LLVM_POLLY_LINK_INTO_TOOLS ON/OFF and it seems okay.
- Test and handle the combination of LLVM_BUILD_EXAMPLE=OFF + LLVM_BYE_LINK_INTO_TOOLS=ON
llvm/cmake/modules/AddLLVM.cmake | ||
---|---|---|
850–853 | [doc] Document the NO_MODULE option | |
865 | [nit] indention | |
llvm/examples/Bye/CMakeLists.txt | ||
1 | [serious] Whether example targets are generated is controlled by LLVM_INCLUDE_EXAMPLES. LLVM_BUILD_EXAMPLES should control EXCLUDE_FROM_ALL or similar. Is there a corresponding option for pass plugins, especially if LLVM_BYE_LINK_INTO_TOOLS is off? I'd assume that if LLVM_BUILD_EXAMPLES is off, but LLVM_BYE_LINK_INTO_TOOLS is on, that latter would effectively override EXCLUDE_FROM_ALL since the plugin is needed to built opt,bugpoint,etc. Btw, what is the "Fix validation issue when building examples"? |
Now it should, but I'm waiting for my pre-commit validation to state if that's actually the case.
llvm/examples/Bye/CMakeLists.txt | ||
---|---|---|
1 |
The problem I wanted to solve here is having LLVM_BYE_LINK_INTO_TOOLS=ON in release build, because Bye is just an example library. But it's nice to be able to test it. Maybe I should just issue a warning in that case?
There was a mismatch between the lit feature « requires build_example » and « LLVM_BUILD_EXAMPLES » I've updated the patch in that aspect, taking into account your remark. |
LGTM
llvm/cmake/modules/AddLLVM.cmake | ||
---|---|---|
897 | [nit] accidental whitespace change | |
llvm/examples/Bye/CMakeLists.txt | ||
9 | The message could be more specific about this is not good, something like "Release build contains example code for development". Recently we had a discussion about adding tutorial pass code to the repository, and how to include it into the build system, where D61446 came up as well. We should at some point agree how to handle example/tutorial code in release builds. [suggestion] Only print the warning in release builds, maybe by checking CMAKE_BUILD_TYPE or LLVM_ENABLE_ASSERTIONS. |
- Fix OSX build.
The problem was deeper than expected and arise when the pass extension mechanism is used from plugin *and* static library, e.g. when Polly is linked in and a plugin is loaded.
In that case, the pass extension registry may be initialized before the first shared library is opened, and thus upon llvm_shutdown, the library ar dlclosed *before* the pass extension registered is deleted. At that moment the callback registered in the pass extension registry points to a function from the dlclosed library, and bad thing happens (namely the std::function destructor segfaults).
The patch consists in forcing the init order through a call to a function that triggers the construction of the managedstatic holding thedlopened library handles.
It would be nicer if you would commit (and upload for review) the three changes separately. You might just commit the first two changes since I think they are ok, but I am not sure about the ensureValid change, which needs some expertise from someone knowing sys::DynamicLibrary and llvm::ManagedStatic.
llvm/lib/Support/DynamicLibrary.cpp | ||
---|---|---|
152 ↗ | (On Diff #237161) | [nit] clang-format would collapse double empty lines into one. |
[doc] Document the NO_MODULE option