[PATCH] D111337: [fir] Add array value copy pass

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added a comment.

I suspect `flang/test/Fir/array-modify.f90` should have a `.mlir` extension right?



================
Comment at: flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp:755
+        [[maybe_unused]] auto outEleTy =
+            fir::unwrapSequenceType(update.getType());
+        if (auto inChrTy = inEleTy.dyn_cast<fir::CharacterType>()) {
----------------
mehdi_amini wrote:
> `unwrapSequenceType` is a pure function: move this in the assert?
ping on comments?


================
Comment at: flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp:876
+
+    mlir::OwningRewritePatternList patterns2(context);
+    patterns2.insert<ArrayLoadConversion>(context);
----------------
Can you document the two-stages process here? (why there are two stages and not a single one)


================
Comment at: flang/test/Fir/array-modify.f90:128
+// CHECK:           return
+// CHECK:         }
+
----------------
I have the same general concerns with the test here as in the other revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111337