[PATCH] D112445: [fir] Add fir.array_access op
Kiran Chandramohan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 24 06:00:25 PST 2022
kiranchandramohan accepted this revision.
kiranchandramohan added a comment.
I have a few Nit comments. Otherwise LGTM.
================
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
----------------
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.
================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:505
+ auto arrTy = op.sequence().getType().cast<fir::SequenceType>();
+ auto indSize = op.indices().size();
+ if (indSize < arrTy.getDimension())
----------------
Nit: Remove auto
================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:511
+ return op.emitOpError("return type does not match array");
+ auto ty = validArraySubobject(op);
+ if (!ty || fir::ReferenceType::get(ty) != op.getType())
----------------
Nit: Remove auto
================
Comment at: flang/lib/Optimizer/Dialect/FIROps.cpp:513
+ if (!ty || fir::ReferenceType::get(ty) != op.getType())
+ return op.emitOpError("return type and/or indices do not type check");
+ return mlir::success();
----------------
Nit: Can you add a test for this check?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112445/new/
https://reviews.llvm.org/D112445
More information about the llvm-commits
mailing list