TableGen: honor LLVM_LINK_LLVM_DYLIB by default

[nhaehnle]

Summary

Most TableGen tools have link dependency chains of the form

${project}-tblgen -> ${project}Support -> LLVMSupport

In LLVM_LINK_LLVM_DYLIB=ON builds, ${project}Support naturally links
aginst LLVMSupport implicitly by linking against libLLVM-*.so, and so
${project}-tblgen ends up with a dependency on the shared library.

Configuring the tool itself with DISABLE_LLVM_LINK_LLVM_DYLIB then
typically leads to LLVMSupport to be also included statically, leading
to duplicate definitions of symbols from LLVMSupport after the dynamic
linker has done its thing.

TableGen tools simply aren't that special: they can be linked dynamically
just fine, and so we do that to simplify the build setup.

The only exception to this rule is llvm-tblgen itself, which must be
statically linked to avoid circular dependencies of the form

llvm-tblgen -> llvm-shlib -> tablegen-generated files -> llvm-tblgen

See: https://discourse.llvm.org/t/rfc-cleaning-up-how-we-link-tablegen-tools/66678

Fixes: https://github.com/llvm/llvm-project/issues/58015

[stephenneuendorffer]

I think the MLIR parts of this code were written before attempting to create libMLIR.so. Probably these tablegen binaries would have to also avoid linking against libMLIR.so and be statically linked to both llvmSupport and MLIRsupport to work properly. I think the proposed solution is reasonable.

[grosser]

This patch overcomes the issue of cl options being defined multiple times. That is great!

I just tried it on Windows on top of e7fb6c394f519d6e6812f1fbbff1571d5e51f5c4 with (msys2), as well as LLVM_LINK_LLVM_DYLIB, LLVM_BUILD_LLVM_DYLIB, and MLIR enabled. I get the following error:

cmd.exe /C "cd . && C:\msys64\clang64\bin\clang++.exe -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 -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -ffunction-sections -fdata-sections -Werror=mismatched-tags -O3 -DNDEBUG -Wl,--stack,16777216    -Wl,--gc-sections tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeFormatGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/DialectGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/DirectiveCommonGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/EnumsGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/FormatGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/LLVMIRConversionGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/LLVMIRIntrinsicGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/mlir-tblgen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpClass.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpDefinitionsGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpDocGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpFormatGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpGenHelpers.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpInterfacesGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/OpPythonBindingGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/PassCAPIGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/PassDocGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/PassGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/RewriterGen.cpp.obj tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/SPIRVUtilsGen.cpp.obj -o bin\mlir-tblgen.exe -Wl,--out-implib,lib\libmlir-tblgen.dll.a -Wl,--major-image-version,0,--minor-image-version,0  lib/libMLIRSupportIndentedOstream.a  lib/libMLIRTblgenLib.a  lib/libMLIRTableGen.a  lib/libLLVM-16git.dll.a  -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -luuid -lcomdlg32 -ladvapi32 && cd ."
ld.lld: error: undefined symbol: llvm::RecordKeeper::getAllDerivedDefinitionsIfDefined(llvm::StringRef) const
>>> referenced by tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj:(std::__1::__function::__func<$_2, std::__1::allocator<$_2>, bool (llvm::RecordKeeper const&, llvm::raw_ostream&)>::operator()(llvm::RecordKeeper const&, llvm::raw_ostream&))
>>> referenced by tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj:(std::__1::__function::__func<$_3, std::__1::allocator<$_3>, bool (llvm::RecordKeeper const&, llvm::raw_ostream&)>::operator()(llvm::RecordKeeper const&, llvm::raw_ostream&))
>>> referenced by tools/mlir/tools/mlir-tblgen/CMakeFiles/mlir-tblgen.dir/AttrOrTypeDefGen.cpp.obj:(std::__1::__function::__func<$_4, std::__1::allocator<$_4>, bool (llvm::RecordKeeper const&, llvm::raw_ostream&)>::operator()(llvm::RecordKeeper const&, llvm::raw_ostream&))
>>> referenced 8 more times

[grosser]

I just checked and I get the same error on a linux system. @nhaehnle, did you try an mlir build with the previously mentioned cmake options?

[grosser]

@nhaehnle, I wanted to check in briefly if you are getting the same error messages?

[nhaehnle]

I haven't looked at this in a while, sorry. I do plan to cycle back to it eventually, but right now my work scheduling is more stack-based than FIFO and this is fairly low down...

[grosser]

That is perfectly fine. I just wanted to let you know that we are still interested in this.

