llvm-shlib: Create object libraries for each component and link against them

[tstellar]

Summary

This makes it possible to build libLLVM.so without first creating a
static library for each component. In the case where only libLLVM.so is
built (i.e. ninja LLVM) this eliminates 150 linker jobs.

[stellaraccident]

Nice - I had been considering this kind of change based on similar observations (prior to my larger refactor escapades).

[bjope]

Unfortunately out downstream bots started to fail with this commit (building the main branch).

Not sure exactly what happens, but my cmake command looks like this

cmake command: CC='/app/llvm/8.0/bin/clang -march=corei7' CXX='/app/llvm/8.0/bin/clang++ -march=corei7' LDFLAGS='-L/app/llvm/8.0/lib64 -Wl,-R/app/llvm/8.0/lib64:/z3/4.8.8-1/lib64' PATH=/app/ninja/1.8.2/bin:$PATH  /app/vbuild/RHEL7-x86_64/cmake/3.16.4/bin/cmake /repo/llvm-upstream/llvm --debug-trycompile -G Ninja -DCMAKE_MAKE_PROGRAM=/app/ninja/1.8.2/bin/ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DCMAKE_INSTALL_PREFIX=/compiler-clang -DLLVM_APPEND_VC_REV=OFF -DLLVM_ENABLE_ASSERTIONS=ON -DLLVM_ENABLE_WERROR=ON -DLLVM_ENABLE_PROJECTS='clang;clang-tools-extra;lld' -DLLVM_ENABLE_Z3_SOLVER=ON -DLLVM_Z3_INSTALL_DIR=/z3/4.8.8-1 -DLLVM_ENABLE_LIBPFM=OFF

And with this patch I get:

FAILED: lib/Support/CMakeFiles/obj.LLVMSupport.dir/Z3Solver.cpp.o 
/app/llvm/8.0/bin/clang++  -march=corei7  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/Support -I../lib/Support -Iinclude -I../include -fPIC -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3    -UNDEBUG -std=c++14  -fno-exceptions -fno-rtti -MD -MT lib/Support/CMakeFiles/obj.LLVMSupport.dir/Z3Solver.cpp.o -MF lib/Support/CMakeFiles/obj.LLVMSupport.dir/Z3Solver.cpp.o.d -o lib/Support/CMakeFiles/obj.LLVMSupport.dir/Z3Solver.cpp.o -c ../lib/Support/Z3Solver.cpp
../lib/Support/Z3Solver.cpp:19:10: fatal error: 'z3.h' file not found
#include <z3.h>
         ^~~~~~
1 error generated.

Without this patch I also see -isystem /z3/4.8.8-1/include on the cmd line for compiling Z3Solver.cpp.

[bjope]

So this seem to break more things than just Z3 builds (considering D99829). Should we revert while sorting out these things. I'm kind of lost here, just know that our integration bots are stuck and they've been failing since this patch landed on April 1.

[bjope]

Not sure if it is the correct thing to do, but a patch like this seem to help for the Z3 problem:

diff --git a/llvm/lib/Support/CMakeLists.txt b/llvm/lib/Support/CMakeLists.txt
index ddacb4feaa0f..36919b25b0ef 100644
--- a/llvm/lib/Support/CMakeLists.txt
+++ b/llvm/lib/Support/CMakeLists.txt
@@ -274,8 +274,5 @@ if(LLVM_INTEGRATED_CRT_ALLOC)
 endif()
 
 if(LLVM_WITH_Z3)
-  target_include_directories(LLVMSupport SYSTEM
-    PRIVATE
-    ${Z3_INCLUDE_DIR}
-    )
+  include_directories(SYSTEM ${Z3_INCLUDE_DIR})
 endif()

[bjope]

I pushed my workaround for Z3 here: https://reviews.llvm.org/rGd66f9c4f1e83e69abf75f97cb5f8fd1dc9422357
Hopefully that will make our bots happy again.

I don't mind if someone wants to do a post-commit review of my workaround.

[tstellar]

In D95727#2667418, @bjope wrote:

I pushed my workaround for Z3 here: https://reviews.llvm.org/rGd66f9c4f1e83e69abf75f97cb5f8fd1dc9422357
Hopefully that will make our bots happy again.

