[PATCH] D80358: [MLIR] Add RegionKindInterface

[Stephen Neuendorffer via Phabricator via llvm-commits]

stephenneuendorffer marked 56 inline comments as done.
stephenneuendorffer added a comment.

If there are no further comments, I'd like to commit this.  @bondhugula @mehdi_amini  Do you want to accept?

Note that there's a small amount of new text under "### Regions" where I tried to capture the understanding about how Region Arguments are shared with the Block Arguments of the entry block, but that operation arguments/return values are explicitly decoupled from region arguments and terminators.  Also terminator arguments do not need to match the block arguments of their successor blocks.  All of this is under the control of specific operations and may be arbitrarily defined.



================
Comment at: mlir/docs/LangRef.md:677
+%3 = "op3"(%1) : (i32) -> (i32))
+})
+```
----------------
mehdi_amini wrote:
> The indentation does not seem right in this example?
> The reuse of %2 does not help readability either, just like %4 before %2
Actually, there were a few other minor problems with this testcase, including the fact that it missed out on the fact that:
  %1 = op(%1) 
should be valid, along with:
  %1 = op() { otherop(%1) }

I also turned this into another testcase.


================
Comment at: mlir/docs/LangRef.md:553
+entry block which are not listed as a successor of a terminator
+operation are unreachable and can be removed without affecting the
+semantics of the containing operation.
----------------
rriddle wrote:
> nit: are treated as unreachable and may be
I've reworded this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80358