[PATCH] D112445: [fir] Add fir.array_access op

[Valentin Clement via Phabricator via llvm-commits]

clementval marked 2 inline comments as done.
clementval added inline comments.


================
Comment at: flang/include/flang/Optimizer/Dialect/FIROps.td:1570
+
+    It is only possible to use `array_access` on an `array_load` result value or
+    a value that can be trace back transitively to an `array_load` as the
----------------
mehdi_amini wrote:
> kiranchandramohan wrote:
> > clementval wrote:
> > > kiranchandramohan wrote:
> > > > Can this be verified in the verifier? Or is it too costly due to the transitive look up?
> > > I think that it can be done. Do you mind if we do a separate patch for this?
> > The reason I asked is that sometimes the verification checks which are expensive are in a pass.
> > 
> > That is OK with me. If no immediate plans then please create a ticket or let me know I will. It might be a good task for a beginner.
> It's not clear to me that having such a structural requirement on the IR is a good idea in general: this makes it fragile to transformations in general and is quite concerning.
This was discussed extensively in the array operations documentation patch. Just to make sure I understand your comment in the right way. Do you mean we should not verify such constraint or drop it? If we drop it, I guess the transformation that requires it will check for it and fails if it cannot verify this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112445