Use llvm_library_add support for components in Polly

[serge-sans-paille]

Summary

Instead of manually registering libraries as target_libraries in Polly, use the LINK_COMPONENTS option. It makes Polly's CMakefile easier to read, and generalizes https://reviews.llvm.org/D78332 to all plugins.

[efriedma]

Makes sense. LGTM

[serge-sans-paille]

Patch updated to make compiler extension compatible with LINK_COMPONENTS

[serge-sans-paille]

@sylvestre.ledru can you check this still work with your setup?

[serge-sans-paille]

Once rebased on https://reviews.llvm.org/D78192 this one should be much cleaner.

[serge-sans-paille]

Rebase this patch on https://reviews.llvm.org/D78192, which make sit much cleaner and straight-forward.

[serge-sans-paille]

@Meinersbur can you confirm this oe is okay with you?

[Meinersbur]

LGTM

[Meinersbur]

I got a new warning when compiling with clang:

In file included from /tmp/persistent/llvm-src/llvm/tools/llvm-config/llvm-config.cpp:51:
tools/llvm-config/ExtensionDependencies.inc:7:2: warning: suggest braces around initialization of subobject [-Wmissing-braces]
{"Polly", {"support", "core", "scalaropts", "instcombine", "transformutils", "analysis", "ipo", "mc", "passes", "linker", "irreader", "analysis", "bitreader", "mcparser", "object", "profiledata", "target", "vectorize", "PollyISL", "Polly",nullptr}}, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 {                                                                                                                                                                                                                                                      }

[serge-sans-paille]

In D78358#2008954, @Meinersbur wrote:

I got a new warning when compiling with clang:

In file included from /tmp/persistent/llvm-src/llvm/tools/llvm-config/llvm-config.cpp:51:
tools/llvm-config/ExtensionDependencies.inc:7:2: warning: suggest braces around initialization of subobject [-Wmissing-braces]
{"Polly", {"support", "core", "scalaropts", "instcombine", "transformutils", "analysis", "ipo", "mc", "passes", "linker", "irreader", "analysis", "bitreader", "mcparser", "object", "profiledata", "target", "vectorize", "PollyISL", "Polly",nullptr}}, ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 {                                                                                                                                                                                                                                                      }

Should be fixed by 37309fb02f60557f18971dc575904c0fc56c91ab

[aheejin]

After this commit, builds with -DBUILD_SHARED_LIBS=ON fail.

[serge-sans-paille]

In D78358#2014810, @aheejin wrote:

After this commit, builds with -DBUILD_SHARED_LIBS=ON fail.

Can you share the other cmake options?

[aheejin]

This is my full compilation option, but I don't think anything other than -DBUILD_SHARED_LIBS=ON is relevant.

cmake -G Ninja -DCMAKE_C_COMPILER=$HOME/apps/clang+llvm/bin/clang -DCMAKE_CXX_COMPILER=$HOME/apps/clang+llvm/bin/clang++ -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DLLVM_TARGETS_TO_BUILD="X86;WebAssembly" -DLLVM_ENABLE_LIBCXX=ON -DLLVM_ENABLE_LLD=ON -DLLVM_ENABLE_ASSERTIONS=ON -DBUILD_SHARED_LIBS=ON -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DLLVM_BUILD_TESTS=ON -DCMAKE_CXX_FLAGS="-D_GLIBCXX_DEBUG" -DLLVM_INSTALL_UTILS=ON -DCMAKE_BUILD_TYPE=Debug -DCMAKE_INSTALL_PREFIX=$HOME/llvm-git/install.debug -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra;compiler-rt;debuginfo-tests;libclc;libcxx;libcxxabi;libunwind;lld;lldb;openmp;parallel-libs;polly;pstl;test-suite" ../llvm

[serge-sans-paille]

@aheejin thanks! I was able to reproduce, https://reviews.llvm.org/D79295 should fix the issue, can you confirm that?