clang/lib/Support/CMakeLists.txt

 set(LLVM_COMMON_DEPENDS_OLD ${LLVM_COMMON_DEPENDS})
 
 # Drop clang-tablegen-targets from LLVM_COMMON_DEPENDS.
 # so that we could use clangSupport within clang-tblgen and other clang
 # component.
 list(REMOVE_ITEM LLVM_COMMON_DEPENDS clang-tablegen-targets)
 
 set(LLVM_LINK_COMPONENTS
   Support
   )
 
-set(clangSupport_sources
-  RISCVVIntrinsicUtils.cpp
-  )
-
-add_clang_library(clangSupport ${clangSupport_sources})
-
-if (NOT XCODE)
-  add_library(clangSupport_tablegen ALIAS obj.clangSupport)
-elseif (NOT LLVM_LINK_LLVM_DYLIB)
-  add_library(clangSupport_tablegen ALIAS clangSupport)
-else()
-  # Build a version of the support library that does not link against
-  # libLLVM-*.so, to be used by clang-tblgen. This is so clang-tblgen doesn't
-  # link against libLLVMSupport twice (once statically and once via
-  # libLLVM-*.so).
-  add_llvm_library(clangSupport_tablegen
-    BUILDTREE_ONLY STATIC DISABLE_LLVM_LINK_LLVM_DYLIB
-    ${clangSupport_sources})
-endif()
+add_clang_library(clangSupport
+  RISCVVIntrinsicUtils.cpp)
 
 set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS_OLD})

clang/utils/TableGen/CMakeLists.txt

add_tablegen(clang-tblgen CLANG
   ClangTypeNodesEmitter.cpp
   MveEmitter.cpp
   NeonEmitter.cpp
   RISCVVEmitter.cpp
   SveEmitter.cpp
   TableGen.cpp
   )
 
-target_link_libraries(clang-tblgen PRIVATE clangSupport_tablegen)
+target_link_libraries(clang-tblgen PRIVATE clangSupport)
 
 set_target_properties(clang-tblgen PROPERTIES FOLDER "Clang tablegenning")

llvm/cmake/modules/CrossCompile.cmake

add_custom_command(OUTPUT ${${project_name}_${target_name}_BUILD}/CMakeCache.txt
     COMMENT "Configuring ${target_name} ${project_name}...")
 
   add_custom_target(CONFIGURE_${project_name}_${target_name}
     DEPENDS ${${project_name}_${target_name}_BUILD}/CMakeCache.txt)
 
 endfunction()
 
 # Sets up a native build for a tool, used e.g. for cross-compilation and
-# LLVM_OPTIMIZED_TABLEGEN. Always builds in Release.
+# LLVM_OPTIMIZED_TABLEGEN. Always builds in Release and with static linking.
 # - target: The target to build natively
 # - output_path_var: A variable name which receives the path to the built target
 # - DEPENDS: Any additional dependencies for the target
 function(build_native_tool target output_path_var)
   cmake_parse_arguments(ARG "" "" "DEPENDS" ${ARGN})
 
   if(CMAKE_CONFIGURATION_TYPES)
     set(output_path "${${PROJECT_NAME}_NATIVE_BUILD}/Release/bin/${target}")

llvm/cmake/modules/TableGen.cmake

function(add_public_tablegen_target target)
   if(LLVM_COMMON_DEPENDS)
     add_dependencies(${target} ${LLVM_COMMON_DEPENDS})
   endif()
   set_target_properties(${target} PROPERTIES FOLDER "Tablegenning")
   set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS} ${target} PARENT_SCOPE)
 endfunction()
 
 macro(add_tablegen target project)
-  cmake_parse_arguments(ADD_TABLEGEN "" "DESTINATION;EXPORT" "" ${ARGN})
+  cmake_parse_arguments(ADD_TABLEGEN "DISABLE_LLVM_LINK_LLVM_DYLIB" "DESTINATION;EXPORT" "" ${ARGN})
 
   set(${target}_OLD_LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS})
   set(LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS} TableGen)
 
   # CMake doesn't let compilation units depend on their dependent libraries on some generators.
   if(NOT CMAKE_GENERATOR MATCHES "Ninja" AND NOT XCODE)
     # FIXME: It leaks to user, callee of add_tablegen.
     set(LLVM_ENABLE_OBJLIB ON)
   endif()
 
