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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

Seems like I didn't click "send" when I reviewed this earlier this week, sorry!



================
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))
----------------
mehdi_amini wrote:
> 
This is marked as done, but it isn't (it isn't the only one I think)


================
Comment at: flang/test/Fir/memref-data-flow.fir:61
+// CHECK-LABEL: ^bb1
+// CHECK-NOT:     fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+// CHECK-NOT:     %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
----------------
Are we not expecting a fir.store? If so the check-not is more robust with simply `// CHECK-NOT:     fir.store`

If you need to match the type as well I don't think you need to match the " to ", so `// CHECK-NOT:     fir.store{{.*}}!fir.ref<i32>`

Same logic everywhere...


================
Comment at: flang/test/Fir/memref-data-flow.fir:62
+// CHECK-NOT:     fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+// CHECK-NOT:     %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+// CHECK-LABEL: ^bb2:
----------------
The two CHECK-NOT above apply to bb1, but in the source it is:

```
^bb1(%2: index, %3: index):  // 2 preds: ^bb0, ^bb2
  %4 = arith.cmpi sgt, %3, %c0 : index
  cond_br %4, ^bb2, ^bb3
```

Why are we specifically checking for no load/store in here?


================
Comment at: flang/test/Fir/memref-data-flow.fir:65-66
+// CHECK-NOT:     fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+// CHECK-NOT:     %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+// CHECK:         %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+// CHECK:         fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
----------------
I really don't understand this sequence: we have a CHECK-NOT followed by the identical CHECK.


================
Comment at: flang/test/Fir/memref-data-flow.fir:73
+// CHECK-NOT:     fir.store %{{.*}} to %{{.*}} : !fir.ref<i32>
+// CHECK-NOT:     %{{.*}} = fir.load %{{.*}} : !fir.ref<i32>
+// CHECK:         %{{.*}} = fir.convert %{{.*}} : (i32) -> i64
----------------
This check-not is also followed by some identical checks, again the whole logic behind these checks is beyond me right now.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111288