llvm/cmake/modules/AddLLVM.cmake

 #   NO_INSTALL_RPATH
 #     Suppress default RPATH settings in shared libraries.
 #   PLUGIN_TOOL
 #     The tool (i.e. cmake target) that this plugin will link against
 #   COMPONENT_LIB
 #      This is used to specify that this is a component library of
 #      LLVM which means that the source resides in llvm/lib/ and it is a
 #      candidate for inclusion into libLLVM.so.
+#   PLUGIN_LIB
+#      This is used to specify that this is a statically loaded plugin library
+#      of LLVM which means that it behaves as a component, except that it
+#      doesn't interact with LLVMBuild.txt.
 #   )
 function(llvm_add_library name)
   cmake_parse_arguments(ARG
-    "MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB;ENABLE_PLUGINS"
+    "MODULE;SHARED;STATIC;OBJECT;DISABLE_LLVM_LINK_LLVM_DYLIB;SONAME;NO_INSTALL_RPATH;COMPONENT_LIB;ENABLE_PLUGINS;PLUGIN_LIB"
     "OUTPUT_NAME;PLUGIN_TOOL;ENTITLEMENTS;BUNDLE_PATH"
     "ADDITIONAL_HEADERS;DEPENDS;LINK_COMPONENTS;LINK_LIBS;OBJLIBS"
     ${ARGN})
   list(APPEND LLVM_COMMON_DEPENDS ${ARG_DEPENDS})
   if(ARG_ADDITIONAL_HEADERS)
     # Pass through ADDITIONAL_HEADERS.
     set(ARG_ADDITIONAL_HEADERS ADDITIONAL_HEADERS ${ARG_ADDITIONAL_HEADERS})
   endif()
   if(ARG_OBJLIBS)
     set(ALL_FILES ${ARG_OBJLIBS})
   else()
     llvm_process_sources(ALL_FILES ${ARG_UNPARSED_ARGUMENTS} ${ARG_ADDITIONAL_HEADERS})
   endif()
-  if(ARG_ENABLE_PLUGINS)
+
+  # If the library is a target for plugins, register it. Unless it links with LLVM_DYLIB that already is a target
+  # of plugins, which is why we always register component_lib.
+  if(ARG_ENABLE_PLUGINS AND (NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB OR ARG_COMPONENT_LIB))
     set_property(GLOBAL APPEND PROPERTY LLVM_PLUGIN_TARGETS ${name})
   endif()
 
   if(ARG_MODULE)
     if(ARG_SHARED OR ARG_STATIC)
       message(WARNING "MODULE with SHARED|STATIC doesn't make sense.")
     endif()
     # Plugins that link against a tool are allowed even when plugins in general are not
if(ARG_MODULE)
     add_library(${name} MODULE ${ALL_FILES})
   elseif(ARG_SHARED)
     add_windows_version_resource_file(ALL_FILES ${ALL_FILES})
     add_library(${name} SHARED ${ALL_FILES})
   else()
     add_library(${name} STATIC ${ALL_FILES})
   endif()
 
-  if(ARG_COMPONENT_LIB)
+  if(ARG_COMPONENT_LIB OR ARG_PLUGIN_LIB)
     set_target_properties(${name} PROPERTIES LLVM_COMPONENT TRUE)
     set_property(GLOBAL APPEND PROPERTY LLVM_COMPONENT_LIBS ${name})
   endif()
 
   if(NOT ARG_NO_INSTALL_RPATH)
     if(ARG_MODULE OR ARG_SHARED)
       llvm_setup_rpath(${name})
     endif()
else()
     # dependency information for this library as defined by LLVMBuild.
     #
     # It would be nice to verify that we have the dependencies for this library
     # name, but using get_property(... SET) doesn't suffice to determine if a
     # property has been set to an empty value.
     get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name})
   endif()
 
