[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.

2018-09-12 Thread Richard Diamond via Phabricator via cfe-commits
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


[PATCH] D51986: Fixes for `LLVM_LINK_LLVM_DYLIB` && Polly.

2018-09-12 Thread Richard Diamond via Phabricator via cfe-commits
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.

2018-09-12 Thread Richard Diamond via Phabricator via cfe-commits
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.

2018-09-12 Thread Richard Diamond via Phabricator via cfe-commits
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.

2018-09-12 Thread Richard Diamond via Phabricator via cfe-commits
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.

2018-09-12 Thread Richard Diamond via Phabricator via cfe-commits
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