[mlir] Remove need for static global ctors from mlir-translate

[jroelofs]

Summary

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

[rriddle]

This is great. I also just sent out https://reviews.llvm.org/D77514 to clean up the internal registration facilities.

[stephenneuendorffer]

Thanks for doing this, it will definitely simplify many things about the way linkage is done.

mlir/include/mlir/InitAllTranslations.h

31

nit: "init_once" is a little misleading, since this doesn't actually ensure that.

mlir/test/EDSC/CMakeLists.txt

24–30

Please verify these changes with BUILD_SHARED_LIBS=on. It should have been the case that the whole_archive_link an link_libraries were mutually exclusive, so I'd expect that all of these need to go in a target_link_libraries instead.

[jroelofs]

clang-format

[jroelofs]

mlir/include/mlir/InitAllTranslations.h

31

Thought that was guaranteed with this pattern C++. What case am I missing?

If multiple threads attempt to initialize the same static local variable concurrently, the initialization occurs exactly once

https://en.cppreference.com/w/cpp/language/storage_duration#Static_local_variables

mlir/test/EDSC/CMakeLists.txt

24–30

BUILD_SHARED_LIBS=ON

Currently building that config again. That that fell over is what led me down this rabbit hole in the first place :)

I'd expect that all of these need to go in a target_link_libraries instead.

Unless I misunderstood, they're there already.

[jroelofs]

mlir/test/EDSC/CMakeLists.txt

24–30

BUILD_SHARED_LIBS=ON

Good call. This broke a bunch of test runners. Investigating.

[stephenneuendorffer]

mlir/include/mlir/InitAllTranslations.h

31

You're right.. I misparsed the code when I scanned it.

[jroelofs]

rebased on top of 4faf05e5a3fcef2b2515270c35fe7692f57c55ca

[jroelofs]

I tried this out with BUILD_SHARED_LIBS=ON, and unfortunately ran into a problem with the way this custom rtti behaves across dso's:

https://github.com/llvm/llvm-project/blob/549e87f3d04bbae91bc7bc38609ce7073e2c8a6d/mlir/include/mlir/Support/STLExtras.h#L101

That variable gets marked dso_local even if the function is inline, meaning that different instances of it in each shared lib have different addresses, breaking the traits implementation.

Discussion on Discord pointed at this thread, which has more info: https://llvm.discourse.group/t/shared-library-support/381/5

Should fixing that be a blocker to landing this patch?

If not, should we have a top-level message indicating that it doesn't work (yet)?

if(BUILD_SHARED_LIBS)
  message(WARNING "This isn't going to behave well at runtime")
endif()

[rriddle]

mlir/lib/Dialect/SPIRV/Serialization/TranslateRegistration.cpp

72

Please remove the extra semicolon here.

[mehdi_amini]

I patched this locally and tried to build with BUILD_SHARED_LIBS=ON but the tests are passing for me? Anything specific to configure?

[mehdi_amini]

Interestingly I reproduce on MacOS but not on Linux

[jroelofs]

review feedback: drop semicolon

[jroelofs]

test update: trying out arcanist

[jroelofs]

In D77515#1965197, @jroelofs wrote:

I tried this out with BUILD_SHARED_LIBS=ON, and unfortunately ran into a problem with the way this custom rtti behaves

snip

Should fixing that be a blocker to landing this patch?

Per discussion on Discord, I'll land this patch without solving that, since it's an incremental improvement and doesn't regress the behavior here.

[jroelofs]

223154d267e2e935d1bbcba77fe222c8ef96e789

mlir/cmake/modules/AddMLIR.cmake

 function(mlir_tablegen ofn)
   tablegen(MLIR ${ARGV})
   set(TABLEGEN_OUTPUT ${TABLEGEN_OUTPUT} ${CMAKE_CURRENT_BINARY_DIR}/${ofn}
       PARENT_SCOPE)
 endfunction()
 
