[PATCH] D75837: Introduce std.execute_region op

Mon Mar 9 07:30:42 PDT 2020

bondhugula marked an inline comment as done.
bondhugula added inline comments.

================
Comment at: mlir/include/mlir/Dialect/StandardOps/IR/Ops.td:981
+      The execute_region operation executes the region held exactly once. The op
+      cannot have any operands, nor does its region have any arguments. All SSA
+      values that dominate the op can be accessed inside. The op's region can
----------------
nicolasvasilache wrote:
> I'll repaste my comment that I don't believe was addressed:
> 
> ```
> To be generally useful for Linalg and other ops with regions that refuse to introduce SSA values prematurely (I.e. that use type information to encode the semantics and delay SSA value creation until inlining) you need both arguments and capture.
> Can this be designed and implemented so it serves today’s needs that are already more general than “just capture”?
> ```
I'm not sure what you may need for LinAlg; so I can't say how it may help there. For the things this op will at least help with, please see the commit summary or the discussion thread for more details - it is based on today's needs but your 'today' may be very different from mine! :-)  I do see the need for ops that take dimensional arguments and captures (and with regions), but their goals are very different from those of this op. I suspect you might be thinking that this op works at a higher level than it really does - so you may need a different op for what you have in mind.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75837