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

[Tom Eccles via Phabricator via flang-commits]

tblah added inline comments.


================
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>>>
----------------
jeanPerier wrote:
> tblah wrote:
> > 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".
> I do not have a strong opination either. If it was not for the fir.store / hlfir.assign / llvm store precedent, I would probably have gone LHS "something" RHS.  I am not sure why LLVM store chose to put the value as its first operand.
> 
> I tried your suggestion, and unfortunately it seems "=" is a reserved keyword in MLIR parser and cannot be used here. I get the funny error:
> ````
> region_assign.fir:15:5: error: custom op 'hlfir.region_assign' expected '='
>   } = {
>     ^
> ``` 
> 
> I find "equal" too predicate like, maybe "with" could do, also I find "assign y to x" a bit more natural than "assign x with y" (but I am not a native English speaker).
>  
> Do you see good short word I could use instead of "="? 
How about `hlfir.region_assign LHS from RHS`?

I don't feel strongly about this so feel free to leave it how it is. If we can't use `=`, I can't think of any very good options.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149442