[mlir][Interfaces][NFC] Simplify and align RegionSuccessor design / API (#174945)

Simplify the design of `RegionSuccessor`. There is no need to store the
`Operation *` pointer when branching out of the region branch op (to the
parent). There is no API to even access the `Operation *` pointer.

Add a new helper function `RegionSuccessor::parent` to construct a
region successor that points to the parent. This aligns the
`RegionSuccessor` design and API with `RegionBranchPoint`:
* Both classes now have a `parent()` helper function.
`ClassName::parent()` can be used in documentation to precisely describe
the source/target of a region branch.
* Both classes now use `nullptr` internally to represent "parent".

This API change also protects against incorrect API usage: users can no
longer pass an incorrect parent op. If a region successor is not a
region of the region branch op, it *must* branch out of region branch op
itself ("parent"). However, the previous API allowed passing other
operations. There was one such API violation in a [test
case](https://github.com/llvm/llvm-project/pull/174945/files#diff-d5717e4a8d7344b2ff77762b8fa480bcfec0eeee97a86195c787d791a6217e13L71).

Also clean up the documentation to use the correct terminology (such as
"successor operands", "successor inputs") consistently.

Note: This PR effectively rolls back some changes from #161575. That PR
introduced `llvm::PointerUnion<Region *, Operation *>
successor{nullptr};`. It is unclear from the commit message why that
change was made.

Note for LLVM integration: You may have to slightly modify
`getSuccessorRegion` implementations: Replace
`RegionSuccessor(getOperation(), getOperation()->getResults())` with
`RegionSuccessor::parent(getResults())`.

clang/lib/CIR/Dialect/IR/CIRDialect.cpp

@@ -324,7 +324,7 @@ void cir::ConditionOp::getSuccessorRegions(
   // Parent is a loop: condition may branch to the body or to the parent op.
   if (auto loopOp = dyn_cast<LoopOpInterface>(getOperation()->getParentOp())) {
     regions.emplace_back(&loopOp.getBody(), loopOp.getBody().getArguments());
-    regions.emplace_back(getOperation(), loopOp->getResults());
+    regions.push_back(RegionSuccessor::parent(loopOp->getResults()));
   }
 
   // Parent is an await: condition may branch to resume or suspend regions.
@@ -1168,8 +1168,7 @@ void cir::IfOp::getSuccessorRegions(mlir::RegionBranchPoint point,
                                     SmallVectorImpl<RegionSuccessor> &regions) {
   // The `then` and the `else` region branch back to the parent operation.
   if (!point.isParent()) {
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
     return;
   }
 
@@ -1219,7 +1218,7 @@ void cir::ScopeOp::getSuccessorRegions(
     mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
   // The only region always branch back to the parent operation.
   if (!point.isParent()) {
-    regions.push_back(RegionSuccessor(getOperation(), getODSResults(0)));
+    regions.push_back(RegionSuccessor::parent(getODSResults(0)));
     return;
   }
 
@@ -1384,8 +1383,7 @@ Block *cir::BrCondOp::getSuccessorForOperands(ArrayRef<Attribute> operands) {
 void cir::CaseOp::getSuccessorRegions(
     mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
   if (!point.isParent()) {
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
     return;
   }
   regions.push_back(RegionSuccessor(&getCaseRegion()));
@@ -1411,8 +1409,7 @@ void cir::CaseOp::build(OpBuilder &builder, OperationState &result,
 void cir::SwitchOp::getSuccessorRegions(
     mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &region) {
   if (!point.isParent()) {
-    region.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    region.push_back(RegionSuccessor::parent(getOperation()->getResults()));
     return;
   }
 
@@ -1626,8 +1623,7 @@ void cir::GlobalOp::getSuccessorRegions(
     mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
   // The `ctor` and `dtor` regions always branch back to the parent operation.
   if (!point.isParent()) {
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
     return;
   }
 
@@ -2312,7 +2308,7 @@ void cir::TernaryOp::getSuccessorRegions(
     mlir::RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
   // The `true` and the `false` region branch back to the parent operation.
   if (!point.isParent()) {
-    regions.push_back(RegionSuccessor(getOperation(), this->getODSResults(0)));
+    regions.push_back(RegionSuccessor::parent(this->getODSResults(0)));
     return;
   }
 
@@ -2617,8 +2613,7 @@ void cir::AwaitOp::getSuccessorRegions(
   // If any index all the underlying regions branch back to the parent
   // operation.
   if (!point.isParent()) {
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
     return;
   }
 
@@ -3532,8 +3527,7 @@ void cir::TryOp::getSuccessorRegions(
     llvm::SmallVectorImpl<mlir::RegionSuccessor> &regions) {
   // The `try` and the `catchers` region branch back to the parent operation.
   if (!point.isParent()) {
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
     return;
   }
 

clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp

@@ -30,7 +30,7 @@ void LoopOpInterface::getLoopOpSuccessorRegions(
 
   // Branching from condition: go to body or exit.
   if (&op.getCond() == parentRegion) {
-    regions.emplace_back(mlir::RegionSuccessor(op, op->getResults()));
+    regions.emplace_back(mlir::RegionSuccessor::parent(op->getResults()));
     regions.emplace_back(&op.getBody(), op.getBody().getArguments());
     return;
   }

flang/lib/Optimizer/Dialect/FIROps.cpp

@@ -4699,7 +4699,7 @@ void fir::IfOp::getSuccessorRegions(
     llvm::SmallVectorImpl<mlir::RegionSuccessor> &regions) {
   // The `then` and the `else` region branch back to the parent operation.
   if (!point.isParent()) {
-    regions.push_back(mlir::RegionSuccessor(getOperation(), getResults()));
+    regions.push_back(mlir::RegionSuccessor::parent(getResults()));
     return;
   }
 
@@ -4710,7 +4710,7 @@ void fir::IfOp::getSuccessorRegions(
   mlir::Region *elseRegion = &this->getElseRegion();
   if (elseRegion->empty())
     regions.push_back(
-        mlir::RegionSuccessor(getOperation(), getOperation()->getResults()));
+        mlir::RegionSuccessor::parent(getOperation()->getResults()));
   else
     regions.push_back(mlir::RegionSuccessor(elseRegion));
 }
@@ -4729,7 +4729,8 @@ void fir::IfOp::getEntrySuccessorRegions(
     if (!getElseRegion().empty())
       regions.emplace_back(&getElseRegion());
     else
-      regions.emplace_back(getOperation(), getOperation()->getResults());
+      regions.push_back(
+          mlir::RegionSuccessor::parent(getOperation()->getResults()));
   }
 }
 

mlir/include/mlir/Interfaces/ControlFlowInterfaces.h

@@ -213,19 +213,19 @@ public:
       : successor(region), inputs(regionInputs) {
     assert(region && "Region must not be null");
   }
+
   /// Initialize a successor that branches back to/out of the parent operation.
   /// The target must be one of the recursive parent operations.
-  RegionSuccessor(Operation *successorOp, Operation::result_range results)
-      : successor(successorOp), inputs(ValueRange(results)) {
-    assert(successorOp && "Successor op must not be null");
+  static RegionSuccessor parent(Operation::result_range results) {
+    return RegionSuccessor(results);
   }
 
   /// Return the given region successor. Returns nullptr if the successor is the
   /// parent operation.
-  Region *getSuccessor() const { return dyn_cast<Region *>(successor); }
+  Region *getSuccessor() const { return successor; }
 
   /// Return true if the successor is the parent operation.
-  bool isParent() const { return isa<Operation *>(successor); }
+  bool isParent() const { return successor == nullptr; }
 
   /// Return the inputs to the successor that are remapped by the exit values of
   /// the current region.
@@ -240,7 +240,11 @@ public:
   }
 
 private:
-  llvm::PointerUnion<Region *, Operation *> successor{nullptr};
+  /// Private constructor to encourage the use of `RegionSuccessor::parent`.
+  RegionSuccessor(Operation::result_range results)
+      : successor(nullptr), inputs(ValueRange(results)) {}
+
+  Region *successor = nullptr;
   ValueRange inputs;
 };
 

mlir/include/mlir/Interfaces/ControlFlowInterfaces.td

@@ -117,26 +117,32 @@ def BranchOpInterface : OpInterface<"BranchOpInterface"> {
 
 def RegionBranchOpInterface : OpInterface<"RegionBranchOpInterface"> {
   let description = [{
-    This interface provides information for region-holding operations that exhibit
-    branching behavior between held regions. I.e., this interface allows for
-    expressing control flow information for region holding operations.
+    This interface provides information for region-holding operations that
+    exhibit branching behavior between held regions. I.e., this interface allows
+    for expressing control flow information for region holding operations.
 
     This interface is meant to model well-defined cases of control-flow and
     value propagation, where what occurs along control-flow edges is assumed to
     be side-effect free.
 
     A "region branch point" indicates a point from which a branch originates. It
-    can indicate either a `RegionBranchTerminatorOpInterface` terminator in any
-    of the immediately nested regions of this op or
-    `RegionBranchPoint::parent()`. In the latter case, the branch originates
-    from outside of the op, i.e., when first executing this op.
+    can indicate:
+    1. A `RegionBranchTerminatorOpInterface` terminator in any of the
+       immediately nested regions of this op.
+    2. `RegionBranchPoint::parent()`: the branch originates from outside of the
+       op, i.e., when first executing this op.
 
-    A "region successor" indicates the target of a branch. It can indicate
-    either a region of this op or this op itself. 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.
+    When branching from a region branch point to a region successor, the
+    "successor operands" to be forwarded from the region branch point can be
+    specified with `getEntrySuccessorOperands` /
+    `RegionBranchTerminatorOpInterface::getSuccessorOperands`.
+
+    A "region successor" indicates the target of a branch. It can indicate:
+    1. A region of this op and a range of entry block arguments ("successor
+       inputs") to which the successor operands are forwarded to.
+    2. `RegionSuccessor::parent()` and a range of op results of this op
+       ("successor inputs") to which the successor operands are forwarded to.
+       The control flow leaves this op.
 
     By default, successor operands and successor block arguments/successor
     results must have the same type. `areTypesCompatible` can be implemented to
@@ -162,19 +168,22 @@ def RegionBranchOpInterface : OpInterface<"RegionBranchOpInterface"> {
     }
     ```
 
-    `scf.for` has one region. The `scf.yield` has two region successors: the
-    region body itself and the `scf.for` op. `%b` is an entry successor
-    operand. `%c` is a successor operand. `%a` is a successor block argument.
-    `%r` is a successor result.
+    `scf.for` has one region. There are two region branch points with two
+    identical region successors:
+    * parent => parent(%r), region0(%a)
+    * `scf.yield` => parent(%r), region0(%a)
+
+    `%a` and %r are successor inputs. `%b` is an entry successor operand. `%c`
+    is a successor operand.
   }];
   let cppNamespace = "::mlir";
 
   let methods = [
     InterfaceMethod<[{
-        Returns the operands of this operation that are forwarded to the region
-        successor's block arguments or this operation's results when branching
-        to `successor`. `successor` is guaranteed to be among the successors that are
-        returned by `getEntrySuccessorRegions`/`getSuccessorRegions(parent())`.
+        Returns the operands of this operation that are forwarded to the
+        successor inputs when branching to `successor`. `successor` is
+        guaranteed to be among the successors that are returned by
+        `getEntrySuccessorRegions`/`getSuccessorRegions(parent())`.
 
         Example: In the above example, this method returns the operand %b of the
         `scf.for` op, regardless of the value of `successor`. I.e., this op always
@@ -226,10 +235,10 @@ def RegionBranchOpInterface : OpInterface<"RegionBranchOpInterface"> {
         by `point`.
 
         Example: In the above example, this method returns the region of the
-        `scf.for` and this operation for either region branch point (`parent`
-        and the region of the `scf.for`). An implementation may choose to filter
-        out region successors when it is statically known (e.g., by examining
-        the operands of this op) that those successors are not branched to.
+        `scf.for` and `parent` for either region branch point. An implementation
+        may choose to filter out region successors when it is statically known
+        (e.g., by examining the operands of this op) that those successors are
+        not branched to.
       }],
       "void", "getSuccessorRegions",
       (ins "::mlir::RegionBranchPoint":$point,
@@ -238,7 +247,6 @@ def RegionBranchOpInterface : OpInterface<"RegionBranchOpInterface"> {
     InterfaceMethod<[{
         Returns the potential region successors when branching from any
         terminator in `region`.
-        These are the regions that may be selected during the flow of control.
       }],
       "void", "getSuccessorRegions",
       (ins "::mlir::Region&":$region,
@@ -255,7 +263,8 @@ def RegionBranchOpInterface : OpInterface<"RegionBranchOpInterface"> {
         }
       }]>,
     InterfaceMethod<[{
-        Returns the potential branching point (predecessors) for a given successor.
+        Returns the potential branching points (predecessors) for a given
+        region successor.
       }],
       "void", "getPredecessors",
       (ins "::mlir::RegionSuccessor":$successor,

mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp

@@ -109,7 +109,7 @@ static void collectUnderlyingAddressValues(OpResult result, unsigned maxDepth,
   if (auto branch = dyn_cast<RegionBranchOpInterface>(op)) {
     LDBG() << "  Processing region branch operation";
     return collectUnderlyingAddressValues2(
-        branch, RegionSuccessor(op, op->getResults()), result,
+        branch, RegionSuccessor::parent(op->getResults()), result,
         result.getResultNumber(), maxDepth, visited, output);
   }
 

mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp

@@ -134,7 +134,7 @@ AbstractSparseForwardDataFlowAnalysis::visitOperation(Operation *op) {
   // The results of a region branch operation are determined by control-flow.
   if (auto branch = dyn_cast<RegionBranchOpInterface>(op)) {
     visitRegionSuccessors(getProgramPointAfter(branch), branch,
-                          /*successor=*/{branch, branch->getResults()},
+                          RegionSuccessor::parent(branch->getResults()),
                           resultLattices);
     return success();
   }
@@ -317,8 +317,8 @@ void AbstractSparseForwardDataFlowAnalysis::visitRegionSuccessors(
           firstIndex = cast<OpResult>(inputs.front()).getResultNumber();
         visitNonControlFlowArgumentsImpl(
             branch,
-            RegionSuccessor(
-                branch, branch->getResults().slice(firstIndex, inputs.size())),
+            RegionSuccessor::parent(
+                branch->getResults().slice(firstIndex, inputs.size())),
             lattices, firstIndex);
       } else {
         if (!inputs.empty())

mlir/lib/Analysis/SliceWalk.cpp

@@ -68,7 +68,7 @@ mlir::getControlFlowPredecessors(Value value) {
     if (!regionOp)
       return std::nullopt;
     // Add the control flow predecessor operands to the work list.
-    RegionSuccessor region(regionOp, regionOp->getResults());
+    RegionSuccessor region = RegionSuccessor::parent(regionOp->getResults());
     SmallVector<Value> predecessorOperands;
     // TODO (#175168): This assumes that there are no non-successor-inputs
     // in front of the op result.

mlir/lib/Dialect/Affine/IR/AffineOps.cpp

@@ -2741,7 +2741,7 @@ void AffineForOp::getSuccessorRegions(
       // From the loop body, if the trip count is one, we can only branch back
       // to the parent.
       if (tripCount == 1) {
-        regions.push_back(RegionSuccessor(getOperation(), getResults()));
+        regions.push_back(RegionSuccessor::parent(getResults()));
         return;
       }
       if (tripCount == 0)
@@ -2752,7 +2752,7 @@ void AffineForOp::getSuccessorRegions(
         return;
       }
       if (tripCount.value() == 0) {
-        regions.push_back(RegionSuccessor(getOperation(), getResults()));
+        regions.push_back(RegionSuccessor::parent(getResults()));
         return;
       }
     }
@@ -2761,7 +2761,7 @@ void AffineForOp::getSuccessorRegions(
   // In all other cases, the loop may branch back to itself or the parent
   // operation.
   regions.push_back(RegionSuccessor(&getRegion(), getRegionIterArgs()));
-  regions.push_back(RegionSuccessor(getOperation(), getResults()));
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 AffineBound AffineForOp::getLowerBound() {
@@ -3150,7 +3150,7 @@ void AffineIfOp::getSuccessorRegions(
         RegionSuccessor(&getThenRegion(), getThenRegion().getArguments()));
     // If the "else" region is empty, branch bach into parent.
     if (getElseRegion().empty()) {
-      regions.push_back(RegionSuccessor(getOperation(), getResults()));
+      regions.push_back(RegionSuccessor::parent(getResults()));
     } else {
       regions.push_back(
           RegionSuccessor(&getElseRegion(), getElseRegion().getArguments()));
@@ -3160,7 +3160,7 @@ void AffineIfOp::getSuccessorRegions(
 
   // If the predecessor is the `else`/`then` region, then branching into parent
   // op is valid.
-  regions.push_back(RegionSuccessor(getOperation(), getResults()));
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 LogicalResult AffineIfOp::verify() {

mlir/lib/Dialect/Async/IR/Async.cpp

@@ -55,7 +55,7 @@ void ExecuteOp::getSuccessorRegions(RegionBranchPoint point,
   if (!point.isParent() &&
       point.getTerminatorPredecessorOrNull()->getParentRegion() ==
           &getBodyRegion()) {
-    regions.push_back(RegionSuccessor(getOperation(), getBodyResults()));
+    regions.push_back(RegionSuccessor::parent(getBodyResults()));
     return;
   }
 

mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp

@@ -564,8 +564,8 @@ BufferDeallocation::updateFunctionSignature(FunctionOpInterface op) {
       op.getFunctionBody().getOps<RegionBranchTerminatorOpInterface>(),
       [&](RegionBranchTerminatorOpInterface branchOp) {
         return branchOp
-            .getSuccessorOperands(RegionSuccessor(
-                op.getOperation(), op.getOperation()->getResults()))
+            .getSuccessorOperands(
+                RegionSuccessor::parent(op.getOperation()->getResults()))
             .getTypes();
       }));
   if (!llvm::all_equal(returnOperandTypes))
@@ -946,7 +946,7 @@ BufferDeallocation::handleInterface(RegionBranchTerminatorOpInterface op) {
   // which condition they are taken, etc.
 
   MutableOperandRange operands = op.getMutableSuccessorOperands(
-      RegionSuccessor(op.getOperation(), op.getOperation()->getResults()));
+      RegionSuccessor::parent(op.getOperation()->getResults()));
 
   SmallVector<Value> updatedOwnerships;
   auto result = deallocation_impl::insertDeallocOpForReturnLike(

mlir/lib/Dialect/EmitC/IR/EmitC.cpp

@@ -878,8 +878,7 @@ void IfOp::getSuccessorRegions(RegionBranchPoint point,
                                SmallVectorImpl<RegionSuccessor> &regions) {
   // The `then` and the `else` region branch back to the parent operation.
   if (!point.isParent()) {
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
     return;
   }
 
@@ -888,8 +887,7 @@ void IfOp::getSuccessorRegions(RegionBranchPoint point,
   // Don't consider the else region if it is empty.
   Region *elseRegion = &this->getElseRegion();
   if (elseRegion->empty())
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
   else
     regions.push_back(RegionSuccessor(elseRegion));
 }
@@ -906,7 +904,8 @@ void IfOp::getEntrySuccessorRegions(ArrayRef<Attribute> operands,
     if (!getElseRegion().empty())
       regions.emplace_back(&getElseRegion());
     else
-      regions.emplace_back(getOperation(), getOperation()->getResults());
+      regions.emplace_back(
+          RegionSuccessor::parent(getOperation()->getResults()));
   }
 }
 

mlir/lib/Dialect/GPU/IR/GPUDialect.cpp

@@ -2398,7 +2398,7 @@ ParseResult WarpExecuteOnLane0Op::parse(OpAsmParser &parser,
 void WarpExecuteOnLane0Op::getSuccessorRegions(
     RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
   if (!point.isParent()) {
-    regions.push_back(RegionSuccessor(getOperation(), getResults()));
+    regions.push_back(RegionSuccessor::parent(getResults()));
     return;
   }
 

mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp

@@ -405,7 +405,7 @@ ParseResult AllocaScopeOp::parse(OpAsmParser &parser, OperationState &result) {
 void AllocaScopeOp::getSuccessorRegions(
     RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &regions) {
   if (!point.isParent()) {
-    regions.push_back(RegionSuccessor(getOperation(), getResults()));
+    regions.push_back(RegionSuccessor::parent(getResults()));
     return;
   }
 

mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp

@@ -411,7 +411,7 @@ getSingleRegionOpSuccessorRegions(Operation *op, Region &region,
     return;
   }
 
-  regions.push_back(RegionSuccessor(op, op->getResults()));
+  regions.push_back(RegionSuccessor::parent(op->getResults()));
 }
 
 void KernelsOp::getSuccessorRegions(RegionBranchPoint point,
@@ -460,13 +460,13 @@ void LoopOp::getSuccessorRegions(RegionBranchPoint point,
       regions.push_back(RegionSuccessor(&getRegion()));
       return;
     }
-    regions.push_back(RegionSuccessor(getOperation(), getResults()));
+    regions.push_back(RegionSuccessor::parent(getResults()));
     return;
   }
 
   // Structured loops: model a loop-shaped region graph similar to scf.for.
   regions.push_back(RegionSuccessor(&getRegion()));
-  regions.push_back(RegionSuccessor(getOperation(), getResults()));
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 //===----------------------------------------------------------------------===//

mlir/lib/Dialect/SCF/IR/SCF.cpp

@@ -307,7 +307,7 @@ void ExecuteRegionOp::getSuccessorRegions(
   }
 
   // Otherwise, the region branches back to the parent operation.
-  regions.push_back(RegionSuccessor(getOperation(), getResults()));
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 //===----------------------------------------------------------------------===//
@@ -337,7 +337,7 @@ void ConditionOp::getSuccessorRegions(
     regions.emplace_back(&whileOp.getAfter(),
                          whileOp.getAfter().getArguments());
   if (!boolAttr || !boolAttr.getValue())
-    regions.emplace_back(whileOp.getOperation(), whileOp.getResults());
+    regions.push_back(RegionSuccessor::parent(whileOp.getResults()));
 }
 
 //===----------------------------------------------------------------------===//
@@ -704,7 +704,7 @@ void ForOp::getSuccessorRegions(RegionBranchPoint point,
   // back into the operation itself. It is possible for loop not to enter the
   // body.
   regions.push_back(RegionSuccessor(&getRegion(), getRegionIterArgs()));
-  regions.push_back(RegionSuccessor(getOperation(), getResults()));
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 SmallVector<Region *> ForallOp::getLoopRegions() { return {&getRegion()}; }
@@ -1827,14 +1827,14 @@ void ForallOp::getSuccessorRegions(RegionBranchPoint point,
     regions.push_back(RegionSuccessor(&getRegion()));
     // However, when there are 0 threads, the control flow may branch back to
     // the parent immediately.
-    regions.emplace_back(getOperation(),
-                         ResultRange{getResults().end(), getResults().end()});
+    regions.push_back(RegionSuccessor::parent(
+        ResultRange{getResults().end(), getResults().end()}));
   } else {
     // In accordance with the semantics of forall, its body is executed in
     // parallel by multiple threads. We should not expect to branch back into
     // the forall body after the region's execution is complete.
-    regions.emplace_back(getOperation(),
-                         ResultRange{getResults().end(), getResults().end()});
+    regions.push_back(RegionSuccessor::parent(
+        ResultRange{getResults().end(), getResults().end()}));
   }
 }
 
@@ -2116,7 +2116,7 @@ void IfOp::getSuccessorRegions(RegionBranchPoint point,
   // The `then` and the `else` region branch back to the parent operation or one
   // of the recursive parent operations (early exit case).
   if (!point.isParent()) {
-    regions.push_back(RegionSuccessor(getOperation(), getResults()));
+    regions.push_back(RegionSuccessor::parent(getResults()));
     return;
   }
 
@@ -2125,8 +2125,7 @@ void IfOp::getSuccessorRegions(RegionBranchPoint point,
   // Don't consider the else region if it is empty.
   Region *elseRegion = &this->getElseRegion();
   if (elseRegion->empty())
-    regions.push_back(
-        RegionSuccessor(getOperation(), getOperation()->getResults()));
+    regions.push_back(RegionSuccessor::parent(getResults()));
   else
     regions.push_back(RegionSuccessor(elseRegion));
 }
@@ -2143,7 +2142,7 @@ void IfOp::getEntrySuccessorRegions(ArrayRef<Attribute> operands,
     if (!getElseRegion().empty())
       regions.emplace_back(&getElseRegion());
     else
-      regions.emplace_back(getOperation(), getResults());
+      regions.emplace_back(RegionSuccessor::parent(getResults()));
   }
 }
 
@@ -3158,8 +3157,8 @@ void ParallelOp::getSuccessorRegions(
   // back into the operation itself. It is possible for loop not to enter the
   // body.
   regions.push_back(RegionSuccessor(&getRegion()));
-  regions.push_back(RegionSuccessor(
-      getOperation(), ResultRange{getResults().end(), getResults().end()}));
+  regions.push_back(RegionSuccessor::parent(
+      ResultRange{getResults().end(), getResults().end()}));
 }
 
 //===----------------------------------------------------------------------===//
@@ -3318,7 +3317,7 @@ void WhileOp::getSuccessorRegions(RegionBranchPoint point,
     return;
   }
 
-  regions.emplace_back(getOperation(), getResults());
+  regions.push_back(RegionSuccessor::parent(getResults()));
   regions.emplace_back(&getAfter(), getAfter().getArguments());
 }
 
@@ -3849,7 +3848,7 @@ void IndexSwitchOp::getSuccessorRegions(
     RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> &successors) {
   // All regions branch back to the parent op.
   if (!point.isParent()) {
-    successors.emplace_back(getOperation(), getResults());
+    successors.push_back(RegionSuccessor::parent(getResults()));
     return;
   }
 

mlir/lib/Dialect/Shape/IR/Shape.cpp

@@ -346,7 +346,7 @@ void AssumingOp::getSuccessorRegions(
   // parent, so return the correct RegionSuccessor purely based on the index
   // being None or 0.
   if (!point.isParent()) {
-    regions.push_back(RegionSuccessor(getOperation(), getResults()));
+    regions.push_back(RegionSuccessor::parent(getResults()));
     return;
   }
 

mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp

@@ -2607,7 +2607,7 @@ void IterateOp::getSuccessorRegions(RegionBranchPoint point,
   // or back into the operation itself.
   regions.push_back(RegionSuccessor(&getRegion(), getRegionIterArgs()));
   // It is possible for loop not to enter the body.
-  regions.push_back(RegionSuccessor(getOperation(), getResults()));
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 void CoIterateOp::build(OpBuilder &builder, OperationState &odsState,

mlir/lib/Dialect/Transform/IR/TransformOps.cpp

@@ -118,7 +118,7 @@ void transform::AlternativesOp::getSuccessorRegions(
                                            : Block::BlockArgListType());
   }
   if (!point.isParent())
-    regions.emplace_back(getOperation(), getOperation()->getResults());
+    regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 void transform::AlternativesOp::getRegionInvocationBounds(
@@ -1747,7 +1747,7 @@ void transform::ForeachOp::getSuccessorRegions(
              &getBody() &&
          "unexpected region index");
   regions.emplace_back(bodyRegion, bodyRegion->getArguments());
-  regions.emplace_back(getOperation(), getOperation()->getResults());
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 OperandRange
@@ -2978,7 +2978,7 @@ void transform::SequenceOp::getSuccessorRegions(
   assert(point.getTerminatorPredecessorOrNull()->getParentRegion() ==
              &getBody() &&
          "unexpected region index");
-  regions.emplace_back(getOperation(), getOperation()->getResults());
+  regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 void transform::SequenceOp::getRegionInvocationBounds(

mlir/lib/Dialect/Transform/TuneExtension/TuneExtensionOps.cpp

@@ -129,7 +129,7 @@ void transform::tune::AlternativesOp::getSuccessorRegions(
       for (Region &alternative : getAlternatives())
         regions.emplace_back(&alternative, Block::BlockArgListType());
   else
-    regions.emplace_back(getOperation(), getOperation()->getResults());
+    regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 void transform::tune::AlternativesOp::getRegionInvocationBounds(

mlir/test/lib/Dialect/Test/TestOpDefs.cpp

@@ -745,7 +745,7 @@ void RegionIfOp::getSuccessorRegions(
         &getJoinRegion())
       regions.push_back(RegionSuccessor(&getJoinRegion(), getJoinArgs()));
     else
-      regions.push_back(RegionSuccessor(getOperation(), getResults()));
+      regions.push_back(RegionSuccessor::parent(getResults()));
     return;
   }
 
@@ -772,7 +772,7 @@ void AnyCondOp::getSuccessorRegions(RegionBranchPoint point,
   if (point.isParent())
     regions.emplace_back(&getRegion());
   else
-    regions.emplace_back(getOperation(), getResults());
+    regions.push_back(RegionSuccessor::parent(getResults()));
 }
 
 void AnyCondOp::getRegionInvocationBounds(
@@ -1242,7 +1242,7 @@ void LoopBlockOp::getSuccessorRegions(
   if (point.isParent())
     return;
 
-  regions.emplace_back(getOperation(), getOperation()->getResults());
+  regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
 }
 
 OperandRange LoopBlockOp::getEntrySuccessorOperands(RegionSuccessor successor) {
@@ -1348,7 +1348,7 @@ void TestStoreWithARegion::getSuccessorRegions(
   if (point.isParent())
     regions.emplace_back(&getBody(), getBody().front().getArguments());
   else
-    regions.emplace_back(getOperation(), getOperation()->getResults());
+    regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
 }
 
 //===----------------------------------------------------------------------===//
@@ -1362,7 +1362,7 @@ void TestStoreWithALoopRegion::getSuccessorRegions(
   // enter the body.
   regions.emplace_back(
       RegionSuccessor(&getBody(), getBody().front().getArguments()));
-  regions.emplace_back(getOperation(), getOperation()->getResults());
+  regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
 }
 
 //===----------------------------------------------------------------------===//

mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp

@@ -68,11 +68,11 @@ struct LoopRegionsOp
       if (region == &(*this)->getRegion(1))
         // This region also branches back to the parent.
         regions.push_back(
-            RegionSuccessor(getOperation()->getParentOp(),
-                            getOperation()->getParentOp()->getResults()));
+            RegionSuccessor::parent(getOperation()->getResults()));
       regions.push_back(RegionSuccessor(region));
     }
   }
+
   using RegionBranchOpInterface::Trait<LoopRegionsOp>::getSuccessorRegions;
 };
 
@@ -92,9 +92,7 @@ struct DoubleLoopRegionsOp
     if (point.getTerminatorPredecessorOrNull()) {
       Region *region =
           point.getTerminatorPredecessorOrNull()->getParentRegion();
-      regions.push_back(
-          RegionSuccessor(getOperation()->getParentOp(),
-                          getOperation()->getParentOp()->getResults()));
+      regions.push_back(RegionSuccessor::parent(getOperation()->getResults()));
       regions.push_back(RegionSuccessor(region));
     }
   }