This is an archive of the discontinued LLVM Phabricator instance.

[build] normalize components dependencies
ClosedPublic

Authored by serge-sans-paille on Nov 13 2020, 1:49 PM.

Details

Summary

Use LINK_COMPONENTS instead of explicit target_link_libraries for components.
This avoids redundancy and potential inconsistencies.

Diff Detail

Event Timeline

serge-sans-paille requested review of this revision.Nov 13 2020, 1:49 PM

This looks like an overdue fix. Thanks.

Before I accept it, have you tested multiple configurations (LLVM_LINK_DYLIB, CMAKE_SHARED_LIBS,...) to not break?

llvm/lib/FileCheck/CMakeLists.txt
7–9

Isn't it LLVM_COMPONENTS?

thopre added inline comments.Nov 13 2020, 3:36 PM
llvm/lib/FileCheck/CMakeLists.txt
7–9

or LINK_COMPONENTS?

s/LINK_COMPONENT/LINK_COMPONENTS

thopre added inline comments.Nov 14 2020, 8:49 AM
llvm/lib/FileCheck/CMakeLists.txt
7–9

I don't see any LLVM_COMPONENTS in the source code, and llvm_add_library used indirectly by add_llvm_component_library knows about LINK_COMPONENTS but not LLVM_COMPONENTS. Is my source outdated and this was introduced recently?

s/LLVM_COMPONENTS/LINK_COMPONENTS/

man do I feel stupid

llvm/lib/FileCheck/CMakeLists.txt
7–9

Indeed :-)

Patch LGTM but there's still @Meinersbur 's question about testing. I'll let him approve once he's happy with the answer.

Meinersbur added inline comments.Nov 14 2020, 8:57 PM
llvm/lib/FileCheck/CMakeLists.txt
7–9

I just noticed the inconsistent plural, not the LLVM_ prefix. Thanks that you noticed.

Tested with

cmake ../llvm  -DLLVM_LINK_LLVM_DYLIB=OFF -DBUILD_SHARED_LIBS=OFF && make check-llvm -j50
cmake ../llvm  -DLLVM_LINK_LLVM_DYLIB=OFF -DBUILD_SHARED_LIBS=OFF && make check-llvm -j50
cmake ../llvm  -DLLVM_LINK_LLVM_DYLIB=ON -DBUILD_SHARED_LIBS=ON && make check-llvm -j50
thopre accepted this revision.Nov 17 2020, 1:37 AM

LGTM

This revision is now accepted and ready to land.Nov 17 2020, 1:37 AM
This revision was automatically updated to reflect the committed changes.
sbc100 added a subscriber: sbc100.Nov 17 2020, 12:39 PM

I bisect a build in my build to this change.

I suspend its because I am building with -DBUILD_SHARED_LIBS=ON?

[242/325] Linking CXX executable bin/llvm-exegesis
FAILED: bin/llvm-exegesis 
: && /usr/local/google/home/sbc/dev/chromium/src/third_party/llvm-build/Release+Asserts/bin/clang++  -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O3  -Wl,-rpath-link,/usr/local/google/home/sbc/dev/wasm/llvm-build/./lib  -Wl,-O3 -Wl,--gc-sections tools/llvm-exegesis/CMakeFiles/llvm-exegesis.dir/llvm-exegesis.cpp.o  -o bin/llvm-exegesis  -Wl,-rpath,"\$ORIGIN/../lib"  -lpthread  lib/libLLVMExegesis.a  lib/libLLVMExegesisX86.a  lib/libLLVMX86CodeGen.so.12git  lib/libLLVMX86AsmParser.so.12git  lib/libLLVMX86Desc.so.12git  lib/libLLVMX86Disassembler.so.12git  lib/libLLVMX86Info.so.12git  lib/libLLVMExegesis.a  lib/libLLVMGlobalISel.so.12git  lib/libLLVMCodeGen.so.12git  lib/libLLVMMCDisassembler.so.12git  lib/libLLVMMCJIT.so.12git  lib/libLLVMExecutionEngine.so.12git  lib/libLLVMAnalysis.so.12git  lib/libLLVMObjectYAML.so.12git  lib/libLLVMObject.so.12git  lib/libLLVMMCParser.so.12git  lib/libLLVMMC.so.12git  lib/libLLVMCore.so.12git  lib/libLLVMSupport.so.12git  -Wl,-rpath-link,/usr/local/google/home/sbc/dev/wasm/llvm-build/lib && :
/usr/bin/ld: lib/libLLVMExegesis.a(Assembler.cpp.o): undefined reference to symbol '_ZN4llvm19RTDyldMemoryManager25getPointerToNamedFunctionERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEb'
/usr/bin/ld: /usr/local/google/home/sbc/dev/wasm/llvm-build/./lib/libLLVMRuntimeDyld.so.12git: error adding symbols: DSO missing from command line
clang: error: linker command failed with exit code 1 (use -v to see invocation)
[262/325] Building CXX object lib/LTO/CMakeFiles/LLVMLTO.dir/LTO.cpp.o
ninja: build stopped: subcommand failed.

