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

[Jacques Pienaar via Phabricator via llvm-commits]

jpienaar accepted this revision.
jpienaar marked an inline comment as done.
jpienaar added a comment.
This revision is now accepted and ready to land.

Yay, thanks good step in the right direction.



================
Comment at: mlir/include/mlir/InitAllPasses.h:39
+
+// This function must be called to force registering the MLIR passes with the
+// global registry.
----------------
s/must be called/may be called/

s/force registering the/register all/ ? 


================
Comment at: mlir/include/mlir/InitAllPasses.h:45
+// individual passes.
+// The global registry is interesting to interact with the command-line.
+inline void registerAllPasses() {
----------------
command-line testing tools ?


================
Comment at: mlir/include/mlir/InitAllPasses.h:48
+
+  // At the moment we still rely on global initializers,
+  // We must reference the passes in such a way that compilers will not
----------------
Sentence fragment ? (, at end) 


================
Comment at: mlir/include/mlir/InitAllPasses.h:52
+  // yet is effectively a NO-OP. As the compiler isn't smart enough
+  // to know that getenv() never returns -1, this will do the job.
+  if (std::getenv("bar") != (char *)-1)
----------------
But you've now documented it inside the compiler, what if the compiler reads its own comments?!?


================
Comment at: mlir/include/mlir/InitAllPasses.h:70
+  createLoopCoalescingPass();
+  createAffineDataCopyGenerationPass(0, 0);
+  createMemRefDataFlowOptPass();
----------------
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 :))


================
Comment at: mlir/include/mlir/Transforms/Passes.h:116
 
-/// Creates a pass which inlines calls and callable operations as defined by the
-/// CallGraph.
+/// Creates a pass which prints the list of op and the number of occurences in
+/// the module.
----------------
list of ops ?


================
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
----------------
I don't know what this does?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74461