cmake: Specify library dependences even without BUILD_SHARED_LIBS

[grosser]

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.

[Meinersbur]

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]

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.

[Meinersbur]

In D24442#541690, @grosser wrote:

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]

OK. I committed your solution as r281438.

lib/CMakeLists.txt

add_polly_library(Polly
   )
 
 if (GPU_CODEGEN)
   target_link_libraries(Polly PollyPPCG)
 endif (GPU_CODEGEN)
 
 target_link_libraries(Polly PollyISL)
 
-if (BUILD_SHARED_LIBS)
+if (LLVM_LINK_LLVM_DYLIB)
+  target_link_libraries(Polly
+    LLVM
+  )
+  link_directories(
+    ${LLVM_LIBRARY_DIR}
+  )
+elseif (BUILD_SHARED_LIBS OR LINK_POLLY_INTO_TOOLS)
   target_link_libraries(Polly
     LLVMSupport
     LLVMCore
     LLVMScalarOpts
     LLVMInstCombine
     LLVMTransformUtils
     LLVMAnalysis
     LLVMipo
     LLVMMC
 # The libraries below are required for darwin: http://PR26392
     LLVMBitReader
     LLVMMCParser
     LLVMObject
     LLVMProfileData
     LLVMTarget
     LLVMVectorize
   )
   link_directories(
     ${LLVM_LIBRARY_DIR}
   )
-elseif (LLVM_LINK_LLVM_DYLIB)
-  target_link_libraries(Polly
-    LLVM
-  )
-  link_directories(
-    ${LLVM_LIBRARY_DIR}
-  )
 endif()
 
 # Build a monolithic Polly.a and a thin module LLVMPolly.moduleext that links to
 # that static library.
+if (NOT LINK_POLLY_INTO_TOOLS)
   if (MSVC)
     # Add dummy target, because loadable modules are not supported on Windows
     add_custom_target(LLVMPolly)
     set_target_properties(LLVMPolly PROPERTIES FOLDER "Polly")
   else ()
     add_polly_loadable_module(LLVMPolly
       Polly.cpp
     )
 
     target_link_libraries(LLVMPolly Polly)
 
     set_target_properties(LLVMPolly
       PROPERTIES
       LINKER_LANGUAGE CXX
       PREFIX "")
   endif ()
+endif(NOT LINK_POLLY_INTO_TOOLS)
 
 if (TARGET intrinsics_gen)
   # Check if we are building as part of an LLVM build
   add_dependencies(Polly intrinsics_gen)
 endif()
 

test/CMakeLists.txt

 add_custom_target(check-polly)
 set_target_properties(check-polly PROPERTIES FOLDER "Polly")
 
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
 
   # We are building polly out of tree, adjust the settings.
   # FIXME: FileCheck is not available in llvm install directory at the moment.
   set(LLVM_LIT ${LLVM_INSTALL_ROOT}/bin/llvm-lit)
+  if (NOT LINK_POLLY_INTO_TOOLS)
     set(POLLY_TEST_DEPS LLVMPolly)
+  endif ()
   if (POLLY_GTEST_AVAIL)
     list(APPEND POLLY_TEST_DEPS PollyUnitTests)
   endif ()
 
   set(LLVM_BINARY_DIR "${LLVM_INSTALL_ROOT}")
   set(LLVM_TOOLS_DIR "${LLVM_INSTALL_ROOT}/bin")
   set(LLVM_LIBS_DIR "${LLVM_INSTALL_ROOT}/lib")
   set(POLLY_LIB_DIR "${POLLY_BINARY_DIR}/lib")
if (POLLY_GTEST_AVAIL)
                   COMMENT "Running Polly unit tests")
       set_target_properties(check-polly-unittests PROPERTIES FOLDER "Polly")
     endif ()
   endif()
 
 else (NOT DEFINED LLVM_MAIN_SRC_DIR)
 
   set(LLVM_LIT ${LLVM_TOOLS_BINARY_DIR}/llvm-lit)
-  set(POLLY_TEST_DEPS llvm-config opt LLVMPolly FileCheck not)
+  set(POLLY_TEST_DEPS llvm-config opt FileCheck not)
+  if (NOT LINK_POLLY_INTO_TOOLS)
+    set(POLLY_TEST_DEPS ${POLLY_TEST_DEPS} LLVMPolly)
+  endif()
   if (POLLY_GTEST_AVAIL)
     list(APPEND POLLY_TEST_DEPS PollyUnitTests)
   endif ()
 
   set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}")
   set(LLVM_TOOLS_DIR "${LLVM_TOOLS_BINARY_DIR}")
   set(LLVM_LIBS_DIR "${LLVM_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}")
   set(POLLY_LIB_DIR "${LLVM_LIBS_DIR}")