[Mlir-commits] [mlir] [mlir][transform] Overhaul `RegionBranchOpInterface` implementations. (PR #111408)

[Matthias Springer]

Ingo =?utf-8?q?Müller?= <ingomueller at google.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/111408 at github.com>


================
@@ -104,16 +104,8 @@ transform::AlternativesOp::getEntrySuccessorOperands(RegionBranchPoint point) {
 
 void transform::AlternativesOp::getSuccessorRegions(
     RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
-  for (Region &alternative : llvm::drop_begin(
----------------
matthias-springer wrote:

I think the `RegionBranchOpInterface` does not support the use case here. When you branch back to the parent op, there is no way to go back into the op a second time.

```
    A "region successor" indicates the target of a branch. It can indicate
    either a region of this op or this op. In the former case, the region
    successor is a region pointer and a range of block arguments to which the
    "successor operands" are forwarded to. In the latter case, the control flow
    leaves this op and the region successor is a range of results of this op to
    which the successor operands are forwarded to.
```

This sentence talks about results of the op, so if we would allow going back into the op, you could set the same results multiple times.
```
    In the latter case, the control flow
    leaves this op and the region successor is a range of results of this op to
    which the successor operands are forwarded to.
```

I think we should clarify this in the documentation of `RegionBranchOpInterface`. (We could also allow it but most transformations on top of `RegionBranchOpInterface` probably assume that you cannot go back into the op.)

Do we have to implement the `RegionBranchOpInterface` on these transform ops at all?

https://github.com/llvm/llvm-project/pull/111408