[PATCH] D77824: [mlir] Emit errors if global constructors are found within lib/

Sat Apr 11 12:47:27 PDT 2020

bondhugula added inline comments.

================
Comment at: mlir/lib/IR/CMakeLists.txt:2
+# TODO: Remove the need for global constructors within IR/
+add_flag_if_supported("-Wno-global-constructors" WNO_GLOBAL_CONSTRUCTOR_MLIR_IR)
+
----------------
mehdi_amini wrote:
> bondhugula wrote:
> > mehdi_amini wrote:
> > > bondhugula wrote:
> > > > This should be added to test/lib/ as well?
> > > We could, but we don't really have a reason to limit what happens in tests do we?
> > > 
> > > The motivation for doing this right now is about preserving the ability to build and embed the compiler in a product without having these ctors.
> > tests/lib is part of mlir-opt. Doesn't static init affect the latter's load time?
> mlir-opt is a testing tool, not a production use-case. Unless there is a point where it measurably hinders developers velocity I'm not sure we should optimize for it.
mlir-opt is run roughly 1800 times each time 'check-mlir' is run and nearly all of them as you know are on small inputs. If static init has a noticeable impact on anything built with the libraries, it'll be noticed on check-mlir. We can actually verify it by timing check-mlir before and after this sequence of commits. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77824