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

[rriddle]

Summary

This avoids adding any additional global constructors, like cl::opt. There is a temporary exception on IR/, which has a few cl::opts that require a bit of plumbing to remove.

[mehdi_amini]

Nice!

[bondhugula]

mlir/lib/IR/CMakeLists.txt

2

This should be added to test/lib/ as well?

[mehdi_amini]

mlir/lib/IR/CMakeLists.txt

2

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.

[bondhugula]

mlir/lib/IR/CMakeLists.txt

2

tests/lib is part of mlir-opt. Doesn't static init affect the latter's load time?

[mehdi_amini]

mlir/lib/IR/CMakeLists.txt

2

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.

[bondhugula]

mlir/lib/IR/CMakeLists.txt

2

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.

[mehdi_amini]

mlir/lib/IR/CMakeLists.txt

2

If static init has a noticeable impact on anything built with the libraries, it'll be noticed on check-mlir.

I doubt it:

1. it'll be dominated by everything else, note that a large part of LLVM itself is linked in mlir-opt.
2. mlir-opt will still likely run the same amount of registration code, it just gonna do it explicitly instead of relying on the global constructors. To start seeing a difference, I would add if (argc ==1) return 0 at the beginning of main() in mlir-opt so that it becomes a no-op and run it in a loop.

[bondhugula]

mlir/lib/IR/CMakeLists.txt

2

I see - thanks!

[mehdi_amini]

mlir/lib/IR/CMakeLists.txt

2

Another thing we've been having an eye on is the ability to be able to use MLIR in an "embedded" environment: getting something like `int main() { context + pass manager + pass} to be as small as possible in term of binary size (this is also an argument to not have "fat" dialects but keep them in smaller pieces: when you register a dialect you get everything for all the ops even if you only use a subset of it).

Something that we can't strip from the binaries at the moment are the parser/printer (and the custom one for every operation), this is also something that we may look into at some point: a production build of the compiler on the device does not need any printer/parser.

mlir/lib/CMakeLists.txt

+# Enable errors for any global constructors.
+add_flag_if_supported("-Werror=global-constructors" WERROR_GLOBAL_CONSTRUCTOR)
+
 add_subdirectory(Analysis)
 add_subdirectory(Conversion)
 add_subdirectory(Dialect)
 add_subdirectory(EDSC)
 add_subdirectory(ExecutionEngine)
 add_subdirectory(IR)
 add_subdirectory(Interfaces)
 add_subdirectory(Parser)
 add_subdirectory(Pass)
 add_subdirectory(Support)
 add_subdirectory(TableGen)
 add_subdirectory(Target)
 add_subdirectory(Transforms)
 add_subdirectory(Translation)

mlir/lib/IR/CMakeLists.txt

+# TODO: Remove the need for global constructors within IR/
+add_flag_if_supported("-Wno-global-constructors" WNO_GLOBAL_CONSTRUCTOR_MLIR_IR)
+
 file(GLOB globbed *.c *.cpp)
 add_mlir_library(MLIRIR
   ${globbed}
 
   ADDITIONAL_HEADER_DIRS
   ${MLIR_MAIN_INCLUDE_DIR}/mlir/IR
 
   DEPENDS
   MLIRCallInterfacesIncGen
   MLIROpAsmInterfacesIncGen
   )
 target_link_libraries(MLIRIR
   PUBLIC
   MLIRSupport
   LLVMSupport
   )