Sadly, it doesn't fix it for me. I'm building tip of tree including that change and still seeing this.

@sbc100 Can you share your cmake configuration?

This broke building LLDB with BUILD_SHARED_LIBS=ON:

$ ninja UnwindAssemblyx86Tests
[530/530] Linking CXX executable tools/lldb/unittests/UnwindAssembly/x86/UnwindAssemblyx86Tests
FAILED: tools/lldb/unittests/UnwindAssembly/x86/UnwindAssemblyx86Tests 
: && /usr/bin/c++ -fPIC -fvisibility-inlines-hidden -Werror=date-time -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-maybe-uninitialized -Wno-class-memaccess -Wno-redundant-move -Wno-noexcept-type -Wdelete-non-virtual-dtor -Wsuggest-override -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -Wno-deprecated-declarations -Wno-unknown-pragmas -Wno-strict-aliasing -Wno-deprecated-register -Wno-vla-extension -Os -DNDEBUG -Wl,-O3 -Wl,--gc-sections tools/lldb/unittests/UnwindAssembly/x86/CMakeFiles/UnwindAssemblyx86Tests.dir/Testx86AssemblyInspectionEngine.cpp.o -o tools/lldb/unittests/UnwindAssembly/x86/UnwindAssemblyx86Tests  -Wl,-rpath,/home/mgorny/git/llvm-project/_build/lib  lib/libLLVMAArch64CodeGen.so.12git  lib/libLLVMAArch64AsmParser.so.12git  lib/libLLVMAArch64Disassembler.so.12git  lib/libLLVMAMDGPUCodeGen.so.12git  lib/libLLVMAMDGPUAsmParser.so.12git  lib/libLLVMAMDGPUDisassembler.so.12git  lib/libLLVMARMCodeGen.so.12git  lib/libLLVMARMAsmParser.so.12git  lib/libLLVMARMDisassembler.so.12git  lib/libLLVMAVRCodeGen.so.12git  lib/libLLVMAVRAsmParser.so.12git  lib/libLLVMAVRDesc.so.12git  lib/libLLVMAVRDisassembler.so.12git  lib/libLLVMAVRInfo.so.12git  lib/libLLVMBPFCodeGen.so.12git  lib/libLLVMBPFAsmParser.so.12git  lib/libLLVMBPFDesc.so.12git  lib/libLLVMBPFDisassembler.so.12git  lib/libLLVMBPFInfo.so.12git  lib/libLLVMHexagonCodeGen.so.12git  lib/libLLVMHexagonAsmParser.so.12git  lib/libLLVMHexagonDisassembler.so.12git  lib/libLLVMLanaiCodeGen.so.12git  lib/libLLVMLanaiAsmParser.so.12git  lib/libLLVMLanaiDisassembler.so.12git  lib/libLLVMMipsCodeGen.so.12git  lib/libLLVMMipsAsmParser.so.12git  lib/libLLVMMipsDesc.so.12git  lib/libLLVMMipsDisassembler.so.12git  lib/libLLVMMipsInfo.so.12git  lib/libLLVMMSP430CodeGen.so.12git  lib/libLLVMMSP430AsmParser.so.12git  lib/libLLVMMSP430Desc.so.12git  lib/libLLVMMSP430Disassembler.so.12git  lib/libLLVMMSP430Info.so.12git  lib/libLLVMNVPTXCodeGen.so.12git  lib/libLLVMNVPTXDesc.so.12git  lib/libLLVMNVPTXInfo.so.12git  lib/libLLVMPowerPCCodeGen.so.12git  lib/libLLVMPowerPCAsmParser.so.12git  lib/libLLVMPowerPCDesc.so.12git  lib/libLLVMPowerPCDisassembler.so.12git  lib/libLLVMPowerPCInfo.so.12git  lib/libLLVMRISCVCodeGen.so.12git  lib/libLLVMRISCVAsmParser.so.12git  lib/libLLVMRISCVDesc.so.12git  lib/libLLVMRISCVDisassembler.so.12git  lib/libLLVMRISCVInfo.so.12git  lib/libLLVMRISCVUtils.so.12git  lib/libLLVMSparcCodeGen.so.12git  lib/libLLVMSparcAsmParser.so.12git  lib/libLLVMSparcDesc.so.12git  lib/libLLVMSparcDisassembler.so.12git  lib/libLLVMSparcInfo.so.12git  lib/libLLVMSystemZCodeGen.so.12git  lib/libLLVMSystemZAsmParser.so.12git  lib/libLLVMSystemZDisassembler.so.12git  lib/libLLVMWebAssemblyCodeGen.so.12git  lib/libLLVMWebAssemblyAsmParser.so.12git  lib/libLLVMWebAssemblyDisassembler.so.12git  lib/libLLVMX86CodeGen.so.12git  lib/libLLVMX86AsmParser.so.12git  lib/libLLVMX86Desc.so.12git  lib/libLLVMX86Disassembler.so.12git  lib/libLLVMX86Info.so.12git  lib/libLLVMXCoreCodeGen.so.12git  lib/libLLVMXCoreDesc.so.12git  lib/libLLVMXCoreDisassembler.so.12git  lib/libLLVMXCoreInfo.so.12git  -lpthread  lib/libgtest_main.so.12git  lib/libgtest.so.12git  -lpthread  lib/liblldbCore.a  lib/liblldbSymbol.a  lib/liblldbPluginUnwindAssemblyX86.a  lib/libLLVMAArch64Desc.so.12git  lib/libLLVMAArch64Info.so.12git  lib/libLLVMAArch64Utils.so.12git  lib/libLLVMAMDGPUDesc.so.12git  lib/libLLVMAMDGPUInfo.so.12git  lib/libLLVMAMDGPUUtils.so.12git  lib/libLLVMARMDesc.so.12git  lib/libLLVMARMInfo.so.12git  lib/libLLVMARMUtils.so.12git  lib/libLLVMHexagonDesc.so.12git  lib/libLLVMHexagonInfo.so.12git  lib/libLLVMLanaiDesc.so.12git  lib/libLLVMLanaiInfo.so.12git  lib/libLLVMSystemZDesc.so.12git  lib/libLLVMSystemZInfo.so.12git  lib/libLLVMWebAssemblyDesc.so.12git  lib/libLLVMWebAssemblyInfo.so.12git  lib/liblldbCore.a  lib/liblldbSymbol.a  lib/liblldbBreakpoint.a  lib/liblldbDataFormatters.a  lib/liblldbExpression.a  lib/liblldbInterpreter.a  lib/liblldbTarget.a  lib/liblldbPluginCPlusPlusLanguage.a  lib/liblldbPluginObjCLanguage.a  lib/liblldbPluginObjectFileJIT.a  lib/liblldbCommands.a  lib/liblldbPluginExpressionParserClang.a  lib/liblldbPluginProcessUtility.a  lib/liblldbPluginClangCommon.a  lib/liblldbPluginCPPRuntime.a  lib/liblldbPluginTypeSystemClang.a  lib/liblldbPluginAppleObjCRuntime.a  lib/liblldbPluginObjCRuntime.a  lib/liblldbPluginRenderScriptRuntime.a  lib/liblldbPluginSymbolFileDWARF.a  lib/liblldbPluginSymbolFilePDB.a  lib/liblldbPluginSymbolFileNativePDB.a  lib/liblldbPluginObjectFilePDB.a  lib/liblldbCore.a  lib/liblldbSymbol.a  lib/liblldbBreakpoint.a  lib/liblldbDataFormatters.a  lib/liblldbExpression.a  lib/liblldbInterpreter.a  lib/liblldbTarget.a  lib/liblldbPluginCPlusPlusLanguage.a  lib/liblldbPluginObjCLanguage.a  lib/liblldbPluginObjectFileJIT.a  lib/liblldbCommands.a  lib/liblldbPluginExpressionParserClang.a  lib/liblldbPluginProcessUtility.a  lib/liblldbPluginClangCommon.a  lib/liblldbPluginCPPRuntime.a  lib/liblldbPluginTypeSystemClang.a  lib/liblldbPluginAppleObjCRuntime.a  lib/liblldbPluginObjCRuntime.a  lib/liblldbPluginRenderScriptRuntime.a  lib/liblldbPluginSymbolFileDWARF.a  lib/liblldbPluginSymbolFilePDB.a  lib/liblldbPluginSymbolFileNativePDB.a  lib/liblldbPluginObjectFilePDB.a  lib/liblldbCore.a  lib/liblldbSymbol.a  lib/liblldbBreakpoint.a  lib/liblldbDataFormatters.a  lib/liblldbExpression.a  lib/liblldbInterpreter.a  lib/liblldbTarget.a  lib/liblldbPluginCPlusPlusLanguage.a  lib/liblldbPluginObjCLanguage.a  lib/liblldbPluginObjectFileJIT.a  lib/liblldbCommands.a  lib/liblldbPluginExpressionParserClang.a  lib/liblldbPluginProcessUtility.a  lib/liblldbPluginClangCommon.a  lib/liblldbPluginCPPRuntime.a  lib/liblldbPluginTypeSystemClang.a  lib/liblldbPluginAppleObjCRuntime.a  lib/liblldbPluginObjCRuntime.a  lib/liblldbPluginRenderScriptRuntime.a  lib/liblldbPluginSymbolFileDWARF.a  lib/liblldbPluginSymbolFilePDB.a  lib/liblldbPluginSymbolFileNativePDB.a  lib/liblldbPluginObjectFilePDB.a  lib/liblldbCore.a  lib/liblldbSymbol.a  lib/liblldbBreakpoint.a  lib/liblldbDataFormatters.a  lib/liblldbExpression.a  lib/liblldbInterpreter.a  lib/liblldbTarget.a  lib/liblldbPluginCPlusPlusLanguage.a  lib/liblldbPluginObjCLanguage.a  lib/liblldbPluginObjectFileJIT.a  lib/liblldbCommands.a  lib/liblldbPluginExpressionParserClang.a  lib/liblldbPluginProcessUtility.a  lib/liblldbPluginClangCommon.a  lib/liblldbPluginCPPRuntime.a  lib/liblldbPluginTypeSystemClang.a  lib/liblldbPluginAppleObjCRuntime.a  lib/liblldbPluginObjCRuntime.a  lib/liblldbPluginRenderScriptRuntime.a  lib/liblldbPluginSymbolFileDWARF.a  lib/liblldbPluginSymbolFilePDB.a  lib/liblldbPluginSymbolFileNativePDB.a  lib/liblldbPluginObjectFilePDB.a  lib/liblldbCore.a  lib/liblldbSymbol.a  lib/liblldbBreakpoint.a  lib/liblldbDataFormatters.a  lib/liblldbExpression.a  lib/liblldbInterpreter.a  lib/liblldbTarget.a  lib/liblldbPluginCPlusPlusLanguage.a  lib/liblldbPluginObjCLanguage.a  lib/liblldbPluginObjectFileJIT.a  lib/liblldbCommands.a  lib/liblldbPluginExpressionParserClang.a  lib/liblldbPluginProcessUtility.a  lib/liblldbPluginClangCommon.a  lib/liblldbPluginCPPRuntime.a  lib/liblldbPluginTypeSystemClang.a  lib/liblldbPluginAppleObjCRuntime.a  lib/liblldbPluginObjCRuntime.a  lib/liblldbPluginRenderScriptRuntime.a  lib/liblldbPluginSymbolFileDWARF.a  lib/liblldbPluginSymbolFilePDB.a  lib/liblldbPluginSymbolFileNativePDB.a  lib/liblldbPluginObjectFilePDB.a  -lcurses  /usr/lib64/libform.so  /usr/lib64/libpanel.so  -ltinfo  lib/liblldbBase.a  lib/libLLVMMCJIT.so.12git  lib/libLLVMExecutionEngine.so.12git  lib/libclangCodeGen.so.12git  lib/libLLVMipo.so.12git  lib/libclangRewriteFrontend.so.12git  lib/libclangRewrite.so.12git  lib/libclangFrontend.so.12git  lib/libclangDriver.so.12git  lib/libclangParse.so.12git  lib/libclangSerialization.so.12git  lib/libclangSema.so.12git  lib/libclangEdit.so.12git  lib/libLLVMIRReader.so.12git  lib/libLLVMTarget.so.12git  lib/liblldbHost.a  /usr/lib64/libxml2.so  -ledit  -llzma  -ledit  -llzma  lib/libLLVMDebugInfoDWARF.so.12git  lib/libclangAST.so.12git  lib/libclangLex.so.12git  lib/libclangBasic.so.12git  lib/libLLVMDebugInfoPDB.so.12git  lib/libLLVMObject.so.12git  lib/libLLVMCore.so.12git  lib/liblldbUtility.a  -lpthread  -ldl  lib/libLLVMMCDisassembler.so.12git  lib/libLLVMMC.so.12git  lib/libLLVMBinaryFormat.so.12git  lib/libLLVMDebugInfoCodeView.so.12git  lib/libLLVMSupport.so.12git  lib/libLLVMDemangle.so.12git  -Wl,-rpath-link,/home/mgorny/git/llvm-project/_build/lib && cd /home/mgorny/git/llvm-project/_build/tools/lldb/unittests/UnwindAssembly/x86 && /usr/bin/cmake -E make_directory /home/mgorny/git/llvm-project/_build/tools/lldb/unittests/UnwindAssembly/x86/./Inputs
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: lib/liblldbExpression.a(IRExecutionUnit.cpp.o): undefined reference to symbol '_ZN4llvm23LegacyJITSymbolResolver6lookupERKSt3setINS_9StringRefESt4lessIS2_ESaIS2_EENS_15unique_functionIFvNS_8ExpectedISt3mapIS2_NS_18JITEvaluatedSymbolES4_SaISt4pairIKS2_SC_EEEEEEEE'
/usr/lib/gcc/x86_64-pc-linux-gnu/9.3.0/../../../../x86_64-pc-linux-gnu/bin/ld: /home/mgorny/git/llvm-project/_build/lib/libLLVMRuntimeDyld.so.12git: error adding symbols: DSO missing from command line
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