I don't mind if someone wants to do a post-commit review of my workaround.

Thanks, let me know if you are still seeing issues.

[tstellar]

Updated patch with fixes for z3 and libffi build failures

https://bugs.llvm.org/show_bug.cgi?id=49818

[MaskRay]

In the case where only libLLVM.so is built (i.e. ninja LLVM) this eliminates 150 linker jobs.

Creating an archive does not need linker.

[MaskRay]

When Polly is in LLVM_ENABLE_PROJECTS, I get this

ld.lld: error: undefined symbol: getPollyPluginInfo()
>>> referenced by Extensions.cpp
>>>               lib/Extensions/CMakeFiles/obj.LLVMExtensions.dir/Extensions.cpp.o:(llvm::details::extensions_anchor())
>>> referenced by LTOBackend.cpp
>>>               lib/LTO/CMakeFiles/obj.LLVMLTO.dir/LTOBackend.cpp.o:(llvm::lto::opt(llvm::lto::Config const&, llvm::TargetMachine*, unsigned int, llvm::Module&, bool, llvm::ModuleSummaryIndex*, llvm::ModuleSummaryIndex const*, std::vector<unsigned char, std::allocator<unsigned char> > const&))

I find that in the linker command line all the .o files are linked without --start-lib. This means there will be no archive selection. Can this increase the output size?

[phosek]

@tstellar what are the blockers for getting this landed?

In D95727#2783931, @MaskRay wrote:

When Polly is in LLVM_ENABLE_PROJECTS, I get this

ld.lld: error: undefined symbol: getPollyPluginInfo()
>>> referenced by Extensions.cpp
>>>               lib/Extensions/CMakeFiles/obj.LLVMExtensions.dir/Extensions.cpp.o:(llvm::details::extensions_anchor())
>>> referenced by LTOBackend.cpp
>>>               lib/LTO/CMakeFiles/obj.LLVMLTO.dir/LTOBackend.cpp.o:(llvm::lto::opt(llvm::lto::Config const&, llvm::TargetMachine*, unsigned int, llvm::Module&, bool, llvm::ModuleSummaryIndex*, llvm::ModuleSummaryIndex const*, std::vector<unsigned char, std::allocator<unsigned char> > const&))

I find that in the linker command line all the .o files are linked without --start-lib. This means there will be no archive selection. Can this increase the output size?

It does, unless you use -Wl,--gc-sections, that's one of the downsides of using object libraries over archives.

[tstellar]

@phosek The only thing blocking this was that I wasn't sure what to do about @MaskRay 's comment. Other than that, I think all the regressions from before have been fixed.

[beanz]

In D95727#2819595, @tstellar wrote:

@phosek The only thing blocking this was that I wasn't sure what to do about @MaskRay 's comment. Other than that, I think all the regressions from before have been fixed.

The fix for that is probably to exclude libLLVMExtensions from the dylib. It probably shouldn't be there anyways.

[MaskRay]

In the case where only libLLVM.so is built (i.e. ninja LLVM) this eliminates 150 linker jobs.

Do you mean ar/llvm-ar?

It does, unless you use -Wl,--gc-sections, that's one of the downsides of using object libraries over archives.

Most non-SHF_ALLOC sections cannot be GCed.

If we want to save the copy/paste work done by ar/llvm-ar, can we use thin archives?

[Meinersbur]

In D95727#2819600, @beanz wrote:

The fix for that is probably to exclude libLLVMExtensions from the dylib. It probably shouldn't be there anyways.

I think it is needed since the dylib references symbols in libLLVMExtensions for statically linked plugins, LLVMLTO depends on it. It was added to resolve build errors. @serge-sans-paille may know more.

[tstellar]

Fix build failure with polly enabled.

[tstellar]

In D95727#2783931, @MaskRay wrote:

When Polly is in LLVM_ENABLE_PROJECTS, I get this

ld.lld: error: undefined symbol: getPollyPluginInfo()
>>> referenced by Extensions.cpp
>>>               lib/Extensions/CMakeFiles/obj.LLVMExtensions.dir/Extensions.cpp.o:(llvm::details::extensions_anchor())
>>> referenced by LTOBackend.cpp
>>>               lib/LTO/CMakeFiles/obj.LLVMLTO.dir/LTOBackend.cpp.o:(llvm::lto::opt(llvm::lto::Config const&, llvm::TargetMachine*, unsigned int, llvm::Module&, bool, llvm::ModuleSummaryIndex*, llvm::ModuleSummaryIndex const*, std::vector<unsigned char, std::allocator<unsigned char> > const&))

