[flang-commits] [PATCH] D142342: [flang][hlfir] Add hlfir.copy_in and hlfir.copy_out operations

[Slava Zakharin via Phabricator via flang-commits]

vzakhari added inline comments.


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:530
+    Copy a variable into a contiguous temporary if the variable is not
+    an absent optional and is not contiguous at runtime.
+    Otherwise, returns the potentially absent variable storage.
----------------
nit: maybe mention the return value (`result(0)`) in this case, so that "Otherwise" below does not look lonely.


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:560
+    OpBuilder<(ins "mlir::Value":$var, "std::optional<bool>":$handle_optional,
+      "bool":$return_box)>
+  ];
----------------
Can you please document this parameter?  Is the intended usage to produce `!fir.box`, when the temporary value is "don't care" after the call, and `!fir.ref` otherwise?


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:577
+    /// box may contain a null address.
+    static constexpr bool kHandleNullAddr = !kHandleNullBox;
+
----------------
nit: it seems a two-bit enum could have been more clear, but I can accept `bool` for this purpose as well :)


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:598
+    the temporary.
+    The copy back is done if $var is provided $was_copied is true.
+    The deallocation of $temp is done if $was_copied is true.
----------------
typo? `$var is provided **and** $was_copied is true`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142342