You seem to have reverted my fix from D36211.

Or to put it the other way, LINK_COMPONENTS is not equivalent to target_link_libraries with the PUBLIC keyword.

Or to put it the other way, LINK_COMPONENTS is not equivalent to target_link_libraries with the PUBLIC keyword.

Do you think we should revert the whole patch, or only a subpart of it?

@sbc100 Can you share your cmake configuration?

cmake -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/home/sbc/dev/wasm/waterfall/src/work/wasm-install -DCMAKE_SHARED_LINKER_FLAGS=-fuse-ld=gold -DBUILD_SHARED_LIBS=ON -DLLVM_BUILD_LLVM_DYLIB=OFF -DLLVM_LINK_LLVM_DYLIB=OFF -DLLVM_ENABLE_PROJECTS=lld;clang;lldb -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DLLVM_ENABLE_ASSERTIONS=ON -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_CXX_COMPILER_LAUNCHER=ccache ../llvm-project/llvm

Or to put it the other way, LINK_COMPONENTS is not equivalent to target_link_libraries with the PUBLIC keyword.

Do you think we should revert the whole patch, or only a subpart of it?

That once instance should be sufficient. If you could put a comment emphasizing that PUBLIC is used to propagate dependencies and therefore it's not the same as LINK_COMPONENTS, that would be great too.