-# TODO: This is to handle the current static registration, but should be
-# factored out a bit.
-function(whole_archive_link target)
-  add_dependencies(${target} ${ARGN})
-  if("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin")
-    set(link_flags "-L${CMAKE_BINARY_DIR}/lib ")
-    FOREACH(LIB ${ARGN})
-      if("${CMAKE_GENERATOR}" STREQUAL "Xcode")
-        string(CONCAT link_flags ${link_flags} "-Wl,-force_load ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/lib${LIB}.a ")
-      else()
-        string(CONCAT link_flags ${link_flags} "-Wl,-force_load ${CMAKE_BINARY_DIR}/lib/lib${LIB}.a ")
-      endif()
-    ENDFOREACH(LIB)
-  elseif(MSVC)
-    FOREACH(LIB ${ARGN})
-      string(CONCAT link_flags ${link_flags} "/WHOLEARCHIVE:${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/lib/${LIB}.lib ")
-    ENDFOREACH(LIB)
-  else()
-    set(link_flags "-L${CMAKE_BINARY_DIR}/lib -Wl,--whole-archive,")
-    FOREACH(LIB ${ARGN})
-      string(CONCAT link_flags ${link_flags} "-l${LIB},")
-    ENDFOREACH(LIB)
-    string(CONCAT link_flags ${link_flags} "--no-whole-archive")
-  endif()
-  set_target_properties(${target} PROPERTIES LINK_FLAGS ${link_flags})
-endfunction(whole_archive_link)
-
 # Declare a dialect in the include directory
 function(add_mlir_dialect dialect dialect_namespace)
   set(LLVM_TARGET_DEFINITIONS ${dialect}.td)
   mlir_tablegen(${dialect}.h.inc -gen-op-decls)
   mlir_tablegen(${dialect}.cpp.inc -gen-op-defs)
   mlir_tablegen(${dialect}Dialect.h.inc -gen-dialect-decls -dialect=${dialect_namespace})
   add_public_tablegen_target(MLIR${dialect}IncGen)
   add_dependencies(mlir-headers MLIR${dialect}IncGen)

mlir/include/mlir/InitAllTranslations.h

+//===- InitAllTranslations.h - MLIR Translations Registration ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file defines a helper to trigger the registration of all translations
+// in and out of MLIR to the system.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef MLIR_INITALLTRANSLATIONS_H
+#define MLIR_INITALLTRANSLATIONS_H
+
+namespace mlir {
+
+void registerFromLLVMIRTranslation();
+void registerToLLVMIRTranslation();
+void registerToSPIRVTranslation();
+void registerToNVVMIRTranslation();
+void registerToROCLDIRTranslation();
+void registerAVX512ToLLVMIRTranslation();
+
+// This function should be called before creating any MLIRContext if one
+// expects all the possible translations to be made available to the context
+// automatically.
+inline void registerAllTranslations() {
+  static bool init_once = []() {
+    registerFromLLVMIRTranslation();
+    registerToLLVMIRTranslation();
+    registerToSPIRVTranslation();
+    registerToNVVMIRTranslation();
+    registerToROCLDIRTranslation();
+    registerAVX512ToLLVMIRTranslation();
+    return true;
+  }();
+  (void)init_once;
+}
+} // namespace mlir
+
+#endif // MLIR_INITALLTRANSLATIONS_H

mlir/include/mlir/Translation.h

 /// Interface of the function that performs file-to-file translation involving
 /// MLIR. The input file is held in the given MemoryBuffer; the output file
 /// should be written to the given raw_ostream. The implementation should create
 /// all MLIR constructs needed during the process inside the given context. This
 /// can be used for round-tripping external formats through the MLIR system.
 using TranslateFunction = std::function<LogicalResult(
     llvm::SourceMgr &sourceMgr, llvm::raw_ostream &output, MLIRContext *)>;
 
-/// Use Translate[ToMLIR|FromMLIR]Registration as a global initializer that
+/// Use Translate[ToMLIR|FromMLIR]Registration as an initializer that
 /// registers a function and associates it with name. This requires that a
 /// translation has not been registered to a given name.
 ///
 /// Usage:
 ///
-///   // At namespace scope.
-///   static TranslateToMLIRRegistration Unused(&MySubCommand, [] { ... });
+///   // At file scope.
+///   namespace mlir {
+///   void registerTRexToMLIRRegistration() {
+///     TranslateToMLIRRegistration Unused(&MySubCommand, [] { ... });
+///   }
+///   } // namespace mlir
 ///
 /// \{
 struct TranslateToMLIRRegistration {
   TranslateToMLIRRegistration(llvm::StringRef name,
                               const TranslateSourceMgrToMLIRFunction &function);
   TranslateToMLIRRegistration(llvm::StringRef name,
                               const TranslateStringRefToMLIRFunction &function);
 };

