Page MenuHomePhabricator

cmake: Specify library dependences even without BUILD_SHARED_LIBS
AbandonedPublic

Authored by grosser on Sep 10 2016, 1:19 PM.

Details

Reviewers
Meinersbur
Summary

This is necessary to make sure Polly's dependency on LLVMTarget is properly
registered. Without this change, we got link time failures in bugpoint,
which apparently does not require llvm::TargetRecip::TargetRecip, which however
is needed by Polly ACC.

Registering library dependences for Polly.a will break LLVMPolly.so, hence we
disable the build of LLVMPolly.so when using LINK_POLLY_INTO_TOOLS.

Diff Detail

Event Timeline

grosser updated this revision to Diff 70954.Sep 10 2016, 1:19 PM
grosser retitled this revision from to cmake: Specify library dependences even without BUILD_SHARED_LIBS.
grosser updated this object.
grosser added a reviewer: Meinersbur.
grosser added subscribers: llvm-commits, pollydev.
Meinersbur edited edge metadata.Sep 12 2016, 7:49 AM

In which configuration does the link error occur? I could not reproduce it.

Wouldn't it be a lot easier to target_link_libraries(opt LLVMTarget) if LINK_POLLY_INTO_TOOLS?

lib/CMakeLists.txt
69

AFAIU this gives priority of linking against libLLVM.so over linking agains the individual LLVM components. Since there is a separate option LLVM_BUILD_LLVM_DYLIB, this looks like the right thing to do, but could land in a separate commit.

(Although, I think the combination BUILD_SHARED_LIBS and LLVM_LINK_LLVM_DYLIB is not something that is supposed to work. cmake stop with the error

CMake Error at tools/llvm-shlib/CMakeLists.txt:41 (list):
  list sub-command REMOVE_DUPLICATES requires list to be present.

)

76

LINK_POLLY_INTO_TOOLS should have no effect on how the Polly components are being built. If that linking into opt/bugpoint doesn't work, doesn't it mean that the Polly libs where built incorrectly in the first place? Assuming a downstream projects links Polly into their tool (say, "mollycc"), how would they link Polly into it without LLVM_POLLY_LINK_INTO_TOOLS which might be unwanted?

I suggest to always call these target_link_libraries (except if LLVM_LINK_LLVM_DYLIB, which get priority), assuming it is only redundant to do so if not BUILD_SHARED_LIBS. If that, as you mentioned, would break LLVMPolly, we might need to split Polly into 3 libraries:

  • PollyImpl, containing all sources, but no dependencies
  • Polly, depends on PollyImpl an its library dependencies, for linking by cmake.
  • LLVMPolly, depends on PollyImpl only, for loading using -load
grosser added a comment.EditedSep 13 2016, 1:50 PM

Hi Michael,

thank you for your feedback. This error happens with:

-DBUILD_SHARED_LIBS=ON -DPOLLY_ENABLE_GPGPU_CODEGEN=ON

Wouldn't it be a lot easier to target_link_libraries(opt LLVMTarget) if LINK_POLLY_INTO_TOOLS?

Like:

--- a/tools/bugpoint/CMakeLists.txt
+++ b/tools/bugpoint/CMakeLists.txt
@@ -39,4 +39,5 @@ if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
        target_link_libraries(bugpoint ${lib})
      endforeach(lib)
    endif(POLLY_LINK_LIBS)
+   target_link_libraries(bugpoint LLVMTarget) 
   endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)

This works as well. LLVMTarget is already part of bugpoints target_link_libraries, but adding it again ensures it is added once more at the end of linker command line, such that it is available to cover symbols only used by Polly, but not by bugpoint.

If this is preferable, I could just add this line.

thank you for your feedback. This error happens with:

-DBUILD_SHARED_LIBS=ON -DPOLLY_ENABLE_GPGPU_CODEGEN=ON

Thanks.

Wouldn't it be a lot easier to target_link_libraries(opt LLVMTarget) if LINK_POLLY_INTO_TOOLS?

Like:

--- a/tools/bugpoint/CMakeLists.txt
+++ b/tools/bugpoint/CMakeLists.txt
@@ -39,4 +39,5 @@ if(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)
        target_link_libraries(bugpoint ${lib})
      endforeach(lib)
    endif(POLLY_LINK_LIBS)
+   target_link_libraries(bugpoint LLVMTarget) 
   endif(WITH_POLLY AND LINK_POLLY_INTO_TOOLS)

This works as well. LLVMTarget is already part of bugpoints target_link_libraries, but adding it again ensures it is added once more at the end of linker command line, such that it is available to cover symbols only used by Polly, but not by bugpoint.

If this is preferable, I could just add this line.

I don't see any downsides. Maybe add a comment why LLVMTarget is linked twice?

There is -Wl,--start-group -Wl,--end-group to make library order not matter anymore, but I don't know how to make CMake use it.

grosser abandoned this revision.Sep 13 2016, 8:56 PM

OK. I committed your solution as r281438.