[flang-commits] [PATCH] D149442: [flang][hlfir] Add hlfir.region_assign and its hlfir.yield terminator

[Tom Eccles via Phabricator via flang-commits]

tblah added a comment.

Great work on this. I like the overall idea.  Other than some nits, this looks good to me.



================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:838-844
+    Example: "X = Y",  where "=" is a user defined elemental assignment "foo"
+    taking Y by value.
+    ```
+    hlfir.region_assign {
+      hlfir.yield %y : !fir.box<!fir.array<?x!f32>>
+    } to {
+      hlfir.yield %x : !fir.box<!fir.array<?x!fir.type<t>>>
----------------
I think maybe something like

hlfir.region_assign {
   hlfir.yield %x ...
} = {
  hlfir.yield %y ...
}

Would be easier. I know there is already precedent (fir.store) for "RHS to LHS" but when I read "assign" I will be already thinking "LHS = RHS".

This is a matter of opinion so feel free to continue without changing if you prefer "RHS to LHS".


================
Comment at: flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp:1046
+
+static mlir::ParseResult parseYieldOpCleanup(mlir::OpAsmParser &parser,
+                                             mlir::Region &cleanup) {
----------------
I think this and printYieldOpCleanup are unused. Are they for something added in a later patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149442