mlir/lib/Dialect/SPIRV/Serialization/TranslateRegistration.cpp

static OwningModuleRef deserializeModule(const llvm::MemoryBuffer *input,
 
   OwningModuleRef module(ModuleOp::create(FileLineColLoc::get(
       input->getBufferIdentifier(), /*line=*/0, /*column=*/0, context)));
   module->getBody()->push_front(spirvModule->getOperation());
 
   return module;
 }
 
-static TranslateToMLIRRegistration fromBinary(
-    "deserialize-spirv", [](llvm::SourceMgr &sourceMgr, MLIRContext *context) {
+namespace mlir {
+void registerToSPIRVTranslation() {
+  TranslateToMLIRRegistration fromBinary(
+      "deserialize-spirv",
+      [](llvm::SourceMgr &sourceMgr, MLIRContext *context) {
         assert(sourceMgr.getNumBuffers() == 1 && "expected one buffer");
         return deserializeModule(
             sourceMgr.getMemoryBuffer(sourceMgr.getMainFileID()), context);
       });
+}
+} // namespace mlir
 
 //===----------------------------------------------------------------------===//
 // Serialization registration
 //===----------------------------------------------------------------------===//
 
 static LogicalResult serializeModule(ModuleOp module, raw_ostream &output) {
   if (!module)
     return failure();
if (failed(spirv::serialize(spirvModules[0], binary)))
     return failure();
 
   output.write(reinterpret_cast<char *>(binary.data()),
                binary.size() * sizeof(uint32_t));
 
   return mlir::success();
 }
 
-static TranslateFromMLIRRegistration
-    toBinary("serialize-spirv", [](ModuleOp module, raw_ostream &output) {
+namespace mlir {
+void registerFromSPIRVTranslation() {
+  TranslateFromMLIRRegistration toBinary(
+      "serialize-spirv", [](ModuleOp module, raw_ostream &output) {
         return serializeModule(module, output);
       });
+}
+} // namespace mlir
 
 //===----------------------------------------------------------------------===//
 // Round-trip registration
 //===----------------------------------------------------------------------===//
 
 static LogicalResult roundTripModule(llvm::SourceMgr &sourceMgr,
                                      raw_ostream &output,
                                      MLIRContext *context) {
static LogicalResult roundTripModule(llvm::SourceMgr &sourceMgr,
   OwningModuleRef dstModule(ModuleOp::create(FileLineColLoc::get(
       /*filename=*/"", /*line=*/0, /*column=*/0, context)));
   dstModule->getBody()->push_front(spirvModule->getOperation());
   dstModule->print(output);
 
   return mlir::success();
 }
 
-static TranslateRegistration roundtrip(
-    "test-spirv-roundtrip",
-    [](llvm::SourceMgr &sourceMgr, raw_ostream &output, MLIRContext *context) {
+namespace mlir {
+void registerTestRoundtripSPIRV() {
+  TranslateRegistration roundtrip(
+      "test-spirv-roundtrip", [](llvm::SourceMgr &sourceMgr,
+                                 raw_ostream &output, MLIRContext *context) {
         return roundTripModule(sourceMgr, output, context);
       });
+}
+} // namespace mlir

mlir/lib/Target/LLVMIR/ConvertFromLLVMIR.cpp

if (!llvmModule) {
     llvm::raw_string_ostream errStream(errStr);
     err.print(/*ProgName=*/"", errStream);
     emitError(UnknownLoc::get(context)) << errStream.str();
     return {};
   }
   return translateLLVMIRToModule(std::move(llvmModule), context);
 }
 
-static TranslateToMLIRRegistration
-    fromLLVM("import-llvm",
-             [](llvm::SourceMgr &sourceMgr, MLIRContext *context) {
-               return translateLLVMIRToModule(sourceMgr, context);
-             });
+namespace mlir {
+void registerFromLLVMIRTranslation() {
+  TranslateToMLIRRegistration fromLLVM(
+      "import-llvm", [](llvm::SourceMgr &sourceMgr, MLIRContext *context) {
+        return ::translateLLVMIRToModule(sourceMgr, context);
+}
+} // namespace mlir

mlir/lib/Target/LLVMIR/ConvertToLLVMIR.cpp

 #include "llvm/Support/ToolOutputFile.h"
 
 using namespace mlir;
 
 std::unique_ptr<llvm::Module> mlir::translateModuleToLLVMIR(ModuleOp m) {
   return LLVM::ModuleTranslation::translateModule<>(m);
 }
 
-static TranslateFromMLIRRegistration
-    registration("mlir-to-llvmir", [](ModuleOp module, raw_ostream &output) {
+namespace mlir {
+void registerToLLVMIRTranslation() {
+  TranslateFromMLIRRegistration registration(
+      "mlir-to-llvmir", [](ModuleOp module, raw_ostream &output) {
         auto llvmModule = LLVM::ModuleTranslation::translateModule<>(module);
         if (!llvmModule)
           return failure();
 
         llvmModule->print(output, nullptr);
         return success();
       });
+}
+} // namespace mlir

mlir/lib/Target/LLVMIR/ConvertToNVVMIR.cpp

llvm::MDNode *llvmMetadataNode =
         llvm::MDNode::get(llvmModule->getContext(), llvmMetadata);
     llvmModule->getOrInsertNamedMetadata("nvvm.annotations")
         ->addOperand(llvmMetadataNode);
   }
 
   return llvmModule;
 }
 
-static TranslateFromMLIRRegistration
-    registration("mlir-to-nvvmir", [](ModuleOp module, raw_ostream &output) {
+namespace mlir {
+void registerToNVVMIRTranslation() {
+  TranslateFromMLIRRegistration registration(
+      "mlir-to-nvvmir", [](ModuleOp module, raw_ostream &output) {
         auto llvmModule = mlir::translateModuleToNVVMIR(module);
         if (!llvmModule)
           return failure();
 
         llvmModule->print(output, nullptr);
         return success();
       });
+}
+} // namespace mlir

mlir/lib/Target/LLVMIR/ConvertToROCDLIR.cpp

for (auto func :
     llvmFunc->setCallingConv(llvm::CallingConv::AMDGPU_KERNEL);
 
     llvmFunc->addFnAttr("amdgpu-flat-work-group-size", "1, 1024");
   }
 
   return llvmModule;
 }
 
-static TranslateFromMLIRRegistration
-    registration("mlir-to-rocdlir", [](ModuleOp module, raw_ostream &output) {
+namespace mlir {
+void registerToROCLDIRTranslation() {
+  TranslateFromMLIRRegistration registration(
+      "mlir-to-rocdlir", [](ModuleOp module, raw_ostream &output) {
         auto llvmModule = mlir::translateModuleToROCDLIR(module);
         if (!llvmModule)
           return failure();
 
         llvmModule->print(output, nullptr);
         return success();
       });
+}
+} // namespace mlir

mlir/lib/Target/LLVMIR/LLVMAVX512Intr.cpp

 };
 
 std::unique_ptr<llvm::Module> translateLLVMAVX512ModuleToLLVMIR(Operation *m) {
   return LLVM::ModuleTranslation::translateModule<LLVMAVX512ModuleTranslation>(
       m);
 }
 } // end namespace
 
-static TranslateFromMLIRRegistration
-    reg("avx512-mlir-to-llvmir", [](ModuleOp module, raw_ostream &output) {
+namespace mlir {
+void registerAVX512ToLLVMIRTranslation() {
+  TranslateFromMLIRRegistration reg(
+      "avx512-mlir-to-llvmir", [](ModuleOp module, raw_ostream &output) {
         auto llvmModule = translateLLVMAVX512ModuleToLLVMIR(module);
         if (!llvmModule)
           return failure();
 
         llvmModule->print(output, nullptr);
         return success();
       });
+}
+} // namespace mlir

mlir/test/EDSC/CMakeLists.txt

target_link_libraries(mlir-edsc-builder-api-test
   MLIRStandardOps
   MLIRTransforms
   MLIRVector
   LLVMCore
   LLVMSupport
 )
 
 target_include_directories(mlir-edsc-builder-api-test PRIVATE ..)
-
-whole_archive_link(mlir-edsc-builder-api-test
-  MLIRAffine
-  MLIRLinalgOps
-  MLIRLoopOps
-  MLIRStandardOps
-  MLIRVector
-  MLIRTransforms
-)

mlir/test/SDBM/CMakeLists.txt

 add_llvm_executable(mlir-sdbm-api-test
   sdbm-api-test.cpp
 )
 
 llvm_update_compile_flags(mlir-sdbm-api-test)
 
 target_link_libraries(mlir-sdbm-api-test
   PRIVATE
   MLIRIR
   MLIRSDBM
   MLIRSupport
   LLVMCore
   LLVMSupport
 )
 
 target_include_directories(mlir-sdbm-api-test PRIVATE ..)
-
-whole_archive_link(mlir-sdbm-api-test
-  MLIRSDBM
-)

mlir/tools/mlir-shlib/CMakeLists.txt

 if(LLVM_BUILD_LLVM_DYLIB)
   add_llvm_library(MLIR
     SHARED
     ${INSTALL_WITH_TOOLCHAIN}
 
     mlir-shlib.cpp
     )
   target_link_libraries(MLIR PRIVATE LLVM ${LLVM_PTHREAD_LIB})
-  whole_archive_link(MLIR ${mlir_libs})
 endif()

mlir/tools/mlir-translate/CMakeLists.txt

 get_property(dialect_libs GLOBAL PROPERTY MLIR_DIALECT_LIBS)
 set(LIBS
   ${dialect_libs}
   MLIRParser
   MLIRPass
   MLIRSPIRV
   MLIRSPIRVSerialization
   MLIRTargetAVX512
   MLIRTargetLLVMIR
   MLIRTargetNVVMIR
   MLIRTargetROCDLIR
   MLIRTranslation
   MLIRSupport
 )
-set(FULL_LIBS
-  MLIRSPIRVSerialization
-  MLIRTargetAVX512
-  MLIRTargetLLVMIR
-  MLIRTargetNVVMIR
-  MLIRTargetROCDLIR
-)
 add_llvm_tool(mlir-translate
   mlir-translate.cpp
 )
 llvm_update_compile_flags(mlir-translate)
-whole_archive_link(mlir-translate ${FULL_LIBS})
 target_link_libraries(mlir-translate PRIVATE MLIRIR MLIRTranslation ${LIBS} LLVMSupport)

mlir/tools/mlir-translate/mlir-translate.cpp

 //===- mlir-translate.cpp - MLIR Translate Driver -------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This is a command line utility that translates a file from/to MLIR using one
 // of the registered translations.
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/IR/Diagnostics.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/InitAllDialects.h"
+#include "mlir/InitAllTranslations.h"
 #include "mlir/Support/FileUtilities.h"
 #include "mlir/Support/LogicalResult.h"
 #include "mlir/Support/ToolUtilities.h"
 #include "mlir/Translation.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/SourceMgr.h"
 #include "llvm/Support/ToolOutputFile.h"
splitInputFile("split-input-file",
                    llvm::cl::init(false));
 
 static llvm::cl::opt<bool> verifyDiagnostics(
     "verify-diagnostics",
     llvm::cl::desc("Check that emitted diagnostics match "
                    "expected-* lines on the corresponding line"),
     llvm::cl::init(false));
 
+namespace mlir {
+// Defined in the test directory, no public header.
+void registerTestRoundtripSPIRV();
+} // namespace mlir
+
+static void registerTestTranslations() { registerTestRoundtripSPIRV(); }
+
 int main(int argc, char **argv) {
   registerAllDialects();
+  registerAllTranslations();
+  registerTestTranslations();
   llvm::InitLLVM y(argc, argv);
 
   // Add flags for all the registered translations.
   llvm::cl::opt<const TranslateFunction *, false, TranslationParser>
       translationRequested("", llvm::cl::desc("Translation to perform"),
                            llvm::cl::Required);
 
   llvm::cl::ParseCommandLineOptions(argc, argv, "MLIR translation driver\n");

mlir/unittests/Dialect/SPIRV/CMakeLists.txt

 add_mlir_unittest(MLIRSPIRVTests
   DeserializationTest.cpp
   SerializationTest.cpp
 )
 target_link_libraries(MLIRSPIRVTests
   PRIVATE
   MLIRSPIRV
   MLIRSPIRVSerialization)
-
-whole_archive_link(MLIRSPIRVTests MLIRSPIRV)
-

mlir/unittests/SDBM/CMakeLists.txt

 add_mlir_unittest(MLIRSDBMTests
   SDBMTest.cpp
 )
 target_link_libraries(MLIRSDBMTests
   PRIVATE
   MLIRSDBM
 )
-whole_archive_link(MLIRSDBMTests MLIRSDBM)