[PATCH] D111288: [fir] Add data flow optimization pass

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: flang/lib/Optimizer/Transforms/MemRefDataFlowOpt.cpp:29
+  std::vector<OpT> ops;
+  for (auto *user : v.getUsers())
+    if (auto op = dyn_cast<OpT>(user))
----------------



================
Comment at: flang/lib/Optimizer/Transforms/MemRefDataFlowOpt.cpp:48
+
+    for (auto &storeOp : storeOps)
+      if (domInfo->dominates(storeOp, loadOp))
----------------
Not sure why it is a reference, and `auto` does not help readability so why not spell out the type? (here and everywhere, in all the upstreaming patches please).


================
Comment at: flang/lib/Optimizer/Transforms/MemRefDataFlowOpt.cpp:52
+
+    if (candidateSet.size() == 0)
+      return {};
----------------



================
Comment at: flang/lib/Optimizer/Transforms/MemRefDataFlowOpt.cpp:84
+                                          std::vector<ReadOp> &&loadOps) {
+    for (auto &loadOp : loadOps) {
+      if (domInfo->dominates(storeOp, loadOp))
----------------
It is a bit misleading that the variable name is loadOps but contains instances of `ReadOp` (while there is also a `LoadOp` class...)


================
Comment at: flang/lib/Optimizer/Transforms/MemRefDataFlowOpt.cpp:106
+      if (maybeStore) {
+        auto storeOp = maybeStore.getValue();
+        LLVM_DEBUG(llvm::dbgs() << "FlangMemDataFlowOpt: In " << f.getName()
----------------



================
Comment at: flang/test/Fir/memref-data-flow.fir:389
+// CHECK:    return
+// CHECK:  }
+
----------------
This looks like purely generated checks, can you minimize these?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111288