[PATCH] D74461: Remove static registration for dialects, and the "alwayslink" hack for passes

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/include/mlir/InitAllPasses.h:70
+  createLoopCoalescingPass();
+  createAffineDataCopyGenerationPass(0, 0);
+  createMemRefDataFlowOptPass();
----------------
jpienaar wrote:
> Do you want to add somewhere a comment as to the values/should these have flags associated and should those flags be in this file ... (can also be follow up if so :))
You noticed that this call will never be executed right?


================
Comment at: mlir/lib/Dialect/CMakeLists.txt:31
+# them in tools.
+file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/all_dialects.c "typedef int make_iso_compilers_happy;\n")
+add_llvm_library(MLIRAllDialects
----------------
jpienaar wrote:
> I don't know what this does?
Added a comment: Empty libraries aren't possible with CMake, create a dummy file.


================
Comment at: mlir/lib/Quantizer/Transforms/InferQuantizedTypesPass.cpp:287
+  // Do nothing, this will be enough to force link this file and the static
+  // registration will kick-in. This is temporary while we're refactoring pass
+  // registration to move away from static constructors.
----------------
rriddle wrote:
> What are you moving pass registration to?
At the moment we just removed the "force link" invocation in favor of pulling it objects by referencing their symbols. Ultimately I'd like this to replace the static below though.

I couldn't use a "createXXXXPass" method here because the pass needs a complex data structure in its initializer.


================
Comment at: mlir/test/Dialect/SPIRV/TestAvailability.cpp:217
+namespace mlir {
+void registerConvertToTargetEnvPass() {
+  PassRegistration<ConvertToTargetEnv> convertToTargetEnvPass(
----------------
rriddle wrote:
> Can you not do `void mlir::registerConvertToTargetEnvPass`?
I would need a header declaration for this, but we don't have public headers for the test directory


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74461