I find that in the linker command line all the .o files are linked without --start-lib. This means there will be no archive selection. Can this increase the output size?

Is --start-lib something specific to lld? I did builds with binutils and libLLVM.so was the same size with and without this patch.

[MaskRay]

Is --start-lib something specific to lld? I did builds with binutils and libLLVM.so was the same size with and without this patch.

gold and ld.lld support --start-lib for a long time. (Bazel uses --start-lib by default.)
GNU ld feature request: https://sourceware.org/bugzilla/show_bug.cgi?id=24600

Switching away from archive semantics (http://maskray.me/blog/2021-06-20-symbol-processing) has the problem that unused .debug_* may be added to the output with gold and ld.lld.

GNU ld may have a rarely known heuristic to not add .debug_* if all text section are garbage collected but I am not clear about its semantics.

[Meinersbur]

I test this with LLVM_LINK_LLVM_DYLIB, and with Windows. both seem to work, so LGTM.

[tstellar]

In D95727#3076485, @MaskRay wrote:

Is --start-lib something specific to lld? I did builds with binutils and libLLVM.so was the same size with and without this patch.

gold and ld.lld support --start-lib for a long time. (Bazel uses --start-lib by default.)
GNU ld feature request: https://sourceware.org/bugzilla/show_bug.cgi?id=24600

Switching away from archive semantics (http://maskray.me/blog/2021-06-20-symbol-processing) has the problem that unused .debug_* may be added to the output with gold and ld.lld.

GNU ld may have a rarely known heuristic to not add .debug_* if all text section are garbage collected but I am not clear about its semantics.

What do I need to do in order to avoid changing the output of lld and gold with this patch? Can I wrap the whole list of object files with -Wl,--start-lib and -Wl,--end-lib or do I need to have multiple --start-lib and --end-lib pairs, one for each original static archive?

[MaskRay]

In D95727#3101898, @tstellar wrote:

In D95727#3076485, @MaskRay wrote:

Is --start-lib something specific to lld? I did builds with binutils and libLLVM.so was the same size with and without this patch.

gold and ld.lld support --start-lib for a long time. (Bazel uses --start-lib by default.)
GNU ld feature request: https://sourceware.org/bugzilla/show_bug.cgi?id=24600

Switching away from archive semantics (http://maskray.me/blog/2021-06-20-symbol-processing) has the problem that unused .debug_* may be added to the output with gold and ld.lld.

GNU ld may have a rarely known heuristic to not add .debug_* if all text section are garbage collected but I am not clear about its semantics.

What do I need to do in order to avoid changing the output of lld and gold with this patch? Can I wrap the whole list of object files with -Wl,--start-lib and -Wl,--end-lib or do I need to have multiple --start-lib and --end-lib pairs, one for each original static archive?

One pair of -Wl,--start-lib and -Wl,--end-lib for each original archive.

If you have -Wl,--start-lib a0.o a1.o -Wl,--end-lib -Wl,--start-lib b0.o b1.o -Wl,--end-lib, -Wl,--end-lib -Wl,--start-lib can be cancelled.

[phosek]

@tstellar Do you still plan to work on this change? If not, would it be fine with you if we commandeered the change?

[tstellar]

@phosek You can take over, that's fine with me.

[tstellar]

Rebase on main.

[tstellar]

I've tested both Release and RelWithDebInfo build configurations, and the libLLVM.so library is approximately the same size. @MaskRay Do we still need --start-lib --end-lib? I noticed ld.bfd still doesn't support this.

llvm/cmake/modules/AddLLVM.cmake

if(ARG_LINK_LIBS)
           # Can't specify a dependence on -lpthread
           if(NOT ${link_lib} STREQUAL ${LLVM_PTHREAD_LIB})
             add_dependencies(${obj_name} ${link_lib})
           endif()
         else()
           add_dependencies(${obj_name} ${link_lib})
         endif()
       endforeach()
+      target_link_libraries(${obj_name} ${ARG_LINK_LIBS})
     endif()
+
+    # Add target include directories from the main target.  This makes it
+    # possible to call target_include_directories() with ${name} after
+    # calling llvm_add_library() and still have it apply to the object library.
+    set_target_properties(obj.${name} PROPERTIES INCLUDE_DIRECTORIES "$<TARGET_PROPERTY:${name},INCLUDE_DIRECTORIES>")
   endif()
 
   if(ARG_SHARED AND ARG_STATIC)
     # static
     set(name_static "${name}_static")
     if(ARG_OUTPUT_NAME)
       set(output_name OUTPUT_NAME "${ARG_OUTPUT_NAME}")
     endif()

 # Additionnaly, the ADD_TO_COMPONENT <component> option make it possible to add this
 # component to the LLVM_LINK_COMPONENTS of <component>.
 function(add_llvm_component_library name)
   cmake_parse_arguments(ARG
     ""
     "COMPONENT_NAME;ADD_TO_COMPONENT"
     ""
     ${ARGN})
-  add_llvm_library(${name} COMPONENT_LIB ${ARG_UNPARSED_ARGUMENTS})
+  add_llvm_library(${name} COMPONENT_LIB OBJECT ${ARG_UNPARSED_ARGUMENTS})
   string(REGEX REPLACE "^LLVM" "" component_name ${name})
   set_property(TARGET ${name} PROPERTY LLVM_COMPONENT_NAME ${component_name})
 
   if(ARG_COMPONENT_NAME)
     set_property(GLOBAL PROPERTY LLVM_COMPONENT_NAME_${ARG_COMPONENT_NAME} ${component_name})
   endif()
 
   if(ARG_ADD_TO_COMPONENT)

llvm/lib/ExecutionEngine/Interpreter/CMakeLists.txt

 # Make sure that the path to libffi headers is on the command
 # line. That path can be a compiler's non-default path even when
 # FFI_INCLUDE_DIR was not used, because cmake has its own paths for
 # searching for headers (CMAKE_SYSTEM_INCLUDE_PATH, for instance):
 if( FFI_INCLUDE_PATH )
   include_directories( ${FFI_INCLUDE_PATH} )
 endif()
 
+if( LLVM_ENABLE_FFI )
+  set(link_libs ${FFI_LIBRARY_PATH})
+endif()
+
 add_llvm_component_library(LLVMInterpreter
   Execution.cpp
   ExternalFunctions.cpp
   Interpreter.cpp
 
   DEPENDS
   intrinsics_gen
 
+  LINK_LIBS PRIVATE ${link_libs}
   LINK_COMPONENTS
   CodeGen
   Core
   ExecutionEngine
   Support
   )
-
-if( LLVM_ENABLE_FFI )
-  target_link_libraries( LLVMInterpreter PRIVATE ${FFI_LIBRARY_PATH} )
-endif()

llvm/tools/llvm-shlib/CMakeLists.txt

if(LLVM_BUILD_LLVM_DYLIB)
   endif()
 
   if (LLVM_LINK_LLVM_DYLIB)
     set(INSTALL_WITH_TOOLCHAIN INSTALL_WITH_TOOLCHAIN)
   endif()
   add_llvm_library(LLVM SHARED DISABLE_LLVM_LINK_LLVM_DYLIB SONAME ${INSTALL_WITH_TOOLCHAIN} ${SOURCES})
 
   list(REMOVE_DUPLICATES LIB_NAMES)
+
+  # Link against the object libraries instead of static libraries.
+  list(TRANSFORM LIB_NAMES PREPEND "obj.")
+
   if(("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") OR (MINGW) OR (HAIKU)
      OR ("${CMAKE_SYSTEM_NAME}" STREQUAL "FreeBSD")
      OR ("${CMAKE_SYSTEM_NAME}" STREQUAL "GNU")
      OR ("${CMAKE_SYSTEM_NAME}" STREQUAL "OpenBSD")
      OR ("${CMAKE_SYSTEM_NAME}" STREQUAL "Fuchsia")
      OR ("${CMAKE_SYSTEM_NAME}" STREQUAL "DragonFly")
      OR ("${CMAKE_SYSTEM_NAME}" STREQUAL "SunOS")) # FIXME: It should be "GNU ld for elf"
     configure_file(