-  add_llvm_executable(${target} DISABLE_LLVM_LINK_LLVM_DYLIB
+  set(add_executable_dylib_opt "")
+  if(ADD_TABLEGEN_DISABLE_LLVM_LINK_LLVM_DYLIB)
+    set(add_executable_dylib_opt "DISABLE_LLVM_LINK_LLVM_DYLIB")
+  endif()
+
+  add_llvm_executable(${target} ${add_executable_dylib_opt}
     ${ADD_TABLEGEN_UNPARSED_ARGUMENTS})
   set(LLVM_LINK_COMPONENTS ${${target}_OLD_LLVM_LINK_COMPONENTS})
 
+  unset(add_executable_dylib_opt)
+
   set(${project}_TABLEGEN "${target}" CACHE
       STRING "Native TableGen executable. Saves building one when cross-compiling.")
 
   # Effective tblgen executable to be used:
   set(${project}_TABLEGEN_EXE ${${project}_TABLEGEN} PARENT_SCOPE)
   set(${project}_TABLEGEN_TARGET ${${project}_TABLEGEN} PARENT_SCOPE)
 
   if(LLVM_USE_HOST_TOOLS)

llvm/utils/TableGen/CMakeLists.txt

 add_subdirectory(GlobalISel)
 
 set(LLVM_LINK_COMPONENTS Support)
 
 add_tablegen(llvm-tblgen LLVM
+  DISABLE_LLVM_LINK_LLVM_DYLIB
   DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
   EXPORT LLVM
   AsmMatcherEmitter.cpp
   AsmWriterEmitter.cpp
   AsmWriterInst.cpp
   Attributes.cpp
   CallingConvEmitter.cpp
   CodeEmitterGen.cpp

mlir/lib/Support/CMakeLists.txt

add_mlir_library(MLIRSupport
   LINK_LIBS PUBLIC
   ${LLVM_PTHREAD_LIB})
 
 # This doesn't use add_mlir_library as it is used in mlir-tblgen and else
 # mlir-tblgen ends up depending on mlir-generic-headers.
 add_llvm_library(MLIRSupportIndentedOstream
   IndentedOstream.cpp
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   Support
   )

mlir/lib/TableGen/CMakeLists.txt

-# This library is unusual, since mlir-tblgen depends on it, which is
-# built with DISABLE_LLVM_LINK_LLVM_DYLIB, this must also be built
-# with that option.  Otherwise builds with LLVM_BUILD_LLVM_DYLIB and
-# LLVM_LINK_LLVM_DYLIB fail.  (Note that even if this has no llvm
-# component dependencies, LLVM_LINK_LLVM_DYLIB tends to introduce a
-# dependence on libLLVM.so)  However, it must also be linkable against
-# libMLIR.so in some contexts (see unittests/Tablegen, for instance, which
-# has a dependence on MLIRIR, which must depend on libLLVM.so).  This works
-# in this special case because this library is static.
 
-llvm_add_library(MLIRTableGen STATIC
+llvm_add_library(MLIRTableGen
   Argument.cpp
   Attribute.cpp
   AttrOrTypeDef.cpp
   Builder.cpp
   Class.cpp
   CodeGenHelpers.cpp
   Constraint.cpp
   Dialect.cpp
   Format.cpp
   GenInfo.cpp
   Interfaces.cpp
   Operator.cpp
   Pass.cpp
   Pattern.cpp
   Predicate.cpp
   Region.cpp
   SideEffects.cpp
   Successor.cpp
   Trait.cpp
   Type.cpp
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/TableGen
 )
 
 mlir_check_all_link_libraries(MLIRTableGen)
 
 add_mlir_library_install(MLIRTableGen)

mlir/lib/Tools/PDLL/Parser/CMakeLists.txt

 
 llvm_add_library(MLIRPDLLParser STATIC
   CodeComplete.cpp
   Lexer.cpp
   Parser.cpp
   
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   Support
   TableGen
 
   LINK_LIBS PUBLIC
   MLIRPDLLAST
   MLIRSupport
   MLIRTableGen
   )

mlir/lib/Tools/mlir-tblgen/CMakeLists.txt

 llvm_add_library(MLIRTblgenLib STATIC
   MlirTblgenMain.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Tools/mlir-tblgen
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_COMPONENTS
   TableGen
 
   LINK_LIBS PUBLIC
   MLIRTableGen
   )
 
 mlir_check_all_link_libraries(MLIRTblgenLib)
 
 add_mlir_library_install(MLIRTblgenLib)

mlir/lib/Tools/tblgen-lsp-server/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Demangle
   Support
   TableGen
 )
 
 llvm_add_library(TableGenLspServerLib
   LSPServer.cpp
   TableGenServer.cpp
   TableGenLspServerMain.cpp
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/Tools/tblgen-lsp-server
 
-  DISABLE_LLVM_LINK_LLVM_DYLIB
-
   LINK_LIBS PUBLIC
   MLIRLspServerSupportLib
   MLIRSupport
   )