[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.
DiamondLovesYou created this revision. DiamondLovesYou added a reviewer: beanz. Herald added subscribers: cfe-commits, mgorny. Herald added a reviewer: bollu. clang doesn't need to link Polly when built with `LLVM_LINK_LLVM_DYLIB`. Repository: rC Clang https://reviews.llvm.org/D51986 Files: tools/driver/CMakeLists.txt Index: tools/driver/CMakeLists.txt === --- tools/driver/CMakeLists.txt +++ tools/driver/CMakeLists.txt @@ -122,6 +122,6 @@ endif() endif() -if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) +if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS AND NOT LLVM_LINK_LLVM_DYLIB) target_link_libraries(clang PRIVATE Polly) endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) Index: tools/driver/CMakeLists.txt === --- tools/driver/CMakeLists.txt +++ tools/driver/CMakeLists.txt @@ -122,6 +122,6 @@ endif() endif() -if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) +if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS AND NOT LLVM_LINK_LLVM_DYLIB) target_link_libraries(clang PRIVATE Polly) endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.
DiamondLovesYou updated this revision to Diff 165090. DiamondLovesYou added a comment. - Fix cmake warning Repository: rC Clang https://reviews.llvm.org/D51986 Files: tools/driver/CMakeLists.txt Index: tools/driver/CMakeLists.txt === --- tools/driver/CMakeLists.txt +++ tools/driver/CMakeLists.txt @@ -122,6 +122,6 @@ endif() endif() -if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) +if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS AND NOT LLVM_LINK_LLVM_DYLIB) target_link_libraries(clang PRIVATE Polly) -endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) +endif() Index: tools/driver/CMakeLists.txt === --- tools/driver/CMakeLists.txt +++ tools/driver/CMakeLists.txt @@ -122,6 +122,6 @@ endif() endif() -if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) +if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS AND NOT LLVM_LINK_LLVM_DYLIB) target_link_libraries(clang PRIVATE Polly) -endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS) +endif() ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.
DiamondLovesYou added a comment.
In https://reviews.llvm.org/D51986#1232320, @beanz wrote:
> I don’t think this is the right solution. The build system knows what
> components are in the dylib and should remove them from the list of libraries
> linked individually. You should be able to make Polly behave like an LLVM
> component, then tools don’t need to care if the dylib is used or not.
That's not true if `target_link_libraries(${target} _whatever_ ${libs})` is
used. That function will pull in all of each of `${libs}`' dependencies, which
causes problems because then, eg, `bugpoint` will then be linked with
`libLLVM.so` (as expected) AND `libPolly.a` (assuming `BUILD_SHARED_MODULES` is
false) AND then a bunch of LLVM components. The LLVM components (including
Polly) will then conflict with `libLLVM.so`.
Polly already enjoys status as an LLVM component. No effort was necessary to
include Polly in `libLLVM.so` for example.
Repository:
rC Clang
https://reviews.llvm.org/D51986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.
DiamondLovesYou added a comment.
In https://reviews.llvm.org/D51986#1232419, @DiamondLovesYou wrote:
> In https://reviews.llvm.org/D51986#1232320, @beanz wrote:
>
> > I don’t think this is the right solution. The build system knows what
> > components are in the dylib and should remove them from the list of
> > libraries linked individually. You should be able to make Polly behave like
> > an LLVM component, then tools don’t need to care if the dylib is used or
> > not.
>
>
> That's not true if `target_link_libraries(${target} _whatever_ ${libs})` is
> used. That function will pull in all of each of `${libs}`' dependencies,
> which causes problems because then, eg, `bugpoint` will then be linked with
> `libLLVM.so` (as expected) AND `libPolly.a` (assuming `BUILD_SHARED_MODULES`
> is false) AND then a bunch of LLVM components. The LLVM components (including
> Polly) will then conflict with `libLLVM.so`.
>
> Polly already enjoys status as an LLVM component. No effort was necessary to
> include Polly in `libLLVM.so` for example.
To add: is there a why to specify optional (as in, link if present) components?
Repository:
rC Clang
https://reviews.llvm.org/D51986
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.
DiamondLovesYou added a comment. In https://reviews.llvm.org/D51986#1232440, @beanz wrote: > After line 18 in this file you could do something like: > > if(WITH_POLLY) > list(APPEND LLVM_LINK_COMPONENTS Polly) > endif() > > > Then you can get rid of the `target_link_libraries` call. Ah, indeed, thanks. I'll update the other patch too. Repository: rC Clang https://reviews.llvm.org/D51986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.
DiamondLovesYou added a comment. In https://reviews.llvm.org/D51986#1232440, @beanz wrote: > After line 18 in this file you could do something like: > > if(WITH_POLLY) > list(APPEND LLVM_LINK_COMPONENTS Polly) > endif() > > > Then you can get rid of the `target_link_libraries` call. It turns out this can't be done. Consider the `LLVMLTO` component and its loadable form, `LTO`. Contrast this with Polly: `Polly` is the component and `LLVMPolly` is the loadable module. This naming convention disagreement prevents Polly's use as a component while also not breaking LTO, I've found (Chris, is this what you were referring to?). Specifically, CMake fails because executables can't link to a `MODULE_LIBRARY` target when trying to link `LLVMPolly`. I think the Polly naming scheme should be changed to match LTO's scheme, rather than vice versa, but not my call. Repository: rC Clang https://reviews.llvm.org/D51986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
