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

[Valentin Clement via Phabricator via flang-commits]

clementval added inline comments.


================
Comment at: flang/include/flang/Optimizer/HLFIR/HLFIROps.td:845
+      hlfir.yield %x : !fir.box<!fir.array<?x!fir.type<t>>>
+    } user_defined_assignment (%rhs_elt : !fir.ref<f32>, %lhs_elt: !fir.ref<!fir.type<t>>) {
+      %0 = fir.load %rhs_elt : !fir.ref<f32>
----------------
jeanPerier wrote:
> clementval wrote:
> > Maybe it's just me but I find it a bit counter intuitive to have the rhs on the left and the lhs on the right. 
> Oups, I did not update the format here... This is actually: `(%rhs_elt : !fir.ref<f32>) to (%lhs_elt:!fir.ref<!fir.type<t>>)`, I moved to custom parser to get the `to` and make it clear what is the RHS vs LHS. I followed fir.store/hlfir.assign/... logic of "RHS" to "LHS".
> 
> 
Ok this makes sense with the new format then. 


================
Comment at: flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp:980
+
+mlir::ParseResult hlfir::RegionAssignOp::parse(mlir::OpAsmParser &parser,
+                                               mlir::OperationState &result) {
----------------
jeanPerier wrote:
> clementval wrote:
> > Was it too tricky to do it with the inlined assembly format?
> The I was not sure how to get the `() to ()` part of the user assignment block arguments with assembly format. I thought is was nicer to be heavy here to disambiguate the RHS/LHS.
> 
> Do you know if one can manipulate region arguments in inlined assembly format?
> 
> I also failed to get the terminator to be added on the the user defined region, while the other region are not implicitly terminated, but I did not try hard here.
I don't think you can manipulate region argument with the inlined assembly format. That's a good reason to make it custom. Thanks for the explanation. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149442