Building llvm/clang/compiler-rt/polly/libc++/openmp with -DLLVM_LINK_LLVM_DYLIB:BOOL=ON on x86_64 results in many llvm test suite regressions. Expected Passes : 7605 Expected Failures : 51 Unsupported Tests : 3841 Unexpected Failures: 3853 These all seem to be some variation of opt reporting an unknown command line argument being passed to it such as... FAIL: LLVM :: Analysis/BasicAA/2003-02-26-AccessSizeTest.ll (1 of 15350) ******************** TEST 'LLVM :: Analysis/BasicAA/2003-02-26-AccessSizeTest.ll' FAILED ******************** Script: -- gtimeout 1m /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/opt < /sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/test/Analysis/BasicAA/2003-02-26-AccessSizeTest.ll -basicaa -gvn -instcombine -S | /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/FileCheck /sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/test/Analysis/BasicAA/2003-02-26-AccessSizeTest.ll -- Exit Code: 2 Command Output (stderr): -- opt: Unknown command line argument '-basicaa'. Try: '/sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/opt -help' opt: Did you mean '-passes'? opt: Unknown command line argument '-gvn'. Try: '/sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/opt -help' opt: Did you mean '-Os'? opt: Unknown command line argument '-instcombine'. Try: '/sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/opt -help' opt: Did you mean '-print-options'? FileCheck error: '-' is empty. --
A manual test of... /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/opt -help | grep basic produces no output rather than the expected... -basicaa - Basic Alias Analysis (stateless AA impl)
The linkage of the broken opt is set in CMakeFiles/opt.dir/link.txt as... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/cc-st2-clang++ -std=c++11 -stdlib=libc++ -cxx-isystem /sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../libcxx-3.9.0.src/include -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wnon-virtual-dtor -std=c++11 -O3 -DNDEBUG -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -L/sw/lib -rdynamic CMakeFiles/opt.dir/AnalysisWrappers.cpp.o CMakeFiles/opt.dir/BreakpointPrinter.cpp.o CMakeFiles/opt.dir/GraphPrinters.cpp.o CMakeFiles/opt.dir/NewPMDriver.cpp.o CMakeFiles/opt.dir/PassPrinters.cpp.o CMakeFiles/opt.dir/PrintSCC.cpp.o CMakeFiles/opt.dir/opt.cpp.o -o ../../bin/opt ../../lib/libLLVM.dylib ../../lib/libLLVMX86CodeGen.a ../../lib/libLLVMX86AsmPrinter.a ../../lib/libLLVMX86AsmParser.a ../../lib/libLLVMX86Desc.a ../../lib/libLLVMX86Info.a ../../lib/libLLVMX86Disassembler.a ../../lib/libLLVMPowerPCCodeGen.a ../../lib/libLLVMPowerPCAsmPrinter.a ../../lib/libLLVMPowerPCAsmParser.a ../../lib/libLLVMPowerPCDesc.a ../../lib/libLLVMPowerPCInfo.a ../../lib/libLLVMPowerPCDisassembler.a ../../lib/libLLVMARMCodeGen.a ../../lib/libLLVMARMAsmPrinter.a ../../lib/libLLVMARMAsmParser.a ../../lib/libLLVMARMDesc.a ../../lib/libLLVMARMInfo.a ../../lib/libLLVMARMDisassembler.a ../../lib/libLLVMAnalysis.a ../../lib/libLLVMBitWriter.a ../../lib/libLLVMCodeGen.a ../../lib/libLLVMCore.a ../../lib/libLLVMipo.a ../../lib/libLLVMIRReader.a ../../lib/libLLVMInstCombine.a ../../lib/libLLVMInstrumentation.a ../../lib/libLLVMMC.a ../../lib/libLLVMObjCARCOpts.a ../../lib/libLLVMScalarOpts.a ../../lib/libLLVMSupport.a ../../lib/libLLVMTarget.a ../../lib/libLLVMTransformUtils.a ../../lib/libLLVMVectorize.a ../../lib/libLLVMPasses.a ../../lib/libLLVMX86AsmPrinter.a ../../lib/libLLVMX86Utils.a ../../lib/libLLVMX86Info.a ../../lib/libLLVMPowerPCAsmPrinter.a ../../lib/libLLVMPowerPCInfo.a ../../lib/libLLVMAsmPrinter.a ../../lib/libLLVMDebugInfoCodeView.a ../../lib/libLLVMSelectionDAG.a ../../lib/libLLVMCodeGen.a ../../lib/libLLVMBitWriter.a ../../lib/libLLVMTarget.a ../../lib/libLLVMARMDesc.a ../../lib/libLLVMARMAsmPrinter.a ../../lib/libLLVMARMInfo.a ../../lib/libLLVMMCDisassembler.a ../../lib/libLLVMipo.a ../../lib/libLLVMIRReader.a ../../lib/libLLVMAsmParser.a ../../lib/libLLVMInstrumentation.a ../../lib/libLLVMLinker.a ../../lib/libLLVMProfileData.a ../../lib/libLLVMObject.a ../../lib/libLLVMMCParser.a ../../lib/libLLVMMC.a ../../lib/libLLVMBitReader.a ../../lib/libLLVMScalarOpts.a ../../lib/libLLVMInstCombine.a ../../lib/libLLVMVectorize.a ../../lib/libLLVMTransformUtils.a ../../lib/libLLVMAnalysis.a ../../lib/libLLVMCore.a ../../lib/libLLVMSupport.a -lcurses -lpthread -lz -lm -Wl,-rpath,@executable_path/../lib and the linkage of the libLLVM.dylib used is set in CMakeFiles/LLVM.dir/link.txt as... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/cc-st2-clang++ -std=c++11 -stdlib=libc++ -cxx-isystem /sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../libcxx-3.9.0.src/include -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wnon-virtual-dtor -std=c++11 -O3 -DNDEBUG -dynamiclib -Wl,-headerpad_max_install_names -Wl,-dead_strip -compatibility_version 1 -current_version 3.9.0 -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -L/sw/lib -o ../../lib/libLLVM.dylib -install_name @rpath/libLLVM.dylib CMakeFiles/LLVM.dir/libllvm.cpp.o -Wl,-all_load ../../lib/libLLVMSupport.a ../../lib/libLLVMCore.a ../../lib/libLLVMIRReader.a ../../lib/libLLVMCodeGen.a ../../lib/libLLVMSelectionDAG.a ../../lib/libLLVMAsmPrinter.a ../../lib/libLLVMMIRParser.a ../../lib/libLLVMBitReader.a ../../lib/libLLVMBitWriter.a ../../lib/libLLVMTransformUtils.a ../../lib/libLLVMInstrumentation.a ../../lib/libLLVMInstCombine.a ../../lib/libLLVMScalarOpts.a ../../lib/libLLVMipo.a ../../lib/libLLVMVectorize.a ../../lib/libLLVMObjCARCOpts.a ../../lib/libLLVMLinker.a ../../lib/libLLVMAnalysis.a ../../lib/libLLVMLTO.a ../../lib/libLLVMMC.a ../../lib/libLLVMMCParser.a ../../lib/libLLVMMCDisassembler.a ../../lib/libLLVMObject.a ../../lib/libLLVMOption.a ../../lib/libLLVMDebugInfoCodeView.a ../../lib/libLLVMDebugInfoDWARF.a ../../lib/libLLVMDebugInfoPDB.a ../../lib/libLLVMSymbolize.a ../../lib/libLLVMExecutionEngine.a ../../lib/libLLVMInterpreter.a ../../lib/libLLVMMCJIT.a ../../lib/libLLVMOrcJIT.a ../../lib/libLLVMRuntimeDyld.a ../../lib/libLLVMTarget.a ../../lib/libLLVMX86CodeGen.a ../../lib/libLLVMX86AsmParser.a ../../lib/libLLVMX86Disassembler.a ../../lib/libLLVMX86AsmPrinter.a ../../lib/libLLVMX86Desc.a ../../lib/libLLVMX86Info.a ../../lib/libLLVMX86Utils.a ../../lib/libLLVMPowerPCCodeGen.a ../../lib/libLLVMPowerPCAsmParser.a ../../lib/libLLVMPowerPCDisassembler.a ../../lib/libLLVMPowerPCAsmPrinter.a ../../lib/libLLVMPowerPCInfo.a ../../lib/libLLVMPowerPCDesc.a ../../lib/libLLVMARMCodeGen.a ../../lib/libLLVMARMInfo.a ../../lib/libLLVMARMAsmParser.a ../../lib/libLLVMARMDisassembler.a ../../lib/libLLVMARMAsmPrinter.a ../../lib/libLLVMARMDesc.a ../../lib/libLLVMAsmParser.a ../../lib/libLLVMLineEditor.a ../../lib/libLLVMProfileData.a ../../lib/libLLVMPasses.a ../../lib/libLLVMLibDriver.a ../../lib/libLLVMObjCARCOpts.a ../../lib/libLLVMDebugInfoDWARF.a ../../lib/libLLVMDebugInfoPDB.a ../../lib/libLLVMExecutionEngine.a ../../lib/libLLVMRuntimeDyld.a ../../lib/libLLVMX86AsmPrinter.a ../../lib/libLLVMX86Utils.a ../../lib/libLLVMPowerPCAsmPrinter.a ../../lib/libLLVMPowerPCInfo.a ../../lib/libLLVMSelectionDAG.a ../../lib/libLLVMAsmPrinter.a ../../lib/libLLVMCodeGen.a ../../lib/libLLVMBitWriter.a ../../lib/libLLVMDebugInfoCodeView.a ../../lib/libLLVMTarget.a ../../lib/libLLVMMCDisassembler.a ../../lib/libLLVMARMInfo.a ../../lib/libLLVMARMAsmPrinter.a -ledit ../../lib/libLLVMipo.a ../../lib/libLLVMIRReader.a ../../lib/libLLVMAsmParser.a ../../lib/libLLVMInstrumentation.a ../../lib/libLLVMScalarOpts.a ../../lib/libLLVMInstCombine.a ../../lib/libLLVMLinker.a ../../lib/libLLVMProfileData.a ../../lib/libLLVMVectorize.a ../../lib/libLLVMTransformUtils.a ../../lib/libLLVMAnalysis.a ../../lib/libLLVMObject.a ../../lib/libLLVMBitReader.a ../../lib/libLLVMCore.a ../../lib/libLLVMMCParser.a ../../lib/libLLVMMC.a ../../lib/libLLVMOption.a ../../lib/libLLVMSupport.a -lcurses -lpthread -lz -lm -Wl,-rpath,@executable_path/../lib
If I remove the unnecessary static libraries from tools/opt/CMakeFiles/opt.dir/link.txt... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/cc-st2-clang++ -std=c++11 -stdlib=libc++ -cxx-isystem /sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../libcxx-3.9.0.src/include -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wnon-virtual-dtor -std=c++11 -O3 -DNDEBUG -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -L/sw/lib -rdynamic CMakeFiles/opt.dir/AnalysisWrappers.cpp.o CMakeFiles/opt.dir/BreakpointPrinter.cpp.o CMakeFiles/opt.dir/GraphPrinters.cpp.o CMakeFiles/opt.dir/NewPMDriver.cpp.o CMakeFiles/opt.dir/PassPrinters.cpp.o CMakeFiles/opt.dir/PrintSCC.cpp.o CMakeFiles/opt.dir/opt.cpp.o -o ../../bin/opt ../../lib/libLLVM.dylib -lcurses -lpthread -lz -lm -Wl,-rpath,@executable_path/../lib the resulting binary then works as expected... # /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/opt -help | grep basic -basicaa - Basic Alias Analysis (stateless AA impl) -basiccg - CallGraph Construction =basic - basic register allocator
I assume the fixes should look something like... Index: tools/opt/CMakeLists.txt =================================================================== --- tools/opt/CMakeLists.txt (revision 259337) +++ tools/opt/CMakeLists.txt (working copy) @@ -1,22 +1,29 @@ -set(LLVM_LINK_COMPONENTS - ${LLVM_TARGETS_TO_BUILD} - Analysis - BitWriter - CodeGen - Core - IPO - IRReader - InstCombine - Instrumentation - MC - ObjCARCOpts - ScalarOpts - Support - Target - TransformUtils - Vectorize - Passes - ) +if (LLVM_LINK_LLVM_DYLIB) + set(LLVM_LINK_COMPONENTS + ${LLVM_TARGETS_TO_BUILD} + LLVM + ) +else () + set(LLVM_LINK_COMPONENTS + ${LLVM_TARGETS_TO_BUILD} + Analysis + BitWriter + CodeGen + Core + IPO + IRReader + InstCombine + Instrumentation + MC + ObjCARCOpts + ScalarOpts + Support + Target + TransformUtils + Vectorize + Passes + ) +endif () # Support plugins. set(LLVM_NO_DEAD_STRIP 1)
No. Actually following that pattern every user of LLVM would need to update their library usage individually. That would suck. There is a blurb in llvm/tools/llvm-c-test/CMakeLists.txt:16-27 that checks to see what components are in the dylib. We need to adapt something like that back into the LLVM-Config module. See the TODO at llvm/cmake/modules/LLVM-Config.cmake:43.
(In reply to comment #5) > No. Actually following that pattern every user of LLVM would need to update > their library usage individually. That would suck. There is a blurb in > llvm/tools/llvm-c-test/CMakeLists.txt:16-27 that checks to see what > components are in the dylib. > > We need to adapt something like that back into the LLVM-Config module. See > the TODO at llvm/cmake/modules/LLVM-Config.cmake:43. So something like... Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259371) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -44,6 +44,18 @@ # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + + if (TARGET LLVM) + if (DEFINED LLVM_DYLIB_COMPONENTS) + foreach(c in ${LLVM_LINK_COMPONENTS}) + list(FIND LLVM_DYLIB_COMPONENTS ${c} C_IDX) + if (NOT C_IDX EQUAL -1) + list(REMOVE_ITEM LLVM_LINK_COMPONENTS ${c}) + endif() + endforeach() + endif() + endif() + target_link_libraries(${executable} LLVM) endif()
That looks reasonable to me. Does it resolve your issues?
(In reply to comment #7) > That looks reasonable to me. Does it resolve your issues? Unfortunately no joy here... -- Performing Test CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG -- Performing Test CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG - Success CMake Error at cmake/modules/LLVM-Config.cmake:206 (message): Library `LLVM' not found in list of llvm libraries. Call Stack (most recent call first): cmake/modules/LLVM-Config.cmake:69 (llvm_map_components_to_libnames) cmake/modules/LLVM-Config.cmake:62 (explicit_llvm_config) cmake/modules/AddLLVM.cmake:662 (llvm_config) cmake/modules/AddLLVM.cmake:695 (add_llvm_executable) tools/opt/CMakeLists.txt:31 (add_llvm_tool)
(In reply to comment #6) > (In reply to comment #5) > > No. Actually following that pattern every user of LLVM would need to update > > their library usage individually. That would suck. There is a blurb in > > llvm/tools/llvm-c-test/CMakeLists.txt:16-27 that checks to see what > > components are in the dylib. > > > > We need to adapt something like that back into the LLVM-Config module. See > > the TODO at llvm/cmake/modules/LLVM-Config.cmake:43. > > So something like... > > Index: cmake/modules/LLVM-Config.cmake > =================================================================== > --- cmake/modules/LLVM-Config.cmake (revision 259371) > +++ cmake/modules/LLVM-Config.cmake (working copy) > @@ -44,6 +44,18 @@ > # To do this, we need special handling for "all", since that > # may imply linking to libraries that are not included in > # libLLVM. > + > + if (TARGET LLVM) > + if (DEFINED LLVM_DYLIB_COMPONENTS) > + foreach(c in ${LLVM_LINK_COMPONENTS}) > + list(FIND LLVM_DYLIB_COMPONENTS ${c} C_IDX) > + if (NOT C_IDX EQUAL -1) > + list(REMOVE_ITEM LLVM_LINK_COMPONENTS ${c}) > + endif() > + endforeach() > + endif() > + endif() > + > target_link_libraries(${executable} LLVM) > endif() I think you need to also check for "all", at least in LLVM_DYLIB_COMPONENTS. LLVM_DYLIB_COMPONENTS defaults to "all". LLVM_LINK_COMPONENTS could, in theory, also have "all" in it; I'm not sure if anyone does that. I don't see any cases in LLVM, Clang, or LLDB, so the caveat on the TODO may be unnecessary.
(In reply to comment #8) > (In reply to comment #7) > > That looks reasonable to me. Does it resolve your issues? > > Unfortunately no joy here... > > -- Performing Test CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG > -- Performing Test CXX_SUPPORTS_NO_NESTED_ANON_TYPES_FLAG - Success > CMake Error at cmake/modules/LLVM-Config.cmake:206 (message): > Library `LLVM' not found in list of llvm libraries. > Call Stack (most recent call first): > cmake/modules/LLVM-Config.cmake:69 (llvm_map_components_to_libnames) > cmake/modules/LLVM-Config.cmake:62 (explicit_llvm_config) > cmake/modules/AddLLVM.cmake:662 (llvm_config) > cmake/modules/AddLLVM.cmake:695 (add_llvm_executable) > tools/opt/CMakeLists.txt:31 (add_llvm_tool) Don't suppose you added "LLVM" to LLVM_LINK_COMPONENTS in tools/opt/CMakeLists.txt? (You shouldn't, and if you did, that would cause this error)
(In reply to comment #10) > Don't suppose you added "LLVM" to LLVM_LINK_COMPONENTS in > tools/opt/CMakeLists.txt? (You shouldn't, and if you did, that would cause > this error) My mistake. Trying again with clean sources.
Created attachment 15796 [details] patch to strip LLVM_DYLIB_COMPONENTS out of link_components
Unfortunately, while that bootstraps, it doesn't seem to eliminate the linkages of the static LLVM components.
(In reply to comment #13) > Unfortunately, while that bootstraps, it doesn't seem to eliminate the > linkages of the static LLVM components. Your patch does not consider "all", as described in Comment 9. Are you explicitly setting LLVM_DYLIB_COMPONENTS?
CMakeCache.txt shows LLVM_DYLIB_COMPONENTS:STRING=all in my builds.
(In reply to comment #15) > CMakeCache.txt shows LLVM_DYLIB_COMPONENTS:STRING=all in my builds. Also I'm not explicitly setting LLVM_DYLIB_COMPONENTS.
(In reply to comment #16) > (In reply to comment #15) > > CMakeCache.txt shows LLVM_DYLIB_COMPONENTS:STRING=all in my builds. > Also I'm not explicitly setting LLVM_DYLIB_COMPONENTS. Please give this a try: if (TARGET LLVM) if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") set(link_components "") else() list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) endif() endif() endif()
(In reply to comment #17) That seems to be working as the build/stage1/tools/opt/CMakeFiles/opt.dir/link.txt now is properly reduced to... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -O3 -DNDEBUG -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -rdynamic CMakeFiles/opt.dir/AnalysisWrappers.cpp.o CMakeFiles/opt.dir/BreakpointPrinter.cpp.o CMakeFiles/opt.dir/GraphPrinters.cpp.o CMakeFiles/opt.dir/NewPMDriver.cpp.o CMakeFiles/opt.dir/PassPrinters.cpp.o CMakeFiles/opt.dir/PrintSCC.cpp.o CMakeFiles/opt.dir/opt.cpp.o -o ../../bin/opt ../../lib/libLLVM.dylib -Wl,-rpath,@executable_path/../lib
On the other hand, llc's linkage isn't getting pruned... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -rdynamic CMakeFiles/llc.dir/llc.cpp.o -o ../../bin/llc ../../lib/libLLVM.dylib ../../lib/libLLVMX86CodeGen.a ../../lib/libLLVMX86AsmPrinter.a ../../lib/libLLVMX86AsmParser.a ../../lib/libLLVMX86Desc.a ../../lib/libLLVMX86Info.a ../../lib/libLLVMX86Disassembler.a ../../lib/libLLVMPowerPCCodeGen.a ../../lib/libLLVMPowerPCAsmPrinter.a ../../lib/libLLVMPowerPCAsmParser.a ../../lib/libLLVMPowerPCDesc.a ../../lib/libLLVMPowerPCInfo.a ../../lib/libLLVMPowerPCDisassembler.a ../../lib/libLLVMARMCodeGen.a ../../lib/libLLVMARMAsmPrinter.a ../../lib/libLLVMARMAsmParser.a ../../lib/libLLVMARMDesc.a ../../lib/libLLVMARMInfo.a ../../lib/libLLVMARMDisassembler.a ../../lib/libLLVMAnalysis.a ../../lib/libLLVMAsmPrinter.a ../../lib/libLLVMCodeGen.a ../../lib/libLLVMCore.a ../../lib/libLLVMIRReader.a ../../lib/libLLVMMC.a ../../lib/libLLVMMIRParser.a ../../lib/libLLVMScalarOpts.a ../../lib/libLLVMSelectionDAG.a ../../lib/libLLVMSupport.a ../../lib/libLLVMTarget.a ../../lib/libLLVMTransformUtils.a ../../lib/libLLVMX86AsmPrinter.a ../../lib/libLLVMX86Utils.a ../../lib/libLLVMX86Info.a ../../lib/libLLVMPowerPCAsmPrinter.a ../../lib/libLLVMPowerPCInfo.a ../../lib/libLLVMDebugInfoCodeView.a ../../lib/libLLVMARMDesc.a ../../lib/libLLVMARMAsmPrinter.a ../../lib/libLLVMARMInfo.a ../../lib/libLLVMMCDisassembler.a ../../lib/libLLVMCodeGen.a ../../lib/libLLVMScalarOpts.a ../../lib/libLLVMInstCombine.a ../../lib/libLLVMBitWriter.a ../../lib/libLLVMInstrumentation.a ../../lib/libLLVMTransformUtils.a ../../lib/libLLVMProfileData.a ../../lib/libLLVMObject.a ../../lib/libLLVMMCParser.a ../../lib/libLLVMBitReader.a ../../lib/libLLVMTarget.a ../../lib/libLLVMAnalysis.a ../../lib/libLLVMMC.a ../../lib/libLLVMAsmParser.a ../../lib/libLLVMCore.a ../../lib/libLLVMSupport.a -lcurses -lpthread -lz -lm -Wl,-rpath,@executable_path/../lib
(In reply to comment #19) > On the other hand, llc's linkage isn't getting pruned... Sorry, I thought something looked off before, but didn't think much of it. You shouldn't have "if (TARGET LLVM)" in there, because some targets (e.g. llc) will be processed before the llvm-shlib target is defined. If LLVM_LINK_LLVM_DYLIB is set, the target will/must be defined; no need for another check.
Created attachment 15797 [details] patch to strip LLVM_DYLIB_COMPONENTS out of link_components
A build using -DLLVM_LINK_LLVM_DYLIB:BOOL=ON and... Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259464) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif() eliminates the unwanted static linkages and produces almost pristine test suite results... Testing Time: 192.00s ******************** Failing Tests (3): LLVM :: tools/lto/hide-linkonce-odr.ll LLVM :: tools/lto/opt-level.ll LLVM :: tools/sanstats/elf.test Expected Passes : 11743 Expected Failures : 51 Unsupported Tests : 3847 Unexpected Failures: 3 Testing Time: 198.22s ******************** Failing Tests (7): Clang :: CodeGen/split-debug-filename.c Clang :: CodeGen/thinlto_backend.c Clang :: CodeGenCXX/PR20038.cpp Clang :: CodeGenCXX/temp-order.cpp Clang :: Frontend/backend-diagnostic.c Clang :: Misc/backend-stack-frame-diagnostics-fallback.cpp Clang :: Misc/backend-stack-frame-diagnostics.cpp Expected Passes : 8834 Expected Failures : 16 Unsupported Tests : 111 Unexpected Failures: 7
The additional change of... Index: lib/CMakeLists.txt =================================================================== --- lib/CMakeLists.txt (revision 259450) +++ lib/CMakeLists.txt (working copy) @@ -77,6 +77,13 @@ 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 allows LLVMPolly.so to be properly linked against libLLVM.dylib and produces clean test suite results... Testing Time: 8.86s Expected Passes : 574 Expected Failures : 17 Unsupported Tests : 7 [100%] Built target check-polly
Even more impressive, applying the patch from comment 22 and the polly build fix from comment 23 to the current 3.8 branch sources, properly builds the llvm executables against libLLVM.dylib with -DLLVM_LINK_LLVM_DYLIB:BOOL=ON as well. The resulting test suite results are even cleaner there... Expected Passes : 11647 Expected Failures : 51 Unsupported Tests : 3808 [100%] Built target check-llvm Expected Passes : 8760 Expected Failures : 16 Unsupported Tests : 101 [100%] Built target check-clang Expected Passes : 231 [100%] Built target check-clang-tools Expected Passes : 567 Expected Failures : 17 Unsupported Tests : 7 [100%] Built target check-polly Expected Passes : 5004 Expected Failures : 12 Unsupported Tests : 10 [100%] Built target check-libcxx Expected Passes : 67 [100%] Built target check-libomp as tested on x86_64-apple-darwin15.
With the patch from comment 22 on trunk I'm seeing some really unsetting test failures (some of which Jack listed in the comment). For example: CodeGenCXX/temp-order.cpp fails with the error: clang (LLVM option parsing): Unknown command line argument '-inline-threshold=1024'. Try: 'clang (LLVM option parsing) -help' clang (LLVM option parsing): Did you mean '-print-options=1024'? This failure is in the LLVM command line option parsing. Which indicates to me that the clang linking against the dynamic library isn't running static constructors correctly, or the static constructors are being stripped from the dylib. Neither of which are really good things.
(In reply to comment #25) > With the patch from comment 22 on trunk I'm seeing some really unsetting > test failures (some of which Jack listed in the comment). For example: > > CodeGenCXX/temp-order.cpp fails with the error: > > clang (LLVM option parsing): Unknown command line argument > '-inline-threshold=1024'. Try: 'clang (LLVM option parsing) -help' > clang (LLVM option parsing): Did you mean '-print-options=1024'? > > This failure is in the LLVM command line option parsing. Which indicates to > me that the clang linking against the dynamic library isn't running static > constructors correctly, or the static constructors are being stripped from > the dylib. Neither of which are really good things. Looking at my trunk build with the comment 22 patch applied, I see for /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/tools/clang/tools/driver/CMakeFiles/clang.dir/link.txt... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/cc-st2-clang++ -std=c++11 -stdlib=libc++ -cxx-isystem /sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../libcxx-3.9.0.src/include -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wnon-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -O3 -DNDEBUG -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/src/fink.build/llvm39-3.9.0-1/llvm-3.9.0.src/../build/last-libcxx/lib -L/sw/lib -rdynamic CMakeFiles/clang.dir/driver.cpp.o CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o -o ../../../../bin/clang-3.9 ../../../../lib/libLLVM.dylib ../../../../lib/libclangBasic.a ../../../../lib/libclangCodeGen.a ../../../../lib/libclangDriver.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangFrontendTool.a -Wl,-sectcreate,__TEXT,__info_plist,/sw/src/fink.build/llvm39-3.9.0-1/build/stage3/tools/clang/tools/driver/Info.plist ../../../../lib/libclangCodeGen.a ../../../../lib/libLLVMBitWriter.a ../../../../lib/libLLVMipo.a ../../../../lib/libLLVMVectorize.a ../../../../lib/libLLVMIRReader.a ../../../../lib/libLLVMAsmParser.a ../../../../lib/libLLVMInstrumentation.a ../../../../lib/libLLVMLinker.a ../../../../lib/libLLVMObjCARCOpts.a ../../../../lib/libLLVMProfileData.a ../../../../lib/libLLVMObject.a ../../../../lib/libLLVMScalarOpts.a ../../../../lib/libLLVMInstCombine.a ../../../../lib/libLLVMTarget.a ../../../../lib/libLLVMTransformUtils.a ../../../../lib/libLLVMAnalysis.a ../../../../lib/libclangRewriteFrontend.a ../../../../lib/libclangARCMigrate.a ../../../../lib/libclangStaticAnalyzerFrontend.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangDriver.a ../../../../lib/libLLVMOption.a ../../../../lib/libclangParse.a ../../../../lib/libLLVMMCParser.a ../../../../lib/libclangSerialization.a ../../../../lib/libLLVMBitReader.a ../../../../lib/libclangSema.a ../../../../lib/libclangEdit.a ../../../../lib/libclangStaticAnalyzerCheckers.a ../../../../lib/libclangStaticAnalyzerCore.a ../../../../lib/libclangAnalysis.a ../../../../lib/libclangAST.a ../../../../lib/libclangRewrite.a ../../../../lib/libclangLex.a ../../../../lib/libclangBasic.a ../../../../lib/libLLVMCore.a ../../../../lib/libLLVMMC.a ../../../../lib/libLLVMSupport.a -lcurses -lpthread -lz -lm -Wl,-rpath,@executable_path/../lib Oddly, I don't see that failure on 3.8 branch with the patch from comment 22 applied despite a similar linkage... /sw/src/fink.build/llvm38-3.8.0-1/opt-bin/cc-st2-clang++ -std=c++11 -stdlib=libc++ -cxx-isystem /sw/src/fink.build/llvm38-3.8.0-1/llvm-3.8.0.src/../libcxx-3.8.0.src/include -L/sw/src/fink.build/llvm38-3.8.0-1/llvm-3.8.0.src/../build/last-libcxx/lib -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wnon-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -O3 -DNDEBUG -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/src/fink.build/llvm38-3.8.0-1/llvm-3.8.0.src/../build/last-libcxx/lib -L/sw/lib -rdynamic CMakeFiles/clang.dir/driver.cpp.o CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o -o ../../../../bin/clang-3.8 ../../../../lib/libLLVM.dylib ../../../../lib/libclangBasic.a ../../../../lib/libclangCodeGen.a ../../../../lib/libclangDriver.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangFrontendTool.a -Wl,-sectcreate,__TEXT,__info_plist,/sw/src/fink.build/llvm38-3.8.0-1/build/stage3/tools/clang/tools/driver/Info.plist ../../../../lib/libclangCodeGen.a ../../../../lib/libLLVMBitWriter.a ../../../../lib/libLLVMipo.a ../../../../lib/libLLVMVectorize.a ../../../../lib/libLLVMIRReader.a ../../../../lib/libLLVMAsmParser.a ../../../../lib/libLLVMInstrumentation.a ../../../../lib/libLLVMLinker.a ../../../../lib/libLLVMObjCARCOpts.a ../../../../lib/libLLVMProfileData.a ../../../../lib/libLLVMObject.a ../../../../lib/libLLVMScalarOpts.a ../../../../lib/libLLVMInstCombine.a ../../../../lib/libLLVMTarget.a ../../../../lib/libLLVMTransformUtils.a ../../../../lib/libLLVMAnalysis.a ../../../../lib/libclangRewriteFrontend.a ../../../../lib/libclangARCMigrate.a ../../../../lib/libclangStaticAnalyzerFrontend.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangDriver.a ../../../../lib/libLLVMOption.a ../../../../lib/libclangParse.a ../../../../lib/libLLVMMCParser.a ../../../../lib/libclangSerialization.a ../../../../lib/libLLVMBitReader.a ../../../../lib/libclangSema.a ../../../../lib/libclangEdit.a ../../../../lib/libclangStaticAnalyzerCheckers.a ../../../../lib/libclangStaticAnalyzerCore.a ../../../../lib/libclangAnalysis.a ../../../../lib/libclangAST.a ../../../../lib/libclangRewrite.a ../../../../lib/libclangLex.a ../../../../lib/libclangBasic.a ../../../../lib/libLLVMCore.a ../../../../lib/libLLVMMC.a ../../../../lib/libLLVMSupport.a -lcurses -lpthread -lz -lm -Wl,-rpath,@executable_path/../lib
(In reply to comment #26) I admit I was tad concerned about the usage of... list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) Doesn't that require those components to be in a specific order? Shouldn't the else section be... if (DEFINED LLVM_DYLIB_COMPONENTS) foreach(c in ${LLVM_LINK_COMPONENTS}) list(FIND LLVM_DYLIB_COMPONENTS ${c} C_IDX) if (NOT C_IDX EQUAL -1) list(REMOVE_ITEM LLVM_LINK_COMPONENTS ${c}) endif() endforeach() endif() to handle any particular ordering for the non-'all' case?
(In reply to comment #25) > With the patch from comment 22 on trunk I'm seeing some really unsetting > test failures (some of which Jack listed in the comment). For example: > > CodeGenCXX/temp-order.cpp fails with the error: > > clang (LLVM option parsing): Unknown command line argument > '-inline-threshold=1024'. Try: 'clang (LLVM option parsing) -help' > clang (LLVM option parsing): Did you mean '-print-options=1024'? > > This failure is in the LLVM command line option parsing. Which indicates to > me that the clang linking against the dynamic library isn't running static > constructors correctly, or the static constructors are being stripped from > the dylib. Neither of which are really good things. In case it helps with diagnosis: I don't get any errors with either "check-llvm" or "check-clang" with the patch applied. I'm on Linux x86-64.
Where is USE_SHARED set? I don't see that in CMakeCache.txt. Is it possible that is being unset during the clang build? I say that because a quick and dirty test with... Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259573) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,19 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + set(link_components "") + endif + else (DEFINED link_components) + set(link_components "") + endif() target_link_libraries(${executable} LLVM) endif() failed to clear out the static linkages in /sw/src/fink.build/llvm39-3.9.0-1/build/stage1/tools/clang/tools/driver/CMakeFiles/clang.dir/link.txt. So either the clang build isn't really using the llvm_config executable macro or USE_SHARED is being unset, no?
Jack, your last comment hit on the problem. I think this is actually done at the wrong layer in the CMake calls. Rather than doing it in llvm_config() we need to do it in llvm_map_components_to_libnames(). Not all code paths call llvm_config(), some call llvm_map_components_to_libnames() directly.
(In reply to comment #30) > Jack, your last comment hit on the problem. > > I think this is actually done at the wrong layer in the CMake calls. Rather > than doing it in llvm_config() we need to do it in > llvm_map_components_to_libnames(). > > Not all code paths call llvm_config(), some call > llvm_map_components_to_libnames() directly. I think I've tracked it down: the logic in llvm_add_library for deciding whether to add libLLVM vs. components libs as link libraries isn't quite right. It says that if the library is static, then to use the component libs. Those are then pulled in as transitive dependencies on the clang target. I'm not sure off hand how to address that. Perhaps something like: if it's not in the LLVM core library set, always use libLLVM (if LLVM_LINK_LLVM_DYLIB is set).
(In reply to comment #31) > (In reply to comment #30) > > Jack, your last comment hit on the problem. > > > > I think this is actually done at the wrong layer in the CMake calls. Rather > > than doing it in llvm_config() we need to do it in > > llvm_map_components_to_libnames(). > > > > Not all code paths call llvm_config(), some call > > llvm_map_components_to_libnames() directly. > > I think I've tracked it down: the logic in llvm_add_library for deciding > whether to add libLLVM vs. components libs as link libraries isn't quite > right. It says that if the library is static, then to use the component > libs. Those are then pulled in as transitive dependencies on the clang > target. > > I'm not sure off hand how to address that. Perhaps something like: if it's > not in the LLVM core library set, always use libLLVM (if > LLVM_LINK_LLVM_DYLIB is set). What would that look like as a test patch?
Does... Index: AddLLVM.cmake =================================================================== --- AddLLVM.cmake (revision 259614) +++ AddLLVM.cmake (working copy) @@ -379,7 +379,7 @@ set_target_properties(${obj_name} PROPERTIES FOLDER "Object Libraries") endif() - if(ARG_SHARED AND ARG_STATIC) + if(ARG_SHARED AND ARG_STATIC AND NOT LLVM_LINK_LLVM_DYLIB) # static set(name_static "${name}_static") if(ARG_OUTPUT_NAME) make sense?
(In reply to comment #32) > (In reply to comment #31) > > (In reply to comment #30) > > > Jack, your last comment hit on the problem. > > > > > > I think this is actually done at the wrong layer in the CMake calls. Rather > > > than doing it in llvm_config() we need to do it in > > > llvm_map_components_to_libnames(). > > > > > > Not all code paths call llvm_config(), some call > > > llvm_map_components_to_libnames() directly. > > > > I think I've tracked it down: the logic in llvm_add_library for deciding > > whether to add libLLVM vs. components libs as link libraries isn't quite > > right. It says that if the library is static, then to use the component > > libs. Those are then pulled in as transitive dependencies on the clang > > target. > > > > I'm not sure off hand how to address that. Perhaps something like: if it's > > not in the LLVM core library set, always use libLLVM (if > > LLVM_LINK_LLVM_DYLIB is set). > > What would that look like as a test patch? Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259475) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,7 +475,7 @@ # property has been set to an empty value. get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB AND DEFINED LLVM_LINK_COMPONENTS) set(llvm_libs LLVM) else() llvm_map_components_to_libnames(llvm_libs
This appears to be doing the right thing... Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259614) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -477,7 +477,7 @@ if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) set(llvm_libs LLVM) - else() + elseif(NOT LLVM_LINK_LLVM_DYLIB) llvm_map_components_to_libnames(llvm_libs ${ARG_LINK_COMPONENTS} ${LLVM_LINK_COMPONENTS} Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259614) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif()
(In reply to comment #35) After this bootstrap completes, I'll try another build with just... Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259614) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -477,7 +477,7 @@ if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) set(llvm_libs LLVM) - else() + elseif(NOT LLVM_LINK_LLVM_DYLIB) llvm_map_components_to_libnames(llvm_libs ${ARG_LINK_COMPONENTS} ${LLVM_LINK_COMPONENTS}
(In reply to comment #36) > (In reply to comment #35) > > After this bootstrap completes, I'll try another build with just... > > Index: cmake/modules/AddLLVM.cmake > =================================================================== > --- cmake/modules/AddLLVM.cmake (revision 259614) > +++ cmake/modules/AddLLVM.cmake (working copy) > @@ -477,7 +477,7 @@ > > if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > set(llvm_libs LLVM) > - else() > + elseif(NOT LLVM_LINK_LLVM_DYLIB) > llvm_map_components_to_libnames(llvm_libs > ${ARG_LINK_COMPONENTS} > ${LLVM_LINK_COMPONENTS} Okay. we need the both bits from the patch in comment 34.
Created attachment 15805 [details] expanded patch to strip LLVM_DYLIB_COMPONENTS out of link_components
Perhaps the new section of the patch would be more correct as... Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259614) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -477,7 +477,7 @@ if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) set(llvm_libs LLVM) - else() + elseif(NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) llvm_map_components_to_libnames(llvm_libs ${ARG_LINK_COMPONENTS} ${LLVM_LINK_COMPONENTS}
Created attachment 15806 [details] revised expanded patch to strip LLVM_DYLIB_COMPONENTS out of link_components
Using the current permutation of this patch... Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259614) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -477,7 +477,7 @@ if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) set(llvm_libs LLVM) - else() + elseif(NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) llvm_map_components_to_libnames(llvm_libs ${ARG_LINK_COMPONENTS} ${LLVM_LINK_COMPONENTS} Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259614) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif() and checking the stage1 build directory here with... find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep libLLVMSupport still shows a number of instances of that static lib leaking into the linkages for cmake build with -DLLVM_LINK_LLVM_DYLIB:BOOL=ON... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -Wl,-dead_strip CMakeFiles/ClangApplyReplacementsTests.dir/ReformattingTest.cpp.o -o ClangApplyReplacementsTests ../../../../../../lib/libLLVM.dylib ../../../../../../lib/libgtest.a ../../../../../../lib/libgtest_main.a ../../../../../../lib/libLLVMSupport.a ../../../../../../lib/libclangApplyReplacements.a ../../../../../../lib/libclangToolingCore.a ../../../../../../lib/libgtest.a ../../../../../../lib/libLLVMSupport.a -lcurses -lpthread -lz -lm ../../../../../../lib/libclangAST.a ../../../../../../lib/libclangRewrite.a ../../../../../../lib/libclangLex.a ../../../../../../lib/libclangBasic.a -Wl,-rpath,@executable_path/../lib This leakage of libLLVMSupport.a seems to be limited to *Test executables.
As far as I can tell, the last remaining issue seems to be these stray instances of libLLVMSupport.a that are appended by cmake whenever libgtest.a appears. I am not having much luck identifying the code emitting it. A test patch of... Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259623) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -477,7 +477,7 @@ if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) set(llvm_libs LLVM) - else() + elseif(NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) llvm_map_components_to_libnames(llvm_libs ${ARG_LINK_COMPONENTS} ${LLVM_LINK_COMPONENTS} @@ -885,11 +885,18 @@ add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO ${ARGN}) set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir}) - target_link_libraries(${test_name} - gtest - gtest_main - LLVMSupport # gtest needs it for raw_ostream. + if (LLVM_LINK_LLVM_DYLIB) + target_link_libraries(${test_name} + gtest + gtest_main ) + else () + target_link_libraries(${test_name} + gtest + gtest_main + LLVMSupport # gtest needs it for raw_ostream. + ) + endif () add_dependencies(${test_suite} ${test_name}) get_target_property(test_suite_folder ${test_suite} FOLDER) Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259623) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif() doesn't suppress those. Also explicitly commenting out the instance of LLVMSupport doesn't suppress its appearance on the linkage so it seemed to be emitting somewhere else.
I believe these stray instances of libLLVMSupport.a are coming in from the instances of... set(LLVM_LINK_COMPONENTS Support ) in the cfe unittests CMakeLists.txt files such as AST/CMakeLists.txt.
(In reply to comment #40) > Created attachment 15806 [details] > revised expanded patch to strip LLVM_DYLIB_COMPONENTS out of link_components The currently posted version of the proposed patch (which doesn't address the gtest/LLVMSupport linkage issue) applied to trunk still shows the following regressions in llvm... ******************** Failing Tests (3): LLVM :: tools/lto/hide-linkonce-odr.ll LLVM :: tools/lto/opt-level.ll LLVM :: tools/sanstats/elf.test Expected Passes : 11761 Expected Failures : 52 Unsupported Tests : 3850 Unexpected Failures: 3 however the clang test suite is now clean Expected Passes : 8849 Expected Failures : 16 Unsupported Tests : 113 [100%] Built target check-clang Expected Passes : 238 [100%] Built target check-clang-tools as are the polly/libc++/libomp test suites. So we are down to just 3 regressions in llvm.
The first two failures in llvm... LLVM :: tools/lto/hide-linkonce-odr.ll LLVM :: tools/lto/opt-level.ll appear to be due to... ld: could not process llvm bitcode object file, because /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./lib/libLTO.dylib could not be loaded file '/sw/src/fink.build/llvm39-3.9.0-1/build/stage3/test/tools/lto/Output/opt-level.ll.tmp.o' for architecture x86_64 which makes me wonder if trunk now needs a newer Xcode than 7.2? for the latest libLTO changes? The third failure in llvm... LLVM :: tools/sanstats/elf.test appears as... /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/./bin/sanstats: /sw/src/fink.build/llvm39-3.9.0-1/build/stage3/test/tools/sanstats/Output/elf.test.tmp.stats: short read
Testing here on x86_64-apple-darwin15 using current trunk, the following patch, currently attached in https://llvm.org/bugs/show_bug.cgi?id=26393#c40, eliminates all of the clang test suite regressions on trunk by eliminating the LLVM static lib linkages on its tools... ndex: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259614) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -477,7 +477,7 @@ if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) set(llvm_libs LLVM) - else() + elseif(NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) llvm_map_components_to_libnames(llvm_libs ${ARG_LINK_COMPONENTS} ${LLVM_LINK_COMPONENTS} Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259614) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif() The only remaining linkages of static LLVM components in executables are those of libLLVMSupport.a associated with the libgtest.a linkages. Since those aren't currently causing regressions, adding a TODO entry in cmake/modules/AddLLVM.cmake to either suppress the LLVMSupport on LLVM_LINK_LLVM_DYLIB or suppress LLVM for these particular test linkages should suffice.
(In reply to comment #46) > Testing here on x86_64-apple-darwin15 using current trunk, the > following patch, currently attached in > https://llvm.org/bugs/show_bug.cgi?id=26393#c40, eliminates all of the clang > test suite regressions on > trunk by eliminating the LLVM static lib linkages on its tools... > > ndex: cmake/modules/AddLLVM.cmake > =================================================================== > --- cmake/modules/AddLLVM.cmake (revision 259614) > +++ cmake/modules/AddLLVM.cmake (working copy) > @@ -477,7 +477,7 @@ > > if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > set(llvm_libs LLVM) > - else() > + elseif(NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) This change isn't quite right. Doing this means that we will add the component libraries as dependencies of static libraries *only* if LLVM_LINK_LLVM_DYLIB is disabled. If you have LLVM_LINK_LLVM_DYLIB enabled, dependent libraries will not be added to the interface of other static libraries. e.g. If I require libclangBasic, I should also get libLLVMCore, libLLVMMC, and libLLVMSupport according to LLVM_LINK_COMPONENTS in clang/lib/Basic/CMakeLists.txt. That won't happen with this change. Can you please test my suggestion in Comment 34? The ARG_STATIC in the "if(...)" above was my misguided approach to breaking the circular dependency of core component libraries and libLLVM. Instead of doing this, we can check if LLVM_LINK_COMPONENTS is set; only if it is set do we link to libLLVM. It is never set for the core component libraries. > llvm_map_components_to_libnames(llvm_libs > ${ARG_LINK_COMPONENTS} > ${LLVM_LINK_COMPONENTS} > Index: cmake/modules/LLVM-Config.cmake > =================================================================== > --- cmake/modules/LLVM-Config.cmake (revision 259614) > +++ cmake/modules/LLVM-Config.cmake (working copy) > @@ -40,10 +40,17 @@ > # done in case libLLVM does not contain all of the components > # the target requires. > # > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > # To do this, we need special handling for "all", since that > # may imply linking to libraries that are not included in > # libLLVM. > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > + set(link_components "") > + else() > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > + endif() > + endif() > target_link_libraries(${executable} LLVM) > endif() > > The only remaining linkages of static LLVM components in executables are > those of libLLVMSupport.a associated with the libgtest.a linkages. Since > those aren't currently causing regressions, adding a TODO entry in > cmake/modules/AddLLVM.cmake to either suppress the LLVMSupport on > LLVM_LINK_LLVM_DYLIB or suppress LLVM for these particular test linkages > should suffice.
(In reply to comment #47) > (In reply to comment #46) > > Testing here on x86_64-apple-darwin15 using current trunk, the > > following patch, currently attached in > > https://llvm.org/bugs/show_bug.cgi?id=26393#c40, eliminates all of the clang > > test suite regressions on > > trunk by eliminating the LLVM static lib linkages on its tools... > > > > ndex: cmake/modules/AddLLVM.cmake > > =================================================================== > > --- cmake/modules/AddLLVM.cmake (revision 259614) > > +++ cmake/modules/AddLLVM.cmake (working copy) > > @@ -477,7 +477,7 @@ > > > > if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > > set(llvm_libs LLVM) > > - else() > > + elseif(NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > > This change isn't quite right. Doing this means that we will add the > component libraries as dependencies of static libraries *only* if > LLVM_LINK_LLVM_DYLIB is disabled. If you have LLVM_LINK_LLVM_DYLIB enabled, > dependent libraries will not be added to the interface of other static > libraries. > > e.g. If I require libclangBasic, I should also get libLLVMCore, libLLVMMC, > and libLLVMSupport according to LLVM_LINK_COMPONENTS in > clang/lib/Basic/CMakeLists.txt. That won't happen with this change. > > Can you please test my suggestion in Comment 34? The ARG_STATIC in the > "if(...)" above was my misguided approach to breaking the circular > dependency of core component libraries and libLLVM. Instead of doing this, > we can check if LLVM_LINK_COMPONENTS is set; only if it is set do we link to > libLLVM. It is never set for the core component libraries. The proposed change produces the same pruning of LLVM_LINK_COMPONENTS as the previous patch with the one change. My previous proposed patch produced... $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 38 and your proposed modification produces... $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 39 So we have recovered one instance of libLLVMSupport being linked with this change. $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep libgtest | grep -c libLLVMSupport 37 shows that all but two of these residual linkages against libLLVMSupport are due to the gtest linkages. $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -v libgtest | grep libLLVMSupport shows that the recovered linkage on libLLVMSupport is in the creation of libclang.dylib which makes sense as this change was required to handled clang's linkages on LLVM components. So we have restrained the pruning by one instance of libLLVMSupport. The > > > llvm_map_components_to_libnames(llvm_libs > > ${ARG_LINK_COMPONENTS} > > ${LLVM_LINK_COMPONENTS} > > Index: cmake/modules/LLVM-Config.cmake > > =================================================================== > > --- cmake/modules/LLVM-Config.cmake (revision 259614) > > +++ cmake/modules/LLVM-Config.cmake (working copy) > > @@ -40,10 +40,17 @@ > > # done in case libLLVM does not contain all of the components > > # the target requires. > > # > > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > > # To do this, we need special handling for "all", since that > > # may imply linking to libraries that are not included in > > # libLLVM. > > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > > + set(link_components "") > > + else() > > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > > + endif() > > + endif() > > target_link_libraries(${executable} LLVM) > > endif() > > > > The only remaining linkages of static LLVM components in executables are > > those of libLLVMSupport.a associated with the libgtest.a linkages. Since > > those aren't currently causing regressions, adding a TODO entry in > > cmake/modules/AddLLVM.cmake to either suppress the LLVMSupport on > > LLVM_LINK_LLVM_DYLIB or suppress LLVM for these particular test linkages > > should suffice.
Created attachment 15819 [details] corrected patch to strip LLVM_DYLIB_COMPONENTS out of link_components
(In reply to comment #48) > (In reply to comment #47) > > (In reply to comment #46) > > > Testing here on x86_64-apple-darwin15 using current trunk, the > > > following patch, currently attached in > > > https://llvm.org/bugs/show_bug.cgi?id=26393#c40, eliminates all of the clang > > > test suite regressions on > > > trunk by eliminating the LLVM static lib linkages on its tools... > > > > > > ndex: cmake/modules/AddLLVM.cmake > > > =================================================================== > > > --- cmake/modules/AddLLVM.cmake (revision 259614) > > > +++ cmake/modules/AddLLVM.cmake (working copy) > > > @@ -477,7 +477,7 @@ > > > > > > if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > > > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > > > set(llvm_libs LLVM) > > > - else() > > > + elseif(NOT LLVM_LINK_LLVM_DYLIB OR ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > > > > This change isn't quite right. Doing this means that we will add the > > component libraries as dependencies of static libraries *only* if > > LLVM_LINK_LLVM_DYLIB is disabled. If you have LLVM_LINK_LLVM_DYLIB enabled, > > dependent libraries will not be added to the interface of other static > > libraries. > > > > e.g. If I require libclangBasic, I should also get libLLVMCore, libLLVMMC, > > and libLLVMSupport according to LLVM_LINK_COMPONENTS in > > clang/lib/Basic/CMakeLists.txt. That won't happen with this change. > > > > Can you please test my suggestion in Comment 34? The ARG_STATIC in the > > "if(...)" above was my misguided approach to breaking the circular > > dependency of core component libraries and libLLVM. Instead of doing this, > > we can check if LLVM_LINK_COMPONENTS is set; only if it is set do we link to > > libLLVM. It is never set for the core component libraries. > > The proposed change produces the same pruning of LLVM_LINK_COMPONENTS as the > previous patch with the one change. My previous proposed patch produced... > > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > -c libLLVMSupport > 38 > > and your proposed modification produces... > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > -c libLLVMSupport > 39 > > So we have recovered one instance of libLLVMSupport being linked with this > change. > > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > libgtest | grep -c libLLVMSupport > 37 > > shows that all but two of these residual linkages against libLLVMSupport are > due to the gtest linkages. > > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > -v libgtest | grep libLLVMSupport > > shows that the recovered linkage on libLLVMSupport is in the creation of > libclang.dylib which makes sense as this change was required to handled > clang's linkages on LLVM components. Actually, I think that's a problem, and I see why now. My suggestion was incomplete: it needs to check for both ARG_LINK_COMPONENTS and LLVM_LINK_COMPONENTS. I'd just change it to: --- cmake/modules/AddLLVM.cmake (revision 259475) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,13 +475,15 @@ # property has been set to an empty value. get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) - set(llvm_libs LLVM) - else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) + set(llvm_libs LLVM) + else() + llvm_map_components_to_libnames(llvm_libs + ${ARG_LINK_COMPONENTS} + ${LLVM_LINK_COMPONENTS} + ) + endif() endif() > So we have restrained the pruning by one instance of libLLVMSupport. The > > > > > llvm_map_components_to_libnames(llvm_libs > > > ${ARG_LINK_COMPONENTS} > > > ${LLVM_LINK_COMPONENTS} > > > Index: cmake/modules/LLVM-Config.cmake > > > =================================================================== > > > --- cmake/modules/LLVM-Config.cmake (revision 259614) > > > +++ cmake/modules/LLVM-Config.cmake (working copy) > > > @@ -40,10 +40,17 @@ > > > # done in case libLLVM does not contain all of the components > > > # the target requires. > > > # > > > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > > > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > > > # To do this, we need special handling for "all", since that > > > # may imply linking to libraries that are not included in > > > # libLLVM. > > > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > > > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > > > + set(link_components "") > > > + else() > > > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > > > + endif() > > > + endif() > > > target_link_libraries(${executable} LLVM) > > > endif() > > > > > > The only remaining linkages of static LLVM components in executables are > > > those of libLLVMSupport.a associated with the libgtest.a linkages. Since > > > those aren't currently causing regressions, adding a TODO entry in > > > cmake/modules/AddLLVM.cmake to either suppress the LLVMSupport on > > > LLVM_LINK_LLVM_DYLIB or suppress LLVM for these particular test linkages > > > should suffice.
(In reply to comment #50) Okay. The extra linkage on libLLVMSupport for libclang.dylib no now gone with the revised patch. We still have 37 instances of linkages of libLLVMSupport.a with libLLVM.dylib libgtest.a . Is it possible that the similar adjustments would solve that for the conditional in... Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259743) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif()
Created attachment 15820 [details] recorrected patch to strip LLVM_DYLIB_COMPONENTS out of link_components
(In reply to comment #51) > (In reply to comment #50) > > Okay. The extra linkage on libLLVMSupport for libclang.dylib no now gone > with the revised patch. > > We still have 37 instances of linkages of libLLVMSupport.a with > libLLVM.dylib libgtest.a . Is it possible that the similar adjustments would > solve that for the conditional in... No, I've just had a look and this is due to a few things: - utils/unittest/CMakeLists.txt - utils/unittest/UnitTestMain/CMakeLists.txt both specify LLVMSupport as a LINK_LIB. These could use LINK_COMPONENTS instead. and - cmake/modules/AddLLVM.cmake adds LLVMSupport as a library dependency in the "add_unittest" function. This shouldn't be added at all, because the dependencies should be added transitively. Having said that, I've made those changes and they're still in there. I don't have time to keep investigating this right now -- I think this should be tackled separately, since it's not causing any issues right now. > > Index: cmake/modules/LLVM-Config.cmake > =================================================================== > --- cmake/modules/LLVM-Config.cmake (revision 259743) > +++ cmake/modules/LLVM-Config.cmake (working copy) > @@ -40,10 +40,17 @@ > # done in case libLLVM does not contain all of the components > # the target requires. > # > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > # To do this, we need special handling for "all", since that > # may imply linking to libraries that are not included in > # libLLVM. > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > + set(link_components "") > + else() > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > + endif() > + endif() > target_link_libraries(${executable} LLVM) > endif()
(In reply to comment #53) So the current permutation is okay for trunk (its already in stage 2 of the bootstrap here)? Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259743) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,13 +475,15 @@ # property has been set to an empty value. get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) - set(llvm_libs LLVM) - else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) + set(llvm_libs LLVM) + else() + llvm_map_components_to_libnames(llvm_libs + ${ARG_LINK_COMPONENTS} + ${LLVM_LINK_COMPONENTS} + ) + endif() endif() if(CMAKE_VERSION VERSION_LESS 2.8.12) Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259743) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif()
(In reply to comment #54) > (In reply to comment #53) > > So the current permutation is okay for trunk (its already in stage 2 of the > bootstrap here)? LGTM. I think it's ready to commit. > Index: cmake/modules/AddLLVM.cmake > =================================================================== > --- cmake/modules/AddLLVM.cmake (revision 259743) > +++ cmake/modules/AddLLVM.cmake (working copy) > @@ -475,13 +475,15 @@ > # property has been set to an empty value. > get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) > > - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > - set(llvm_libs LLVM) > - else() > - llvm_map_components_to_libnames(llvm_libs > - ${ARG_LINK_COMPONENTS} > - ${LLVM_LINK_COMPONENTS} > - ) > + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) > + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > + set(llvm_libs LLVM) > + else() > + llvm_map_components_to_libnames(llvm_libs > + ${ARG_LINK_COMPONENTS} > + ${LLVM_LINK_COMPONENTS} > + ) > + endif() > endif() > > if(CMAKE_VERSION VERSION_LESS 2.8.12) > Index: cmake/modules/LLVM-Config.cmake > =================================================================== > --- cmake/modules/LLVM-Config.cmake (revision 259743) > +++ cmake/modules/LLVM-Config.cmake (working copy) > @@ -40,10 +40,17 @@ > # done in case libLLVM does not contain all of the components > # the target requires. > # > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > # To do this, we need special handling for "all", since that > # may imply linking to libraries that are not included in > # libLLVM. > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > + set(link_components "") > + else() > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > + endif() > + endif() > target_link_libraries(${executable} LLVM) > endif()
(In reply to comment #55) > (In reply to comment #54) > > (In reply to comment #53) > > > > So the current permutation is okay for trunk (its already in stage 2 of the > > bootstrap here)? > > LGTM. I think it's ready to commit. > Unfortunately, there seems to be a glitch in this latest revision of the patch which I noticed due to regressions... Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.AddsTargetAndMode Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingMode Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingTarget Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.PathIgnored Expected Passes : 8869 Expected Failures : 16 Unsupported Tests : 113 Unexpected Failures: 4 The previous patch from comment #46, which produced no clang test suite regressions, has the compiler linked with properly libLLVM.dylib in the front of the linkage... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -rdynamic CMakeFiles/clang.dir/driver.cpp.o CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o -o ../../../../bin/clang-3.9 ../../../../lib/libLLVM.dylib ../../../../lib/libclangBasic.a ../../../../lib/libclangCodeGen.a ../../../../lib/libclangDriver.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangFrontendTool.a -Wl,-sectcreate,__TEXT,__info_plist,/sw/src/fink.build/llvm39-3.9.0-1/build/stage1/tools/clang/tools/driver/Info.plist ../../../../lib/libclangCodeGen.a ../../../../lib/libclangRewriteFrontend.a ../../../../lib/libclangARCMigrate.a ../../../../lib/libclangStaticAnalyzerFrontend.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangDriver.a ../../../../lib/libclangParse.a ../../../../lib/libclangSerialization.a ../../../../lib/libclangSema.a ../../../../lib/libclangEdit.a ../../../../lib/libclangStaticAnalyzerCheckers.a ../../../../lib/libclangStaticAnalyzerCore.a ../../../../lib/libclangAnalysis.a ../../../../lib/libclangAST.a ../../../../lib/libclangRewrite.a ../../../../lib/libclangLex.a ../../../../lib/libclangBasic.a -Wl,-rpath,@executable_path/../lib whereas this new version of the patch, using the change from comment #50, has the linkage of libLLVN.dylib incorrectly moved to the end of the linkage as... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -rdynamic CMakeFiles/clang.dir/driver.cpp.o CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o -o ../../../../bin/clang-3.9 ../../../../lib/libclangBasic.a ../../../../lib/libclangCodeGen.a ../../../../lib/libclangDriver.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangFrontendTool.a -Wl,-sectcreate,__TEXT,__info_plist,/sw/src/fink.build/llvm39-3.9.0-1/build/stage1/tools/clang/tools/driver/Info.plist ../../../../lib/libclangCodeGen.a ../../../../lib/libclangRewriteFrontend.a ../../../../lib/libclangARCMigrate.a ../../../../lib/libclangStaticAnalyzerFrontend.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangDriver.a ../../../../lib/libclangParse.a ../../../../lib/libclangSerialization.a ../../../../lib/libclangSema.a ../../../../lib/libclangEdit.a ../../../../lib/libclangStaticAnalyzerCheckers.a ../../../../lib/libclangStaticAnalyzerCore.a ../../../../lib/libclangAnalysis.a ../../../../lib/libclangAST.a ../../../../lib/libclangRewrite.a ../../../../lib/libclangLex.a ../../../../lib/libclangBasic.a ../../../../lib/libLLVM.dylib -Wl,-rpath,@executable_path/../lib
(In reply to comment #56) > (In reply to comment #55) > > (In reply to comment #54) > > > (In reply to comment #53) > > > > > > So the current permutation is okay for trunk (its already in stage 2 of the > > > bootstrap here)? > > > > LGTM. I think it's ready to commit. > > > > Unfortunately, there seems to be a glitch in this latest revision of the > patch which I noticed due to regressions... > > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.AddsTargetAndMode > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingMode > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingTarget > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.PathIgnored > > Expected Passes : 8869 > Expected Failures : 16 > Unsupported Tests : 113 > Unexpected Failures: 4 > > The previous patch from comment #46, which produced no clang test suite > regressions, has the compiler linked with properly libLLVM.dylib in the > front of the linkage... > > /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC > -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings > -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long > -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor > -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 > -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib > -rdynamic CMakeFiles/clang.dir/driver.cpp.o > CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o > -o ../../../../bin/clang-3.9 ../../../../lib/libLLVM.dylib > ../../../../lib/libclangBasic.a ../../../../lib/libclangCodeGen.a > ../../../../lib/libclangDriver.a ../../../../lib/libclangFrontend.a > ../../../../lib/libclangFrontendTool.a > -Wl,-sectcreate,__TEXT,__info_plist,/sw/src/fink.build/llvm39-3.9.0-1/build/ > stage1/tools/clang/tools/driver/Info.plist ../../../../lib/libclangCodeGen.a > ../../../../lib/libclangRewriteFrontend.a > ../../../../lib/libclangARCMigrate.a > ../../../../lib/libclangStaticAnalyzerFrontend.a > ../../../../lib/libclangFrontend.a ../../../../lib/libclangDriver.a > ../../../../lib/libclangParse.a ../../../../lib/libclangSerialization.a > ../../../../lib/libclangSema.a ../../../../lib/libclangEdit.a > ../../../../lib/libclangStaticAnalyzerCheckers.a > ../../../../lib/libclangStaticAnalyzerCore.a > ../../../../lib/libclangAnalysis.a ../../../../lib/libclangAST.a > ../../../../lib/libclangRewrite.a ../../../../lib/libclangLex.a > ../../../../lib/libclangBasic.a -Wl,-rpath,@executable_path/../lib > > whereas this new version of the patch, using the change from comment #50, > has the linkage of libLLVN.dylib incorrectly moved to the end of the linkage > as... Why "incorrectly"? libclangX depend on the LLVM libraries, and dependencies go after the dependents on the link line? Can't see why that would cause these failures, and can't reproduce on Linux, sorry. > /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC > -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings > -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long > -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor > -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 > -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib > -rdynamic CMakeFiles/clang.dir/driver.cpp.o > CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o > -o ../../../../bin/clang-3.9 ../../../../lib/libclangBasic.a > ../../../../lib/libclangCodeGen.a ../../../../lib/libclangDriver.a > ../../../../lib/libclangFrontend.a ../../../../lib/libclangFrontendTool.a > -Wl,-sectcreate,__TEXT,__info_plist,/sw/src/fink.build/llvm39-3.9.0-1/build/ > stage1/tools/clang/tools/driver/Info.plist ../../../../lib/libclangCodeGen.a > ../../../../lib/libclangRewriteFrontend.a > ../../../../lib/libclangARCMigrate.a > ../../../../lib/libclangStaticAnalyzerFrontend.a > ../../../../lib/libclangFrontend.a ../../../../lib/libclangDriver.a > ../../../../lib/libclangParse.a ../../../../lib/libclangSerialization.a > ../../../../lib/libclangSema.a ../../../../lib/libclangEdit.a > ../../../../lib/libclangStaticAnalyzerCheckers.a > ../../../../lib/libclangStaticAnalyzerCore.a > ../../../../lib/libclangAnalysis.a ../../../../lib/libclangAST.a > ../../../../lib/libclangRewrite.a ../../../../lib/libclangLex.a > ../../../../lib/libclangBasic.a ../../../../lib/libLLVM.dylib > -Wl,-rpath,@executable_path/../lib
I compared it to the clang driver linkage from 3.7.1 built with -DBUILD_SHARED_LIBS:BOOL=ON... /sw/src/fink.build/llvm37-3.7.1-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -fno-strict-aliasing -Wno-nested-anon-types -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib CMakeFiles/clang.dir/driver.cpp.o CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o -o ../../../../bin/clang-3.7 ../../../../lib/libLLVMX86CodeGen.3.7.1.dylib ../../../../lib/libLLVMX86AsmPrinter.3.7.1.dylib ../../../../lib/libLLVMX86AsmParser.3.7.1.dylib ../../../../lib/libLLVMX86Desc.3.7.1.dylib ../../../../lib/libLLVMX86Info.3.7.1.dylib ../../../../lib/libLLVMX86Disassembler.3.7.1.dylib ../../../../lib/libLLVMPowerPCCodeGen.3.7.1.dylib ../../../../lib/libLLVMPowerPCAsmPrinter.3.7.1.dylib ../../../../lib/libLLVMPowerPCAsmParser.3.7.1.dylib ../../../../lib/libLLVMPowerPCDesc.3.7.1.dylib ../../../../lib/libLLVMPowerPCInfo.3.7.1.dylib ../../../../lib/libLLVMPowerPCDisassembler.3.7.1.dylib ../../../../lib/libLLVMARMCodeGen.3.7.1.dylib ../../../../lib/libLLVMARMAsmPrinter.3.7.1.dylib ../../../../lib/libLLVMARMAsmParser.3.7.1.dylib ../../../../lib/libLLVMARMDesc.3.7.1.dylib ../../../../lib/libLLVMARMInfo.3.7.1.dylib ../../../../lib/libLLVMARMDisassembler.3.7.1.dylib ../../../../lib/libLLVMAnalysis.3.7.1.dylib ../../../../lib/libLLVMCodeGen.3.7.1.dylib ../../../../lib/libLLVMCore.3.7.1.dylib ../../../../lib/libLLVMipa.3.7.1.dylib ../../../../lib/libLLVMipo.3.7.1.dylib ../../../../lib/libLLVMInstCombine.3.7.1.dylib ../../../../lib/libLLVMInstrumentation.3.7.1.dylib ../../../../lib/libLLVMMC.3.7.1.dylib ../../../../lib/libLLVMMCParser.3.7.1.dylib ../../../../lib/libLLVMObjCARCOpts.3.7.1.dylib ../../../../lib/libLLVMOption.3.7.1.dylib ../../../../lib/libLLVMScalarOpts.3.7.1.dylib ../../../../lib/libLLVMSupport.3.7.1.dylib ../../../../lib/libLLVMTransformUtils.3.7.1.dylib ../../../../lib/libLLVMVectorize.3.7.1.dylib ../../../../lib/libclangBasic.3.7.1.dylib ../../../../lib/libclangCodeGen.3.7.1.dylib ../../../../lib/libclangDriver.3.7.1.dylib ../../../../lib/libclangFrontend.3.7.1.dylib ../../../../lib/libclangFrontendTool.3.7.1.dylib -Wl,-sectcreate,__TEXT,__info_plist,/sw/src/fink.build/llvm37-3.7.1-1/build/stage1/tools/clang/tools/driver/Info.plist -Wl,-rpath,@executable_path/../lib which has the LLVM components in front of the clang ones.
putting the AND NOT ARG_STATIC back in... Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259743) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,13 +475,15 @@ # property has been set to an empty value. get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) - set(llvm_libs LLVM) - else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) + set(llvm_libs LLVM) + else() + llvm_map_components_to_libnames(llvm_libs + ${ARG_LINK_COMPONENTS} + ${LLVM_LINK_COMPONENTS} + ) + endif() endif() if(CMAKE_VERSION VERSION_LESS 2.8.12) moves the linkage of libLLVM,dylib back up to the front but results in 14 instances of missed pruning of static libs... $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 52 $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep libgtest | grep -c libLLVMSupport 37
Also looking at unpatched trunk built using -DBUILD_SHARED_LIBS:BOOL=ON, the clang driver linkage still has the LLVM shared libs placed before those of the clang shared libs.
My assumption here is that the failures I am now seeing on x86_64 darwin in... Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.AddsTargetAndMode Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingMode Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingTarget Clang-Unit :: Tooling/ToolingTests/addTargetAndModeForProgramName.PathIgnored are due to this shift in libLLVM.dylib to the back of the linkage exposing the latent issue of the extra linkage of libLLVMSupport.a with libgtest.a. Won't the symbols in LLVMSupport be resolved from the libLLVM.dylib when it is linked in before the static libLLVMSupport.a while moving the linkage of libLLVM.dylib to the end would favor the symbols from the static lib?
I would also point out the the patch from comment #46, which places libLLVM.dylib in the front, and the patch from comment #50, which places libLLVM.dylib at the end of the linkage, both produce identical levels of pruning of LLVM static libs according to... $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 38
(In reply to comment #62) > I would also point out the the patch from comment #46, which places > libLLVM.dylib in the front, and the patch from comment #50, which places > libLLVM.dylib at the end of the linkage, both produce identical levels of > pruning of LLVM static libs according to... > > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > -c libLLVMSupport > 38 (In reply to comment #61) > My assumption here is that the failures I am now seeing on x86_64 darwin > in... > > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.AddsTargetAndMode > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingMode > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.IgnoresExistingTarget > Clang-Unit :: > Tooling/ToolingTests/addTargetAndModeForProgramName.PathIgnored > > are due to this shift in libLLVM.dylib to the back of the linkage exposing > the latent issue of the extra linkage of libLLVMSupport.a with libgtest.a. > Won't the symbols in LLVMSupport be resolved from the libLLVM.dylib when it > is linked in before the static libLLVMSupport.a while moving the linkage of > libLLVM.dylib to the end would favor the symbols from the static lib? This sounds plausible. When I run "check-clang", all is well. But... $ LD_LIBRARY_PATH=lib ./tools/clang/unittests/Tooling/ToolingTests ToolingTests: /home/andrew/code/src/llvm.org/llvm/lib/Support/CommandLine.cpp:202: void {anonymous}::CommandLineParser::registerCategory(llvm::cl::OptionCategory*): Assertion `std::count_if(RegisteredOptionCategories.begin(), RegisteredOptionCategories.end(), [cat](const OptionCategory *Category) { return cat->getName() == Category->getName(); }) == 0 && "Duplicate option categories"' failed. Aborted (core dumped) If I rebuild ToolingTests without libLLVMSupport.a, then it runs fine. I don't know why check-clang is happy. I found why libLLVMSupport.a is being added. In AddLLVM.cmake, in llvm_add_library, "lib_deps" is added as dependencies of built libraries. The value of lib_deps is extracted from LLVMBuild.txt. In utils/unittests/LLVMBuild.txt, gtest has "required_libraries: Support". Removing that line will remove libLLVMSupport.a from the link. Can you try removing that to see if it gets the tests to pass? I don't enough about LLVMBuild.txt and its users to know what the right thing to do is, though. There seems to be a lot of duplication of dependencies between CMake and LLVMBuild.txt. Just removing the lib_deps code from AddLLVM.cmake doesn't work, though; that causes build failures.
(In reply to comment #63) > > are due to this shift in libLLVM.dylib to the back of the linkage exposing > > the latent issue of the extra linkage of libLLVMSupport.a with libgtest.a. > > Won't the symbols in LLVMSupport be resolved from the libLLVM.dylib when it > > is linked in before the static libLLVMSupport.a while moving the linkage of > > libLLVM.dylib to the end would favor the symbols from the static lib? > > This sounds plausible. > > When I run "check-clang", all is well. But... > > $ LD_LIBRARY_PATH=lib ./tools/clang/unittests/Tooling/ToolingTests > ToolingTests: > /home/andrew/code/src/llvm.org/llvm/lib/Support/CommandLine.cpp:202: void > {anonymous}::CommandLineParser::registerCategory(llvm::cl::OptionCategory*): > Assertion `std::count_if(RegisteredOptionCategories.begin(), > RegisteredOptionCategories.end(), [cat](const OptionCategory *Category) { > return cat->getName() == Category->getName(); }) == 0 && "Duplicate option > categories"' failed. > Aborted (core dumped) > > If I rebuild ToolingTests without libLLVMSupport.a, then it runs fine. I > don't know why check-clang is happy. > > I found why libLLVMSupport.a is being added. In AddLLVM.cmake, in > llvm_add_library, "lib_deps" is added as dependencies of built libraries. > The value of lib_deps is extracted from LLVMBuild.txt. In > utils/unittests/LLVMBuild.txt, gtest has "required_libraries: Support". > Removing that line will remove libLLVMSupport.a from the link. Can you try > removing that to see if it gets the tests to pass? > > I don't enough about LLVMBuild.txt and its users to know what the right > thing to do is, though. There seems to be a lot of duplication of > dependencies between CMake and LLVMBuild.txt. Just removing the lib_deps > code from AddLLVM.cmake doesn't work, though; that causes build failures. Removing 'required_libraries = Support' from utils/unittest/LLVMBuild.txt unfortunately doesn't impact the results... $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 38 nor does also adding in... @@ -885,11 +887,18 @@ add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO ${ARGN}) set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir}) - target_link_libraries(${test_name} - gtest - gtest_main - LLVMSupport # gtest needs it for raw_ostream. - ) + if (LLVM_LINK_LLVM_DYLIB) + target_link_libraries(${test_name} + gtest + gtest_main + ) + else() + target_link_libraries(${test_name} + gtest + gtest_main + LLVMSupport # gtest needs it for raw_ostream. + ) + endif()
Andrew, what happens when you run cmake with the -DLLVM_LINK_LLVM_DYLIB:BOOL=ON option and then execute 'find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport' (or find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.so | grep -c libLLVMSupport on linux)? Do you really see a reduction in the count of libLLVM linkages that also pass libLLVMSupport.a with your proposed changes?
Andrew, perhaps you should just go ahead and commit the current patch from comment #50 into trunk and we can open a new bug report for this unpruned linkage of libLLVMSupport.a when libgtest is emitted? The handling of libgtest and its LLVM support lib dependency in cmake looks like a real rat's nest.
Eureka! This permutation works... Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259799) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,13 +475,15 @@ # property has been set to an empty value. get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) - set(llvm_libs LLVM) - else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) + set(llvm_libs LLVM) + else() + llvm_map_components_to_libnames(llvm_libs + ${ARG_LINK_COMPONENTS} + ${LLVM_LINK_COMPONENTS} + ) + endif() endif() if(CMAKE_VERSION VERSION_LESS 2.8.12) @@ -885,11 +887,18 @@ add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO ${ARGN}) set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir}) - target_link_libraries(${test_name} - gtest - gtest_main - LLVMSupport # gtest needs it for raw_ostream. - ) + if (LLVM_LINK_LLVM_DYLIB) + target_link_libraries(${test_name} + gtest + gtest_main + ) + else() + target_link_libraries(${test_name} + gtest + gtest_main + LLVMSupport # gtest needs it for raw_ostream. + ) + endif() add_dependencies(${test_suite} ${test_name}) get_target_property(test_suite_folder ${test_suite} FOLDER) Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259799) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif() Index: utils/unittest/CMakeLists.txt =================================================================== --- utils/unittest/CMakeLists.txt (revision 259799) +++ utils/unittest/CMakeLists.txt (working copy) @@ -32,9 +32,11 @@ add_definitions( -DGTEST_HAS_PTHREAD=0 ) endif() -set(LIBS - LLVMSupport # Depends on llvm::raw_ostream -) +if (NOT LLVM_LINK_LLVM_DYLIB) + set(LIBS + LLVMSupport # Depends on llvm::raw_ostream + ) +endif() find_library(PTHREAD_LIBRARY_PATH pthread) if (PTHREAD_LIBRARY_PATH) Index: utils/unittest/LLVMBuild.txt =================================================================== --- utils/unittest/LLVMBuild.txt (revision 259799) +++ utils/unittest/LLVMBuild.txt (working copy) @@ -19,7 +19,6 @@ type = Library name = gtest parent = Libraries -required_libraries = Support installed = 0 [component_1] Index: utils/unittest/UnitTestMain/CMakeLists.txt =================================================================== --- utils/unittest/UnitTestMain/CMakeLists.txt (revision 259799) +++ utils/unittest/UnitTestMain/CMakeLists.txt (working copy) @@ -1,7 +1,16 @@ -add_llvm_library(gtest_main - TestMain.cpp +if (LLVM_LINK_LLVM_DYLIB) + add_llvm_library(gtest_main + TestMain.cpp - LINK_LIBS - gtest - LLVMSupport # Depends on llvm::cl - ) + LINK_LIBS + gtest + ) +else() + add_llvm_library(gtest_main + TestMain.cpp + + LINK_LIBS + gtest + LLVMSupport # Depends on llvm::cl + ) +endif() $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 1
(In reply to comment #67) > Eureka! This permutation works... > I did a few permutations pruning of each the new sections out of the working patch and I am pretty sure that we need every part of it to eliminate the stray libLLVMSupport.a linkages.
(In reply to comment #68) > (In reply to comment #67) > > Eureka! This permutation works... > > > > I did a few permutations pruning of each the new sections out of the working > patch and I am pretty sure that we need every part of it to eliminate the > stray libLLVMSupport.a linkages. Reconfirmed that the section of the patch... Index: utils/unittest/LLVMBuild.txt =================================================================== --- utils/unittest/LLVMBuild.txt (revision 259799) +++ utils/unittest/LLVMBuild.txt (working copy) @@ -19,7 +19,6 @@ type = Library name = gtest parent = Libraries -required_libraries = Support installed = 0 [component_1] is essential. Removing goes back to... $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 38
Created attachment 15827 [details] complete patch to strip LLVM_DYLIB_COMPONENTS out of link_components
A quick check using the same trunk pull and the complete patch on x86_64-apple-darwin13 but with -DBUILD_SHARED_LIBS:BOOL=ON instead of -DLLVM_LINK_LLVM_DYLIB:BOOL=ON suggests that the removal of 'required_libraries = Support' from utils/unittest/LLVMBuild.txt isn't an issue... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wdelete-non-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -Wl,-dead_strip CMakeFiles/ClangApplyReplacementsTests.dir/ReformattingTest.cpp.o -o ClangApplyReplacementsTests ../../../../../../lib/libgtest.dylib ../../../../../../lib/libgtest_main.dylib ../../../../../../lib/libLLVMSupport.dylib ../../../../../../lib/libclangApplyReplacements.dylib ../../../../../../lib/libclangToolingCore.dylib -Wl,-rpath,@executable_path/../lib as the unit tests have no problems in retaining the LLVMSupport linkage.
Perhaps we ought to put a comment in utils/unittest/LLVMBuild.txt to remind folks that 'required_libraries = Support' shouldn't be reinserted.
A 3-stage bootstrap (with stage2/stage3 file comparisons), using current llvm/clang/clang-tools-extras/compiler-rt/polly/openmp/libc++ from trunk and the complete PR26393_v5.patch, on x86_64-apple-darwin13 using -DBUILD_SHARED_LIBS:BOOL=ON and on x86_64-apple-darwin15 using -DLLVM_LINK_LLVM_DYLIB:BOOL=ON both produce identical test suite results of... Failing Tests (3): LLVM :: tools/lto/hide-linkonce-odr.ll LLVM :: tools/lto/opt-level.ll LLVM :: tools/sanstats/elf.test Expected Passes : 11765 Expected Failures : 55 Unsupported Tests : 3852 Unexpected Failures: 3 Expected Passes : 8875 Expected Failures : 16 Unsupported Tests : 113 [100%] Built target check-clang Expected Passes : 238 [100%] Built target check-clang-tools Expected Passes : 581 Expected Failures : 17 Unsupported Tests : 7 [100%] Built target check-polly Expected Passes : 5013 Expected Failures : 12 Unsupported Tests : 10 [100%] Built target check-libcxx Expected Passes : 67 [100%] Built target check-libomp
(In reply to comment #71) > A quick check using the same trunk pull and the complete patch on > x86_64-apple-darwin13 but with -DBUILD_SHARED_LIBS:BOOL=ON instead of > -DLLVM_LINK_LLVM_DYLIB:BOOL=ON suggests that the removal of > 'required_libraries = Support' from utils/unittest/LLVMBuild.txt isn't an > issue... > > /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC > -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings > -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long > -Wcovered-switch-default -Wdelete-non-virtual-dtor -std=c++11 -fno-common > -Woverloaded-virtual -Wno-nested-anon-types -O3 -Wl,-search_paths_first > -Wl,-headerpad_max_install_names -L/sw/lib -Wl,-dead_strip > CMakeFiles/ClangApplyReplacementsTests.dir/ReformattingTest.cpp.o -o > ClangApplyReplacementsTests ../../../../../../lib/libgtest.dylib > ../../../../../../lib/libgtest_main.dylib > ../../../../../../lib/libLLVMSupport.dylib > ../../../../../../lib/libclangApplyReplacements.dylib > ../../../../../../lib/libclangToolingCore.dylib > -Wl,-rpath,@executable_path/../lib > > as the unit tests have no problems in retaining the LLVMSupport linkage. Same test of trunk with the complete patch on x86_64-apple-darwin15 with stock build (no -DBUILD_SHARED_LIBS:BOOL=ON or -DLLVM_LINK_LLVM_DYLIB:BOOL=ON options) shows the expected libLLVMSupport static linkages... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -Wl,-dead_strip CMakeFiles/ClangApplyReplacementsTests.dir/ReformattingTest.cpp.o -o ClangApplyReplacementsTests ../../../../../../lib/libgtest.a ../../../../../../lib/libgtest_main.a ../../../../../../lib/libLLVMSupport.a ../../../../../../lib/libclangApplyReplacements.a ../../../../../../lib/libclangToolingCore.a ../../../../../../lib/libgtest.a ../../../../../../lib/libclangAST.a ../../../../../../lib/libclangRewrite.a ../../../../../../lib/libclangLex.a ../../../../../../lib/libclangBasic.a ../../../../../../lib/libLLVMCore.a ../../../../../../lib/libLLVMMC.a ../../../../../../lib/libLLVMSupport.a -lcurses -lpthread -lz -lm -Wl,-rpath,@executable_path/../lib
Also confirmed that a 3-stage bootstrap (with stage2/stage3 file comparisons), using current llvm/clang/clang-tools-extras/compiler-rt/polly/openmp/libc++ from trunk and the complete PR26393_v5.patch, on x86_64-apple-darwin15 produces identical test suite results for the stock static build as was seen with -DBUILD_SHARED_LIBS:BOOL=ON and -.DLLVM_LINK_LLVM_DYLIB:BOOL=ON builds (comment #73).
(In reply to comment #67) > Eureka! This permutation works... Great! Please see my comments below. > Index: cmake/modules/AddLLVM.cmake > =================================================================== > --- cmake/modules/AddLLVM.cmake (revision 259799) > +++ cmake/modules/AddLLVM.cmake (working copy) > @@ -475,13 +475,15 @@ > # property has been set to an empty value. > get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) > > - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > - set(llvm_libs LLVM) > - else() > - llvm_map_components_to_libnames(llvm_libs > - ${ARG_LINK_COMPONENTS} > - ${LLVM_LINK_COMPONENTS} > - ) > + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) > + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > + set(llvm_libs LLVM) > + else() > + llvm_map_components_to_libnames(llvm_libs > + ${ARG_LINK_COMPONENTS} > + ${LLVM_LINK_COMPONENTS} > + ) > + endif() > endif() > > if(CMAKE_VERSION VERSION_LESS 2.8.12) > @@ -885,11 +887,18 @@ > add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO ${ARGN}) > set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) > set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR > ${outdir}) > - target_link_libraries(${test_name} > - gtest > - gtest_main > - LLVMSupport # gtest needs it for raw_ostream. > - ) > + if (LLVM_LINK_LLVM_DYLIB) > + target_link_libraries(${test_name} > + gtest > + gtest_main > + ) > + else() > + target_link_libraries(${test_name} > + gtest > + gtest_main > + LLVMSupport # gtest needs it for raw_ostream. > + ) > + endif() IIANM, it should be OK to unconditionally drop LLVMSupport here. gtest should depend on either libLLVM or libLLVMSupport, and adding gtest should add the dependencies. > > add_dependencies(${test_suite} ${test_name}) > get_target_property(test_suite_folder ${test_suite} FOLDER) > Index: cmake/modules/LLVM-Config.cmake > =================================================================== > --- cmake/modules/LLVM-Config.cmake (revision 259799) > +++ cmake/modules/LLVM-Config.cmake (working copy) > @@ -40,10 +40,17 @@ > # done in case libLLVM does not contain all of the components > # the target requires. > # > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > # To do this, we need special handling for "all", since that > # may imply linking to libraries that are not included in > # libLLVM. > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > + set(link_components "") > + else() > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > + endif() > + endif() > target_link_libraries(${executable} LLVM) > endif() > > Index: utils/unittest/CMakeLists.txt > =================================================================== > --- utils/unittest/CMakeLists.txt (revision 259799) > +++ utils/unittest/CMakeLists.txt (working copy) > @@ -32,9 +32,11 @@ > add_definitions( -DGTEST_HAS_PTHREAD=0 ) > endif() > > -set(LIBS > - LLVMSupport # Depends on llvm::raw_ostream > -) > +if (NOT LLVM_LINK_LLVM_DYLIB) > + set(LIBS > + LLVMSupport # Depends on llvm::raw_ostream > + ) > +endif() Rather than just dropping it altogether, instead add the following to the "add_llvm_library" below: ... LLVM_COMPONENTS Support ... > find_library(PTHREAD_LIBRARY_PATH pthread) > if (PTHREAD_LIBRARY_PATH) > Index: utils/unittest/LLVMBuild.txt > =================================================================== > --- utils/unittest/LLVMBuild.txt (revision 259799) > +++ utils/unittest/LLVMBuild.txt (working copy) > @@ -19,7 +19,6 @@ > type = Library > name = gtest > parent = Libraries > -required_libraries = Support > installed = 0 > > [component_1] This bit I'm not so sure about. I know that changing this has the immediately desired effect, but I'm not sure if it's the right change. gtest *does* require LLVMSupport, so I think instead of removing it here, we should change the llvm-build and CMake bits to do the libLLVM/component-lib thing. > Index: utils/unittest/UnitTestMain/CMakeLists.txt > =================================================================== > --- utils/unittest/UnitTestMain/CMakeLists.txt (revision 259799) > +++ utils/unittest/UnitTestMain/CMakeLists.txt (working copy) > @@ -1,7 +1,16 @@ > -add_llvm_library(gtest_main > - TestMain.cpp > +if (LLVM_LINK_LLVM_DYLIB) > + add_llvm_library(gtest_main > + TestMain.cpp > > - LINK_LIBS > - gtest > - LLVMSupport # Depends on llvm::cl > - ) > + LINK_LIBS > + gtest > + ) > +else() > + add_llvm_library(gtest_main > + TestMain.cpp > + > + LINK_LIBS > + gtest > + LLVMSupport # Depends on llvm::cl > + ) > +endif() Again, I think we should just be able to remove LLVMSupport here and rely on CMake to add the transitive dependencies. > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > -c libLLVMSupport > 1
(In reply to comment #76) > (In reply to comment #67) > > Eureka! This permutation works... > > Great! Please see my comments below. > > > Index: cmake/modules/AddLLVM.cmake > > =================================================================== > > --- cmake/modules/AddLLVM.cmake (revision 259799) > > +++ cmake/modules/AddLLVM.cmake (working copy) > > @@ -475,13 +475,15 @@ > > # property has been set to an empty value. > > get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) > > > > - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > > - set(llvm_libs LLVM) > > - else() > > - llvm_map_components_to_libnames(llvm_libs > > - ${ARG_LINK_COMPONENTS} > > - ${LLVM_LINK_COMPONENTS} > > - ) > > + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) > > + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > > + set(llvm_libs LLVM) > > + else() > > + llvm_map_components_to_libnames(llvm_libs > > + ${ARG_LINK_COMPONENTS} > > + ${LLVM_LINK_COMPONENTS} > > + ) > > + endif() > > endif() > > > > if(CMAKE_VERSION VERSION_LESS 2.8.12) > > @@ -885,11 +887,18 @@ > > add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO ${ARGN}) > > set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}) > > set_output_directory(${test_name} BINARY_DIR ${outdir} LIBRARY_DIR > > ${outdir}) > > - target_link_libraries(${test_name} > > - gtest > > - gtest_main > > - LLVMSupport # gtest needs it for raw_ostream. > > - ) > > + if (LLVM_LINK_LLVM_DYLIB) > > + target_link_libraries(${test_name} > > + gtest > > + gtest_main > > + ) > > + else() > > + target_link_libraries(${test_name} > > + gtest > > + gtest_main > > + LLVMSupport # gtest needs it for raw_ostream. > > + ) > > + endif() > > IIANM, it should be OK to unconditionally drop LLVMSupport here. gtest > should depend on either libLLVM or libLLVMSupport, and adding gtest should > add the dependencies. > > > > > add_dependencies(${test_suite} ${test_name}) > > get_target_property(test_suite_folder ${test_suite} FOLDER) > > Index: cmake/modules/LLVM-Config.cmake > > =================================================================== > > --- cmake/modules/LLVM-Config.cmake (revision 259799) > > +++ cmake/modules/LLVM-Config.cmake (working copy) > > @@ -40,10 +40,17 @@ > > # done in case libLLVM does not contain all of the components > > # the target requires. > > # > > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > > # To do this, we need special handling for "all", since that > > # may imply linking to libraries that are not included in > > # libLLVM. > > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > > + set(link_components "") > > + else() > > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > > + endif() > > + endif() > > target_link_libraries(${executable} LLVM) > > endif() > > > > Index: utils/unittest/CMakeLists.txt > > =================================================================== > > --- utils/unittest/CMakeLists.txt (revision 259799) > > +++ utils/unittest/CMakeLists.txt (working copy) > > @@ -32,9 +32,11 @@ > > add_definitions( -DGTEST_HAS_PTHREAD=0 ) > > endif() > > > > -set(LIBS > > - LLVMSupport # Depends on llvm::raw_ostream > > -) > > +if (NOT LLVM_LINK_LLVM_DYLIB) > > + set(LIBS > > + LLVMSupport # Depends on llvm::raw_ostream > > + ) > > +endif() > > Rather than just dropping it altogether, instead add the following to the > "add_llvm_library" below: > ... > LLVM_COMPONENTS > Support > ... I don't understand what you want here. Do you mean? add_llvm_library(gtest googletest/src/gtest-all.cc LLVM_COMPONENTS Support ${LIBS} } instead of add_llvm_library(gtest googletest/src/gtest-all.cc LINK_LIBS ${LIBS} ) in utils/unittest/CMakeLists.txt? And what about the preceding instance of... set(LIBS LLVMSupport # Depends on llvm::raw_ostream ) > > > find_library(PTHREAD_LIBRARY_PATH pthread) > > if (PTHREAD_LIBRARY_PATH) > > Index: utils/unittest/LLVMBuild.txt > > =================================================================== > > --- utils/unittest/LLVMBuild.txt (revision 259799) > > +++ utils/unittest/LLVMBuild.txt (working copy) > > @@ -19,7 +19,6 @@ > > type = Library > > name = gtest > > parent = Libraries > > -required_libraries = Support > > installed = 0 > > > > [component_1] > > This bit I'm not so sure about. I know that changing this has the > immediately desired effect, but I'm not sure if it's the right change. > > gtest *does* require LLVMSupport, so I think instead of removing it here, we > should change the llvm-build and CMake bits to do the libLLVM/component-lib > thing. > > > Index: utils/unittest/UnitTestMain/CMakeLists.txt > > =================================================================== > > --- utils/unittest/UnitTestMain/CMakeLists.txt (revision 259799) > > +++ utils/unittest/UnitTestMain/CMakeLists.txt (working copy) > > @@ -1,7 +1,16 @@ > > -add_llvm_library(gtest_main > > - TestMain.cpp > > +if (LLVM_LINK_LLVM_DYLIB) > > + add_llvm_library(gtest_main > > + TestMain.cpp > > > > - LINK_LIBS > > - gtest > > - LLVMSupport # Depends on llvm::cl > > - ) > > + LINK_LIBS > > + gtest > > + ) > > +else() > > + add_llvm_library(gtest_main > > + TestMain.cpp > > + > > + LINK_LIBS > > + gtest > > + LLVMSupport # Depends on llvm::cl > > + ) > > +endif() > > Again, I think we should just be able to remove LLVMSupport here and rely on > CMake to add the transitive dependencies. > > > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > > -c libLLVMSupport > > 1
> > > Index: utils/unittest/CMakeLists.txt > > > =================================================================== > > > --- utils/unittest/CMakeLists.txt (revision 259799) > > > +++ utils/unittest/CMakeLists.txt (working copy) > > > @@ -32,9 +32,11 @@ > > > add_definitions( -DGTEST_HAS_PTHREAD=0 ) > > > endif() > > > > > > -set(LIBS > > > - LLVMSupport # Depends on llvm::raw_ostream > > > -) > > > +if (NOT LLVM_LINK_LLVM_DYLIB) > > > + set(LIBS > > > + LLVMSupport # Depends on llvm::raw_ostream > > > + ) > > > +endif() > > > > Rather than just dropping it altogether, instead add the following to the > > "add_llvm_library" below: > > ... > > LLVM_COMPONENTS > > Support > > ... > > I don't understand what you want here. Do you mean? > > add_llvm_library(gtest > googletest/src/gtest-all.cc > > LLVM_COMPONENTS > Support > ${LIBS} > } Not quite, see diff below (also, LLVM_COMPONENTS was a typo -- should be LINK_COMPONENTS). > instead of > > add_llvm_library(gtest > googletest/src/gtest-all.cc > > LINK_LIBS > ${LIBS} > ) > > in utils/unittest/CMakeLists.txt? And what about the preceding instance of... > > set(LIBS > LLVMSupport # Depends on llvm::raw_ostream > ) Index: utils/unittest/CMakeLists.txt =================================================================== --- utils/unittest/CMakeLists.txt (revision 259475) +++ utils/unittest/CMakeLists.txt (working copy) @@ -32,10 +32,6 @@ add_definitions( -DGTEST_HAS_PTHREAD=0 ) endif() -set(LIBS - LLVMSupport # Depends on llvm::raw_ostream -) - find_library(PTHREAD_LIBRARY_PATH pthread) if (PTHREAD_LIBRARY_PATH) list(APPEND LIBS pthread) @@ -46,6 +42,9 @@ LINK_LIBS ${LIBS} + + LINK_COMPONENTS + Support )
Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259799) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,13 +475,15 @@ # property has been set to an empty value. get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) - set(llvm_libs LLVM) - else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) + set(llvm_libs LLVM) + else() + llvm_map_components_to_libnames(llvm_libs + ${ARG_LINK_COMPONENTS} + ${LLVM_LINK_COMPONENTS} + ) + endif() endif() if(CMAKE_VERSION VERSION_LESS 2.8.12) @@ -888,7 +890,6 @@ target_link_libraries(${test_name} gtest gtest_main - LLVMSupport # gtest needs it for raw_ostream. ) add_dependencies(${test_suite} ${test_name}) Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259799) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif() Index: utils/unittest/CMakeLists.txt =================================================================== --- utils/unittest/CMakeLists.txt (revision 259799) +++ utils/unittest/CMakeLists.txt (working copy) @@ -32,10 +32,6 @@ add_definitions( -DGTEST_HAS_PTHREAD=0 ) endif() -set(LIBS - LLVMSupport # Depends on llvm::raw_ostream -) - find_library(PTHREAD_LIBRARY_PATH pthread) if (PTHREAD_LIBRARY_PATH) list(APPEND LIBS pthread) @@ -45,6 +41,8 @@ googletest/src/gtest-all.cc LINK_LIBS + LLVM_COMPONENTS + Support ${LIBS} ) Index: utils/unittest/UnitTestMain/CMakeLists.txt =================================================================== --- utils/unittest/UnitTestMain/CMakeLists.txt (revision 259799) +++ utils/unittest/UnitTestMain/CMakeLists.txt (working copy) @@ -3,5 +3,4 @@ LINK_LIBS gtest - LLVMSupport # Depends on llvm::cl ) doesn't work... $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 38 Unless I also use... Index: utils/unittest/LLVMBuild.txt =================================================================== --- utils/unittest/LLVMBuild.txt (revision 259799) +++ utils/unittest/LLVMBuild.txt (working copy) @@ -19,7 +19,6 @@ type = Library name = gtest parent = Libraries -required_libraries = Support installed = 0 [component_1] $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep -c libLLVMSupport 1 I don't see how you can ever leave 'required_libraries = Support' in that file. My understanding is that required_libraries refers to the base names of files and cmake has no way to know that LLVM contains the contents of Support. It just sees them as two different files.
Also, my understanding of the previous complete patch was that all those instance of explicitly passing LLVMSupport that you want to prune were in fact making the use of that lib certain for gtest when 'required_libraries = Support' is removed from utils/unittest/LLVMBuild.txt. As I suspected, when 'required_libraries = Support' is removed from when the changes in comment #79 in order to avoid the 37 extra linkages on libSupport under -DLLVM_LINK_LLVM_DYLIB:BOOL=ON, the -DBUILD_SHARED_LIBS:BOOL=ON build no longer passes libLLVMSupport with libgtest... /sw/src/fink.build/llvm39-3.9.0-1/opt-bin/ccclang++ -fno-common -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wdelete-non-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/sw/lib -Wl,-dead_strip CMakeFiles/ClangApplyReplacementsTests.dir/ReformattingTest.cpp.o -o ClangApplyReplacementsTests ../../../../../../lib/libgtest.dylib ../../../../../../lib/libgtest_main.dylib ../../../../../../lib/libclangApplyReplacements.dylib ../../../../../../lib/libclangToolingCore.dylib -Wl,-rpath,@executable_path/../lib
docs/LLVMBuild.rst has... LLVMBuild files are expected to define a strict set of sections and properties. A typical component description file for a library component would look like the following example: .. code-block:: ini [component_0] type = Library name = Linker parent = Libraries required_libraries = Archive BitReader Core Support TransformUtils ... - ``required_libraries`` **[optional]** If given, a list of the names of ``Library`` or ``LibraryGroup`` components which must also be linked in whenever this library is used. That is, the link time dependencies for this component. When tools are built, the build system will include the transitive closure of all ``required_libraries`` for the components the tool needs. Perhaps things could be refactored to use... - ``add_to_library_groups`` **[optional]** If given, a list of the names of ``LibraryGroup`` components which this component is also part of. This allows nesting groups of components. For example, the ``X86`` target might define a library group for all of the ``X86`` components. That library group might then be included in the ``all-targets`` library group. but that probably is a lot of work.
If I understand this correctly, your pruning of the instances of LLVMSupport would require us to find where the libLLVM is defined with a... [component_0] type = Library name = LLVM and add a add_to_library_groups = containsSupport then the file utils/unittest/LLVMBuild.txt would be changed from... required_libraries = Support to required_libraries = containsSupport
(In reply to comment #79) > Index: cmake/modules/AddLLVM.cmake > =================================================================== > --- cmake/modules/AddLLVM.cmake (revision 259799) > +++ cmake/modules/AddLLVM.cmake (working copy) > @@ -475,13 +475,15 @@ > # property has been set to an empty value. > get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) > > - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT > ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > - set(llvm_libs LLVM) > - else() > - llvm_map_components_to_libnames(llvm_libs > - ${ARG_LINK_COMPONENTS} > - ${LLVM_LINK_COMPONENTS} > - ) > + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) > + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) > + set(llvm_libs LLVM) > + else() > + llvm_map_components_to_libnames(llvm_libs > + ${ARG_LINK_COMPONENTS} > + ${LLVM_LINK_COMPONENTS} > + ) > + endif() > endif() > > if(CMAKE_VERSION VERSION_LESS 2.8.12) > @@ -888,7 +890,6 @@ > target_link_libraries(${test_name} > gtest > gtest_main > - LLVMSupport # gtest needs it for raw_ostream. > ) > > add_dependencies(${test_suite} ${test_name}) > Index: cmake/modules/LLVM-Config.cmake > =================================================================== > --- cmake/modules/LLVM-Config.cmake (revision 259799) > +++ cmake/modules/LLVM-Config.cmake (working copy) > @@ -40,10 +40,17 @@ > # done in case libLLVM does not contain all of the components > # the target requires. > # > - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. > + # Strip LLVM_DYLIB_COMPONENTS out of link_components. > # To do this, we need special handling for "all", since that > # may imply linking to libraries that are not included in > # libLLVM. > + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) > + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") > + set(link_components "") > + else() > + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) > + endif() > + endif() > target_link_libraries(${executable} LLVM) > endif() > > Index: utils/unittest/CMakeLists.txt > =================================================================== > --- utils/unittest/CMakeLists.txt (revision 259799) > +++ utils/unittest/CMakeLists.txt (working copy) > @@ -32,10 +32,6 @@ > add_definitions( -DGTEST_HAS_PTHREAD=0 ) > endif() > > -set(LIBS > - LLVMSupport # Depends on llvm::raw_ostream > -) > - > find_library(PTHREAD_LIBRARY_PATH pthread) > if (PTHREAD_LIBRARY_PATH) > list(APPEND LIBS pthread) > @@ -45,6 +41,8 @@ > googletest/src/gtest-all.cc > > LINK_LIBS > + LLVM_COMPONENTS > + Support > ${LIBS} > ) > > Index: utils/unittest/UnitTestMain/CMakeLists.txt > =================================================================== > --- utils/unittest/UnitTestMain/CMakeLists.txt (revision 259799) > +++ utils/unittest/UnitTestMain/CMakeLists.txt (working copy) > @@ -3,5 +3,4 @@ > > LINK_LIBS > gtest > - LLVMSupport # Depends on llvm::cl > ) > > doesn't work... > > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > -c libLLVMSupport > 38 > > Unless I also use... Indeed, the suggested change was unrelated to this bit. > Index: utils/unittest/LLVMBuild.txt > =================================================================== > --- utils/unittest/LLVMBuild.txt (revision 259799) > +++ utils/unittest/LLVMBuild.txt (working copy) > @@ -19,7 +19,6 @@ > type = Library > name = gtest > parent = Libraries > -required_libraries = Support > installed = 0 > > [component_1] > > $ find . -name link.txt -print0 | xargs -0 cat | grep libLLVM.dylib | grep > -c libLLVMSupport > 1 > > I don't see how you can ever leave 'required_libraries = Support' in that > file. My understanding is that required_libraries refers to the base names > of files and cmake has no way to know that LLVM contains the contents of > Support. It just sees them as two different files. That is how it currently works, I'm suggesting that we change that. Having "required_libraries = Support" *should* be equivalent to: set(LLVM_LINK_COMPONENTS Support) add_llvm_library(gtest ...) (which would link with libLLVM if LLVM_LINK_LLVM_DYLIB=ON) but in fact it is doing: add_llvm_library(gtest ...) target_link_libraries(gtest LLVMSupport) (which will link with libLLVMSupport regardless of LLVM_LINK_LLVM_DYLIB). I'm not sure what the correct way to do that is. I tried modifying llvm-build to just output the component names, and then add that to the llvm_map_components_to_libnames call in llvm_add_library, but now I'm getting circular dependency errors.
My reading of the documentation is that LibraryGroup's can only be defined with a 'add_to_library_groups' entry in each Library's LLVMBuild.txt. I don't see anything about associating individual libraries to common library group outside of the library definitions in LLVMBuild.txt files.
It looks like all of the handling of add_to_library_groups is done in utils/llvm-build/llvmbuild/componentinfo.py and utils/llvm-build/llvmbuild/main.py so that is likely unusable outside of defining the association with a given LibraryGroup in explicit library declarations in LLVMBuild.txt files.
Also tested a back port of the final complete patch, PR26393_v5.patch, applied to current 3.8 branch and 3-stage bootstrapped (with stage2/stage3 file comparisons), of llvm/clang/clang-tools-extras/compiler-rt/polly/openmp/libc++ for the stock static, -DBUILD_SHARED_LIBS:BOOL=ON and -DLLVM_LINK_LLVM_DYLIB:BOOL=ON builds. All three builds bootstrap successfully and produce identical clean test suite results on x86_64 darwin of... Expected Passes : 11648 Expected Failures : 51 Unsupported Tests : 3808 [100%] Built target check-llvm Expected Passes : 8761 Expected Failures : 16 Unsupported Tests : 101 [100%] Built target check-clang Expected Passes : 231 [100%] Built target check-clang-tools Expected Passes : 571 Expected Failures : 17 Unsupported Tests : 7 [100%] Built target check-polly Expected Passes : 5004 Expected Failures : 12 Unsupported Tests : 10 [100%] Built target check-libcxx Expected Passes : 67 [100%] Built target check-libomp
FYI, the original commit to hard-code in the linkages of LLVMSupport for gtest was... http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20101122/112439.html
(In reply to comment #88) > FYI, the original commit to hard-code in the linkages of LLVMSupport for > gtest was... > > http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20101122/112439.html The other one was committed in... http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20140210/204202.html
(In reply to comment #89) > (In reply to comment #88) > > FYI, the original commit to hard-code in the linkages of LLVMSupport for > > gtest was... > > > > http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20101122/112439.html > > The other one was committed in... > > http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20140210/204202.html Lastly the utils/unittest/LLVMBuild.txt was added until... http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20111107/131728.html
(In reply to comment #90) > (In reply to comment #89) > > (In reply to comment #88) > > > FYI, the original commit to hard-code in the linkages of LLVMSupport for > > > gtest was... > > > > > > http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20101122/112439.html > > > > The other one was committed in... > > > > http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20140210/204202.html > > Lastly the utils/unittest/LLVMBuild.txt was added until... > > http://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20111107/131728.html My reading of these commits is that the first two, r120101 and r201079, automated the build system to always link LLVMSupport in with gtest. The third commit, r144445, simply added the LLVMBuild.txt with the normal format. However, the pre-existing automation of the linkage for LLVMSupport allows us to safely comment out the 'required_libraries = Support' line in LLVMBuild.txt for this particular case without concern.
Index: CMakeLists.txt =================================================================== --- CMakeLists.txt (revision 259875) +++ CMakeLists.txt (working copy) @@ -462,7 +462,7 @@ message(STATUS "Constructing LLVMBuild project information") execute_process( - COMMAND ${PYTHON_EXECUTABLE} ${LLVMBUILDTOOL} + COMMAND ${PYTHON_EXECUTABLE} -m trace --trace ${LLVMBUILDTOOL} --native-target "${LLVM_NATIVE_ARCH}" --enable-targets "${LLVM_TARGETS_TO_BUILD}" --enable-optional-components "${LLVMOPTIONALCOMPONENTS}" will allow you to see the python code execution during the cmake run... --- modulename: componentinfo, funcname: get_library_name componentinfo.py(176): return self.library_name or self.name componentinfo.py(190): if basename in ('gtest', 'gtest_main'): componentinfo.py(193): return 'LLVM%s' % basename main.py(348): is_installed = c.installed main.py(355): self.component_info_map[dep].get_llvmconfig_component_name() main.py(356): for dep in c.required_libraries] main.py(359): for dep in c.add_to_library_groups: main.py(363): entries[c.name] = (llvmconfig_component_name, library_name, main.py(364): required_llvmconfig_component_names, main.py(365): is_installed) main.py(325): for c in self.ordered_component_infos: main.py(327): if c.type_name == 'OptionalLibrary' \ main.py(332): tg = c.get_parent_target_group() --- modulename: componentinfo, funcname: get_parent_target_group componentinfo.py(82): if self.type_name == 'TargetGroup': componentinfo.py(86): if self.parent_instance: componentinfo.py(87): return self.parent_instance.get_parent_target_group() --- modulename: componentinfo, funcname: get_parent_target_group componentinfo.py(82): if self.type_name == 'TargetGroup': componentinfo.py(83): return self main.py(333): if tg and not tg.enabled: main.py(334): continue main.py(325): for c in self.ordered_component_infos: main.py(327): if c.type_name == 'OptionalLibrary' \ main.py(332): tg = c.get_parent_target_group() --- modulename: componentinfo, funcname: get_parent_target_group componentinfo.py(82): if self.type_name == 'TargetGroup': componentinfo.py(86): if self.parent_instance: componentinfo.py(87): return self.parent_instance.get_parent_target_group() --- modulename: componentinfo, funcname: get_parent_target_group componentinfo.py(82): if self.type_name == 'TargetGroup': componentinfo.py(86): if self.parent_instance: componentinfo.py(87): return self.parent_instance.get_parent_target_group() --- modulename: componentinfo, funcname: get_parent_target_group componentinfo.py(82): if self.type_name == 'TargetGroup': componentinfo.py(86): if self.parent_instance: main.py(333): if tg and not tg.enabled: main.py(337): if c.type_name not in ('Library', 'OptionalLibrary', \ main.py(338): 'LibraryGroup', 'TargetGroup'): main.py(343): llvmconfig_component_name = c.get_llvmconfig_component_name() --- modulename: componentinfo, funcname: get_llvmconfig_component_name componentinfo.py(196): return self.get_library_name().lower() --- modulename: componentinfo, funcname: get_library_name componentinfo.py(176): return self.library_name or self.name main.py(346): if c.type_name == 'Library' or c.type_name == 'OptionalLibrary': main.py(347): library_name = c.get_prefixed_library_name() --- modulename: componentinfo, funcname: get_prefixed_library_name componentinfo.py(186): basename = self.get_library_name() --- modulename: componentinfo, funcname: get_library_name componentinfo.py(176): return self.library_name or self.name componentinfo.py(190): if basename in ('gtest', 'gtest_main'): componentinfo.py(193): return 'LLVM%s' % basename main.py(348): is_installed = c.installed main.py(355): self.component_info_map[dep].get_llvmconfig_component_name() main.py(356): for dep in c.required_libraries] --- modulename: componentinfo, funcname: get_llvmconfig_component_name componentinfo.py(196): return self.get_library_name().lower() --- modulename: componentinfo, funcname: get_library_name componentinfo.py(176): return self.library_name or self.name
It is unclear from the python trace if that code has access to the cmake option defines like DLLVM_LINK_LLVM_DYLIB. If it does, perhaps a check could be inserted to prune the Support dependency for that particular case.
We should be able to use this approach to get the setting of LLVM_LINK_LLVM_DYLIB into the llvm-build python scripts... Index: utils/llvm-build/llvmbuild/main.py =================================================================== --- utils/llvm-build/llvmbuild/main.py (revision 259975) +++ utils/llvm-build/llvmbuild/main.py (working copy) @@ -907,6 +907,13 @@ help=("Enable the given space or semi-colon separated " "list of optional components"), action="store", default="") + group.add_option("", "--enable-llvm-link-llvm-dylib", + dest="llvm-link-llvm-dylib", + help=("Use libLLVM instead of LLVM component shared " + "libraries for linkage", + action="store_true", default=False) + + parser.add_option_group(group) (opts, args) = parser.parse_args() Index: CMakeLists.txt =================================================================== --- CMakeLists.txt (revision 259975) +++ CMakeLists.txt (working copy) @@ -460,9 +460,16 @@ set(LLVMOPTIONALCOMPONENTS ${LLVMOPTIONALCOMPONENTS} OProfileJIT) endif (LLVM_USE_OPROFILE) +if (LLVM_LINK_LLVM_DYLIB) + set(ENABLE_LLVM_LINK_LLVM_DYLIB "--enable-llvm-link-llvm-dylib") +else() + set(ENABLE_LLVM_LINK_LLVM_DYLIB "") +endif() + message(STATUS "Constructing LLVMBuild project information") execute_process( COMMAND ${PYTHON_EXECUTABLE} ${LLVMBUILDTOOL} + "${ENABLE_LLVM_LINK_LLVM_DYLIB}" --native-target "${LLVM_NATIVE_ARCH}" --enable-targets "${LLVM_TARGETS_TO_BUILD}" --enable-optional-components "${LLVMOPTIONALCOMPONENTS}" and then use it to override the 'required_libraries = Support' setting in LLVMBuild.txt for the gtest library.
Index: CMakeLists.txt =================================================================== --- CMakeLists.txt (revision 259975) +++ CMakeLists.txt (working copy) @@ -460,9 +460,16 @@ set(LLVMOPTIONALCOMPONENTS ${LLVMOPTIONALCOMPONENTS} OProfileJIT) endif (LLVM_USE_OPROFILE) +if (LLVM_LINK_LLVM_DYLIB) + set(ENABLE_LLVM_LINK_LLVM_DYLIB "--enable-llvm-link-llvm-dylib") +else() + set(ENABLE_LLVM_LINK_LLVM_DYLIB "") +endif() + message(STATUS "Constructing LLVMBuild project information") execute_process( COMMAND ${PYTHON_EXECUTABLE} ${LLVMBUILDTOOL} + "${ENABLE_LLVM_LINK_LLVM_DYLIB}" --native-target "${LLVM_NATIVE_ARCH}" --enable-targets "${LLVM_TARGETS_TO_BUILD}" --enable-optional-components "${LLVMOPTIONALCOMPONENTS}" Index: utils/llvm-build/llvmbuild/main.py =================================================================== --- utils/llvm-build/llvmbuild/main.py (revision 259975) +++ utils/llvm-build/llvmbuild/main.py (working copy) @@ -907,6 +907,13 @@ help=("Enable the given space or semi-colon separated " "list of optional components"), action="store", default="") + group.add_option("", "--enable-llvm-link-llvm-dylib", + dest="llvm_link_llvm_dylib", + help=("Use libLLVM instead of LLVM component shared " + "libraries for linkage"), + action="store_true", default=False) + + parser.add_option_group(group) (opts, args) = parser.parse_args() @@ -937,6 +944,14 @@ # Validate the project component info. project_info.validate_components() + # Use libLLVM instead of LLVM component shared libraries for linkage + if opts.llvm_link_llvm_dylib: + use_llvm_link_llvm_dylib = True + print "--enable-llvm-link-llvm-dylib passed" + else: + use_llvm_link_llvm_dylib = False + print "--enable-llvm-link-llvm-dylib not passed" + # Print the component tree, if requested. if opts.print_tree: project_info.print_tree() works here in passing the flag over. Now to puzzle out how to utilize it with these python scripts to suppress the LLVMSupport dependency for gtest.
Created attachment 15842 [details] complete fix by adding --enable-llvm-link-llvm-dylib option to llvmbuild
Note that the new patch, which implements and uses --enable-llvm-link-llvm-dylib flag for utils/llvm-build/llvmbuild/main.py changes the tools/llvm-config/LibraryDependencies.inc file generated by llvm-build from containing... { "gtest", "libgtest.a", false, { "support" } }, { "gtest_main", "libgtest_main.a", false, { "gtest" } }, to { "gtest", "libgtest.a", false, { } }, { "gtest_main", "libgtest_main.a", false, { "gtest" } }, The current patch now looks like... Index: CMakeLists.txt =================================================================== --- CMakeLists.txt (revision 259975) +++ CMakeLists.txt (working copy) @@ -460,9 +460,16 @@ set(LLVMOPTIONALCOMPONENTS ${LLVMOPTIONALCOMPONENTS} OProfileJIT) endif (LLVM_USE_OPROFILE) +if (LLVM_LINK_LLVM_DYLIB) + set(ENABLE_LLVM_LINK_LLVM_DYLIB "--enable-llvm-link-llvm-dylib") +else() + set(ENABLE_LLVM_LINK_LLVM_DYLIB "") +endif() + message(STATUS "Constructing LLVMBuild project information") execute_process( COMMAND ${PYTHON_EXECUTABLE} ${LLVMBUILDTOOL} + "${ENABLE_LLVM_LINK_LLVM_DYLIB}" --native-target "${LLVM_NATIVE_ARCH}" --enable-targets "${LLVM_TARGETS_TO_BUILD}" --enable-optional-components "${LLVMOPTIONALCOMPONENTS}" Index: cmake/modules/AddLLVM.cmake =================================================================== --- cmake/modules/AddLLVM.cmake (revision 259975) +++ cmake/modules/AddLLVM.cmake (working copy) @@ -475,13 +475,15 @@ # property has been set to an empty value. get_property(lib_deps GLOBAL PROPERTY LLVMBUILD_LIB_DEPS_${name}) - if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_STATIC AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) - set(llvm_libs LLVM) - else() - llvm_map_components_to_libnames(llvm_libs - ${ARG_LINK_COMPONENTS} - ${LLVM_LINK_COMPONENTS} - ) + if (DEFINED LLVM_LINK_COMPONENTS OR DEFINED ARG_LINK_COMPONENTS) + if (LLVM_LINK_LLVM_DYLIB AND NOT ARG_DISABLE_LLVM_LINK_LLVM_DYLIB) + set(llvm_libs LLVM) + else() + llvm_map_components_to_libnames(llvm_libs + ${ARG_LINK_COMPONENTS} + ${LLVM_LINK_COMPONENTS} + ) + endif() endif() if(CMAKE_VERSION VERSION_LESS 2.8.12) @@ -888,7 +890,6 @@ target_link_libraries(${test_name} gtest gtest_main - LLVMSupport # gtest needs it for raw_ostream. ) add_dependencies(${test_suite} ${test_name}) Index: cmake/modules/LLVM-Config.cmake =================================================================== --- cmake/modules/LLVM-Config.cmake (revision 259975) +++ cmake/modules/LLVM-Config.cmake (working copy) @@ -40,10 +40,17 @@ # done in case libLLVM does not contain all of the components # the target requires. # - # TODO strip LLVM_DYLIB_COMPONENTS out of link_components. + # Strip LLVM_DYLIB_COMPONENTS out of link_components. # To do this, we need special handling for "all", since that # may imply linking to libraries that are not included in # libLLVM. + if (DEFINED link_components AND DEFINED LLVM_DYLIB_COMPONENTS) + if("${LLVM_DYLIB_COMPONENTS}" STREQUAL "all") + set(link_components "") + else() + list(REMOVE_ITEM link_components ${LLVM_DYLIB_COMPONENTS}) + endif() + endif() target_link_libraries(${executable} LLVM) endif() Index: utils/llvm-build/llvmbuild/main.py =================================================================== --- utils/llvm-build/llvmbuild/main.py (revision 259975) +++ utils/llvm-build/llvmbuild/main.py (working copy) @@ -350,6 +350,10 @@ library_name = None is_installed = True + # Check for gtest + if (library_name == 'gtest') and use_llvm_link_llvm_dylib: + c.required_libraries.remove('Support') + # Get the component names of all the required libraries. required_llvmconfig_component_names = [ self.component_info_map[dep].get_llvmconfig_component_name() @@ -907,6 +911,13 @@ help=("Enable the given space or semi-colon separated " "list of optional components"), action="store", default="") + group.add_option("", "--enable-llvm-link-llvm-dylib", + dest="llvm_link_llvm_dylib", + help=("Use libLLVM instead of LLVM component shared " + "libraries for linkage"), + action="store_true", default=False) + + parser.add_option_group(group) (opts, args) = parser.parse_args() @@ -937,6 +948,13 @@ # Validate the project component info. project_info.validate_components() + # Use libLLVM instead of LLVM component shared libraries for linkage + global use_llvm_link_llvm_dylib + if opts.llvm_link_llvm_dylib: + use_llvm_link_llvm_dylib = True + else: + use_llvm_link_llvm_dylib = False + # Print the component tree, if requested. if opts.print_tree: project_info.print_tree() Index: utils/unittest/CMakeLists.txt =================================================================== --- utils/unittest/CMakeLists.txt (revision 259975) +++ utils/unittest/CMakeLists.txt (working copy) @@ -32,10 +32,6 @@ add_definitions( -DGTEST_HAS_PTHREAD=0 ) endif() -set(LIBS - LLVMSupport # Depends on llvm::raw_ostream -) - find_library(PTHREAD_LIBRARY_PATH pthread) if (PTHREAD_LIBRARY_PATH) list(APPEND LIBS pthread) Index: utils/unittest/UnitTestMain/CMakeLists.txt =================================================================== --- utils/unittest/UnitTestMain/CMakeLists.txt (revision 259975) +++ utils/unittest/UnitTestMain/CMakeLists.txt (working copy) @@ -3,5 +3,4 @@ LINK_LIBS gtest - LLVMSupport # Depends on llvm::cl )
Moving the latest revision of the proposed patch to http://reviews.llvm.org/D16945.
Fixed with the commit of http://llvm.org/viewvc/llvm-project?view=revision&revision=260641 and back port of http://llvm.org/viewvc/llvm-project?view=revision&revision=260693 to 3.8 branch