[PATCH] D80358: [MLIR] Add RegionKindInterface

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: mlir/docs/LangRef.md:440
+control-flow dependent can be referenced directly and do not need to
+be passed through block arguments.
 
----------------
This defines block in a way that does not seem to me to be compatible with graph regions.


================
Comment at: mlir/docs/LangRef.md:495
+type or attributes. The first block in the region cannot be a
+successor of any other block. The syntax for the region is as follows:
 
----------------
You define a notion of successor here, does it generalize to graph region?


================
Comment at: mlir/docs/LangRef.md:539
+executes until the operation is the terminator operation at the end of
+a block, in which case some other operation will execute.  The
+determination of the next instruction to execute is the 'passing of
----------------
> in which case some other operation will execute

That does not read well to me right now.


================
Comment at: mlir/docs/LangRef.md:595
 
-This can be further restricted using the custom verifier associated with the
-enclosing operation, for example, disallowing references to values defined
-outside the region completely.
-
-### Control Flow
-
-Regions are Single-Entry-Multiple-Exit (SEME). This means that control can only
-flow into the first block of the region, but can flow out of the region at the
-end of any of the contained blocks (This behavior is similar to that of a
-function body in most programming languages). A terminator of a block within a
-region may transfer the control flow to another block in this region, or return
-it to the immediately enclosing op. The semantics of the enclosing op defines
-where the control flow is transmitted next. It may, for example, enter a region
-of the same op, including the same region that returned the control flow.
-
-The enclosing operation determines the way in which control is transmitted into
-the entry block of a Region. The successor to a Region’s exit points may not
-necessarily exist: for example a call to a function that does not return.
-Concurrent or asynchronous execution of Regions is unspecified. Operations may
-define specific rules of execution, e.g. sequential loops or switch cases.
-
-A Region may also enter another region within the enclosing operation. If an
-operation has multiple regions, the semantics of the operation defines into
-which regions the control flows and in which order, if any. An operation may
-transmit control into Regions that were specified in other operations, in
-particular those that defined the values the given operation uses. Thus, such
-operations can be treated opaquely in the enclosing control flow graph,
-providing a level of control flow isolation similar to that of the call
-operation.
+#### Operations with Multiple Regions
+
----------------
It is interesting that this ends up nested under the SSACFG region case


================
Comment at: mlir/docs/LangRef.md:626
+appropriate for representing cyclic relationships between coupled
+values where there is no fundamental order to the relatinoships. For
+instance, operation in a graph region may represent independent
----------------
typo: relatinoships



================
Comment at: mlir/docs/LangRef.md:628
+instance, operation in a graph region may represent independent
+threads of control with values represent streams of data. As usual in
+MLIR, the particular semantics of a region is completely determined by
----------------
I think it should be either `with values representing streams of data.` or `where values represent streams of data.`


================
Comment at: mlir/docs/LangRef.md:656
+})
+```
+
----------------
The story about block terminators, successors, etc. isn't clear to me with respect to graph region. Even the relationship between blocks isn't entirely clear here actually. 


================
Comment at: mlir/include/mlir/IR/Dominance.h:63
   /// Return true if the specified block is reachable from the entry
   /// block of its region.
   bool isReachableFromEntry(Block *a) const;
----------------
The concept of entry block is no longer a guarantee


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80358