Author: Matthias Springer Date: 2026-01-14T10:57:22+01:00 New Revision: 5f3b40ec7afa40da628edcfd85e934aa0e627718
URL: https://github.com/llvm/llvm-project/commit/5f3b40ec7afa40da628edcfd85e934aa0e627718 DIFF: https://github.com/llvm/llvm-project/commit/5f3b40ec7afa40da628edcfd85e934aa0e627718.diff LOG: [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())`. Added: Modified: clang/lib/CIR/Dialect/IR/CIRDialect.cpp clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp flang/lib/Optimizer/Dialect/FIROps.cpp mlir/include/mlir/Interfaces/ControlFlowInterfaces.h mlir/include/mlir/Interfaces/ControlFlowInterfaces.td mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp mlir/lib/Analysis/SliceWalk.cpp mlir/lib/Dialect/Affine/IR/AffineOps.cpp mlir/lib/Dialect/Async/IR/Async.cpp mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp mlir/lib/Dialect/EmitC/IR/EmitC.cpp mlir/lib/Dialect/GPU/IR/GPUDialect.cpp mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp mlir/lib/Dialect/SCF/IR/SCF.cpp mlir/lib/Dialect/Shape/IR/Shape.cpp mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp mlir/lib/Dialect/Transform/IR/TransformOps.cpp mlir/lib/Dialect/Transform/TuneExtension/TuneExtensionOps.cpp mlir/test/lib/Dialect/Test/TestOpDefs.cpp mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp Removed: ################################################################################ diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index 6c4607abb40e7..f3ed928449746 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/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> ®ions) { // 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> ®ions) { // 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> ®ions) { 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> ®ion) { 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> ®ions) { // 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> ®ions) { // 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> ®ions) { // 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; } diff --git a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp b/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp index 6de51f12837ba..4fe04b36173dc 100644 --- a/clang/lib/CIR/Interfaces/CIRLoopOpInterface.cpp +++ b/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; } diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp index 90c960843787e..a3850de83f614 100644 --- a/flang/lib/Optimizer/Dialect/FIROps.cpp +++ b/flang/lib/Optimizer/Dialect/FIROps.cpp @@ -4699,7 +4699,7 @@ void fir::IfOp::getSuccessorRegions( llvm::SmallVectorImpl<mlir::RegionSuccessor> ®ions) { // 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())); } } diff --git a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.h b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.h index c7eec97a477c9..1e21348b4ea39 100644 --- a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.h +++ b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.h @@ -213,19 +213,19 @@ class RegionSuccessor { : 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 @@ class RegionSuccessor { } 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; }; diff --git a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td index b8559e3afa2dc..d1451552d7b0f 100644 --- a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td +++ b/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. - - 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. + 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. + + 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, diff --git a/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp b/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp index 24cb123e51877..be53a4e56f37a 100644 --- a/mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp +++ b/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); } diff --git a/mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp b/mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp index 9fa9329430736..bc236aa13db04 100644 --- a/mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp +++ b/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()) diff --git a/mlir/lib/Analysis/SliceWalk.cpp b/mlir/lib/Analysis/SliceWalk.cpp index ae2d32f04f408..9baf856186979 100644 --- a/mlir/lib/Analysis/SliceWalk.cpp +++ b/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. diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp index c6addfb010856..df1b93e367fc6 100644 --- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp +++ b/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() { diff --git a/mlir/lib/Dialect/Async/IR/Async.cpp b/mlir/lib/Dialect/Async/IR/Async.cpp index e19b917787df1..11fd87ed925d8 100644 --- a/mlir/lib/Dialect/Async/IR/Async.cpp +++ b/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; } diff --git a/mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp b/mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp index 36a759c279eb7..9e8746cb8ea35 100644 --- a/mlir/lib/Dialect/Bufferization/Transforms/OwnershipBasedBufferDeallocation.cpp +++ b/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( diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp index b0566dd10f490..ca29ff833535d 100644 --- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp +++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp @@ -878,8 +878,7 @@ void IfOp::getSuccessorRegions(RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) { // 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())); } } diff --git a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp index ff2d792f9cebd..ed4be4dad6704 100644 --- a/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp +++ b/mlir/lib/Dialect/GPU/IR/GPUDialect.cpp @@ -2398,7 +2398,7 @@ ParseResult WarpExecuteOnLane0Op::parse(OpAsmParser &parser, void WarpExecuteOnLane0Op::getSuccessorRegions( RegionBranchPoint point, SmallVectorImpl<RegionSuccessor> ®ions) { if (!point.isParent()) { - regions.push_back(RegionSuccessor(getOperation(), getResults())); + regions.push_back(RegionSuccessor::parent(getResults())); return; } diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp index 13310c59f9682..9a604d1f109de 100644 --- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp +++ b/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> ®ions) { if (!point.isParent()) { - regions.push_back(RegionSuccessor(getOperation(), getResults())); + regions.push_back(RegionSuccessor::parent(getResults())); return; } diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp index bb643d6d1466b..50b4d0563faef 100644 --- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp +++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp @@ -411,7 +411,7 @@ getSingleRegionOpSuccessorRegions(Operation *op, Region ®ion, 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())); } //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp index f035eafa461c0..2075cad593abf 100644 --- a/mlir/lib/Dialect/SCF/IR/SCF.cpp +++ b/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; } diff --git a/mlir/lib/Dialect/Shape/IR/Shape.cpp b/mlir/lib/Dialect/Shape/IR/Shape.cpp index f0f22e5ef4a83..7de285976f42f 100644 --- a/mlir/lib/Dialect/Shape/IR/Shape.cpp +++ b/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; } diff --git a/mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp b/mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp index af4d2c37f651c..55c0fbdc6f52f 100644 --- a/mlir/lib/Dialect/SparseTensor/IR/SparseTensorDialect.cpp +++ b/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, diff --git a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp index d02a48b8fa308..4c5461d6f6ee6 100644 --- a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp +++ b/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( diff --git a/mlir/lib/Dialect/Transform/TuneExtension/TuneExtensionOps.cpp b/mlir/lib/Dialect/Transform/TuneExtension/TuneExtensionOps.cpp index 2bd6205c69341..fe81b9a1e7173 100644 --- a/mlir/lib/Dialect/Transform/TuneExtension/TuneExtensionOps.cpp +++ b/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( diff --git a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp b/mlir/test/lib/Dialect/Test/TestOpDefs.cpp index 765e44f3d16f3..40dbb453fb088 100644 --- a/mlir/test/lib/Dialect/Test/TestOpDefs.cpp +++ b/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())); } //===----------------------------------------------------------------------===// diff --git a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp b/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp index 2e6950fca6be2..24cef9afbac1c 100644 --- a/mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp +++ b/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)); } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
