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

[Pete Steinfeld via Phabricator via flang-commits]

PeteSteinfeld requested changes to this revision.
PeteSteinfeld added a comment.
This revision now requires changes to proceed.

All builds and tests correctly, but it looks like some changes are needed to HLFIROps.td.



================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:533
+    This is meant to be used in combination with the hlfir.copy_out operation
+    that deletes the temporary if it was created and copy the data back if needed.
+    This operation allows passing non contiguous arrays as procedure reference
----------------
Should this be "copies the data"?


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:534-537
+    This operation allows passing non contiguous arrays as procedure reference
+    dummy arguments that must be contiguous, which is possible in Fortran (that
+    mandates that any pointers made to these dummy arguments are invalidated after
+    the call).
----------------
I didn't understand this sentence.


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:541
+    null input box or an input box containing a null address (but it cannot
+    support both at the same time).  In such case, if the input box matches
+    the absence criterion at runtime, the operation will return a null output
----------------
Should read "In such cases ..."


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:579-587
+    /// Does this operation handle a null input box?
+    bool getHandleNullBox() {
+      return getHandleOptional() && *getHandleOptional() == kHandleNullBox;
+    }
+
+    /// Does this operation handle an input box containing a null address?
+    bool getHandleNullAddr() {
----------------
"getHandleNullBox()" is not used anywhere.  Do we need it?

Both of these functions would read more clearly if their names evoked the fact that they return bools.  For example, "handlesNullAddr".

I couldn't find a definition of "getHandleOptional" anywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142342