-  if(ARG_STATIC)
+  if(ARG_STATIC OR ARG_PLUGIN_LIB)
     set(libtype PUBLIC)
   else()
     # We can use PRIVATE since SO knows its dependent libs.
     set(libtype PRIVATE)
   endif()
 
   target_link_libraries(${name} ${libtype}
       ${ARG_LINK_LIBS}
if(NOT ARG_IGNORE_EXTERNALIZE_DEBUGINFO)
     llvm_externalize_debuginfo(${name})
   endif()
   if (LLVM_PTHREAD_LIB)
     # libpthreads overrides some standard library symbols, so main
     # executable must be linked with it in order to provide consistent
     # API for all shared libaries loaded by this executable.
     target_link_libraries(${name} PRIVATE ${LLVM_PTHREAD_LIB})
   endif()
-  if(ARG_ENABLE_PLUGINS)
+
+  # A tool that already links with LLVM_DYLIB does not need plugins, as plugins are already linked in LLVM_DYLIB
+  if(ARG_ENABLE_PLUGINS AND (NOT LLVM_LINK_LLVM_DYLIB OR DISABLE_LLVM_LINK_LLVM_DYLIB))
     set_property(GLOBAL APPEND PROPERTY LLVM_PLUGIN_TARGETS ${name})
   endif()
 
   llvm_codesign(${name} ENTITLEMENTS ${ARG_ENTITLEMENTS} BUNDLE_PATH ${ARG_BUNDLE_PATH})
 endmacro(add_llvm_executable name)
 
 # add_llvm_pass_plugin(name [NO_MODULE] ...)
 #   Add ${name} as an llvm plugin.
function(add_llvm_pass_plugin name)
   # Note: If was set to "all", LLVM's CMakeLists.txt replaces it with a
   # list of all projects, counting as explicitly enabled.
   set(link_into_tools_default OFF)
   if (ARG_SUBPROJECT AND LLVM_TOOL_${name_upper}_BUILD)
     set(link_into_tools_default ON)
   endif()
   option(LLVM_${name_upper}_LINK_INTO_TOOLS "Statically link ${name} into tools (if available)" ${link_into_tools_default})
 
+  # If we statically link the plugin, don't use llvm dylib
+  if(LLVM_${name_upper}_LINK_INTO_TOOLS)
+      list(APPEND ARG_UNPARSED_ARGUMENTS DISABLE_LLVM_LINK_LLVM_DYLIB)
+  endif()
+
   if(LLVM_${name_upper}_LINK_INTO_TOOLS)
     list(REMOVE_ITEM ARG_UNPARSED_ARGUMENTS BUILDTREE_ONLY)
     # process_llvm_pass_plugins takes care of the actual linking, just create an
     # object library as of now
-    add_llvm_component_library(${name} OBJECT ${ARG_UNPARSED_ARGUMENTS})
+    add_llvm_library(${name} OBJECT ${ARG_UNPARSED_ARGUMENTS} PLUGIN_LIB)
     target_compile_definitions(${name} PRIVATE LLVM_${name_upper}_LINK_INTO_TOOLS)
     set_property(TARGET ${name} APPEND PROPERTY COMPILE_DEFINITIONS LLVM_LINK_INTO_TOOLS)
     if (TARGET intrinsics_gen)
       add_dependencies(obj.${name} intrinsics_gen)
     endif()
     set_property(GLOBAL APPEND PROPERTY LLVM_STATIC_EXTENSIONS ${name})
   elseif(NOT ARG_NO_MODULE)
     add_llvm_library(${name} MODULE ${ARG_UNPARSED_ARGUMENTS})

polly/lib/CMakeLists.txt

 # Compile ISL into a separate library.
 add_subdirectory(External)
 
 set(POLLY_HEADER_FILES)
 if (MSVC_IDE OR XCODE)
   file(GLOB_RECURSE POLLY_HEADER_FILES "${POLLY_SOURCE_DIR}/include/polly/*.h")
 endif ()
 
+set(POLLY_COMPONENTS
+    Support
+    Core
+    ScalarOpts
+    InstCombine
+    TransformUtils
+    Analysis
+    ipo
+    MC
+    Passes
+    Linker
+    IRReader
+    Analysis
+    # The libraries below are required for darwin: http://PR26392
+    BitReader
+    MCParser
+    Object
+    ProfileData
+    Target
+    Vectorize
+)
+
+# Polly-ACC requires the NVPTX backend to work. Ask LLVM about its libraries.
+if (GPU_CODEGEN)
+  # This call emits an error if they NVPTX backend is not enable.
+  list(APPEND POLLY_COMPONENTS NVPTX)
+endif ()
+
 # Use an object-library to add the same files to multiple libs without requiring
 # the sources them to be recompiled for each of them.
 add_llvm_pass_plugin(Polly
   NO_MODULE
   SUBPROJECT Polly
   Analysis/DependenceInfo.cpp
   Analysis/PolyhedralInfo.cpp
   Analysis/ScopDetection.cpp
add_llvm_pass_plugin(Polly
   Transform/ForwardOpTree.cpp
   Transform/DeLICM.cpp
   Transform/ZoneAlgo.cpp
   Transform/Simplify.cpp
   Transform/MaximalStaticExpansion.cpp
   Transform/RewriteByReferenceParameters.cpp
   Transform/ScopInliner.cpp
   ${POLLY_HEADER_FILES}
+
+  LINK_COMPONENTS
+  ${POLLY_COMPONENTS}
   )
 set_target_properties(obj.Polly PROPERTIES FOLDER "Polly")
 set_target_properties(Polly PROPERTIES FOLDER "Polly")
 
 if (MSVC_IDE OR XCODE)
   # Configure source groups for Polly source files. By default, in the IDE there
   # will be a source and include folder. In the source folder will be all the
   # source files in a flat list, and in the include folder will be all the
target_link_libraries(Polly PUBLIC
   ${ISL_TARGET}
 )
 
 # Additional dependencies for Polly-ACC.
 if (GPU_CODEGEN)
   target_link_libraries(Polly PUBLIC PollyPPCG)
 endif ()
 
-
-# Polly-ACC requires the NVPTX backend to work. Ask LLVM about its libraries.
-set(nvptx_libs)
-if (GPU_CODEGEN)
-  # This call emits an error if they NVPTX backend is not enable.
-  llvm_map_components_to_libnames(nvptx_libs NVPTX)
-endif ()
-
-if (LLVM_LINK_LLVM_DYLIB AND NOT LLVM_POLLY_LINK_INTO_TOOLS)
-  # The shlib/dylib contains all the LLVM components
-  # (including NVPTX is enabled) already. Adding them to target_link_libraries
-  # would cause them being twice in the address space
-  # (their LLVM*.a/so and their copies in libLLVM.so)
-  # which results in errors when the two instances try to register the same
-  # command-line switches.
-  target_link_libraries(Polly PUBLIC LLVM)
-else ()
-  target_link_libraries(Polly PUBLIC
-    LLVMSupport
-    LLVMCore
-    LLVMScalarOpts
-    LLVMInstCombine
-    LLVMTransformUtils
-    LLVMAnalysis
-    LLVMipo
-    LLVMMC
-    LLVMPasses
-    LLVMLinker
-    LLVMIRReader
-    ${nvptx_libs}
-    # The libraries below are required for darwin: http://PR26392
-    LLVMBitReader
-    LLVMMCParser
-    LLVMObject
-    LLVMProfileData
-    LLVMTarget
-    LLVMVectorize
-    )
-
+if (NOT LLVM_LINK_LLVM_DYLIB AND NOT LLVM_POLLY_LINK_INTO_TOOLS)
     # Polly-ACC requires the NVPTX target to be present in the executable it is linked to
     set_property(TARGET bugpoint APPEND PROPERTY LINK_LIBRARIES LLVMTarget)
 endif ()
 
 # Create a loadable module Polly.so that can be loaded using
 # LLVM's/clang's "-load" option.
 if (MSVC)
   # Add dummy target, because loadable modules are not supported on Windows