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

[Jonathan Roelofs via Phabricator via llvm-commits]

jroelofs marked 2 inline comments as done.
jroelofs added inline comments.


================
Comment at: mlir/include/mlir/InitAllTranslations.h:30
+inline void registerAllTranslations() {
+  static bool init_once = []() {
+    registerFromLLVMIRTranslation();
----------------
stephenneuendorffer wrote:
> nit: "init_once" is a little misleading, since this doesn't actually ensure that.
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


================
Comment at: mlir/test/EDSC/CMakeLists.txt:24-30
-whole_archive_link(mlir-edsc-builder-api-test
-  MLIRAffine
-  MLIRLinalgOps
-  MLIRLoopOps
-  MLIRStandardOps
-  MLIRVector
-  MLIRTransforms
----------------
stephenneuendorffer wrote:
> 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.
> 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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77515/new/

https://reviews.llvm.org/D77515