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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
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>
----------------
clementval wrote:
> mehdi_amini wrote:
> > awarzynski wrote:
> > > mehdi_amini wrote:
> > > > I really don't understand this sequence: we have a CHECK-NOT followed by the identical CHECK.
> > > IIUC, the first `CHECK-NOT` can safely be removed without affecting the test. From https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-not-directive :
> > > > The “CHECK-NOT:” directive is used to verify that a string doesn’t occur between two matches (or before the first match, or after the last match). 
> > > I also find it confusing, but I believe that it's valid and that it does check that there's no `store-load` chain here.
> > If I had to document your check right now it would be (as I understand them):
> > 
> > ```
> > // Check that there is at least one fir.load followed by at least one fir.store, 
> > // both on i32. The load/store don't have to be related to each other.
> > ```
> > 
> > And I could write the exact same test with:
> > ```
> > // CHECK-LABEL: ^bb2:
> > // CHECK:     fir.load{{.*}} !fir.ref<i32>
> > // CHECK:     fir.store{{.*} }!fir.ref<i32>
> > ```
> > 
> > Do you see an input to FileCheck that would be handled differently by the code you have right now and the above?
> > 
> > Also please provide the kind of plain text english documentation I wrote above on what you're checking in each section.
> > 
> In the initial code we have two sequences of load/store. We want to make sure only one stays. Maybe using 
> ```
> // Verify that all store-load pairs have been eliminated. Other stores and loads are preserved.
> CHECK-COUNT-1: fir.store
> CHECK-COUNT-1: fir.load
> ```
> 
> 
That would make more sense to me.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111288