[Mlir-commits] [mlir] 3bf5384 - [mlir][SCF] Fix region branch op interfaces for `scf.forall` and its terminator (#174221)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Tue Jan 13 06:54:47 PST 2026
Author: Matthias Springer
Date: 2026-01-13T15:54:42+01:00
New Revision: 3bf53844cfedf09f22d2786e57ef81d5d41fe249
URL: https://github.com/llvm/llvm-project/commit/3bf53844cfedf09f22d2786e57ef81d5d41fe249
DIFF: https://github.com/llvm/llvm-project/commit/3bf53844cfedf09f22d2786e57ef81d5d41fe249.diff
LOG: [mlir][SCF] Fix region branch op interfaces for `scf.forall` and its terminator (#174221)
`scf.forall` does not completely implement the
`RegionBranchOpInterface`: `scf.forall.in_parallel` does not implement
the `RegionBranchTerminatorOpInterface`.
Incomplete interface implementation is a problem for transformations
that try to understand the control flow by querying the
`RegionBranchOpInterface`.
Detailed explanation of what is wrong with the current implementation.
- There is exactly one region branch point: "parent". `in_parallel` is
not a region branch point because it does not implement the
`RegionBranchTerminatorOpInterface`. (Clarified in #174978.)
- `ForallOp::getSuccessorRegions(parent)` returns one region successors:
the region of the `scf.forall` op.
- Since there is no region branch point in the region, there is no way
to leave the region. This means: once you enter the region, you are
stuck in it indefinitely. (It is unspecified what happens once you are
in the region, but we can be sure that you cannot leave it.)
This commit adds the `RegionBranchTerminatorOpInterface` (via
`ReturnLike`) to `scf.forall.in_parallel` to indicate the after leaving
the region, the control flow returns to the parent. (Note: Only block
terminators in directly nested regions can be region branch terminators,
so `in_parallel` is the only possible op. I.e., `parallel_insert_slice`
cannot be a region branch terminator.)
This commit also removes all successor operands / inputs from the
implementation. The number of successor operands and successor inputs
must match, but `scf.forall.in_parallel` has no operands. Therefore, the
region must also have 0 successor inputs. Therefore, the `scf.forall` op
must also have 0 successor operands.
This commit also adds a missing control flow edge from "parent" to
"parent": in case of 0 threads, the region is not entered.
Depends on #174978.
Added:
Modified:
mlir/include/mlir/Dialect/SCF/IR/SCFOps.td
mlir/lib/Dialect/SCF/IR/SCF.cpp
mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
Removed:
################################################################################
diff --git a/mlir/include/mlir/Dialect/SCF/IR/SCFOps.td b/mlir/include/mlir/Dialect/SCF/IR/SCFOps.td
index 7c26d78683de9..48a377491df02 100644
--- a/mlir/include/mlir/Dialect/SCF/IR/SCFOps.td
+++ b/mlir/include/mlir/Dialect/SCF/IR/SCFOps.td
@@ -650,13 +650,6 @@ def ForallOp : SCF_Op<"forall", [
/// Returns true if the mapping specified for this forall op is linear.
bool usesLinearMapping();
-
- /// RegionBranchOpInterface
-
- OperandRange getEntrySuccessorOperands(RegionSuccessor successor) {
- return getInits();
- }
-
}];
}
@@ -668,6 +661,7 @@ def InParallelOp : SCF_Op<"forall.in_parallel", [
Pure,
Terminator,
DeclareOpInterfaceMethods<InParallelOpInterface>,
+ ReturnLike,
HasParent<"ForallOp">,
] # GraphRegionNoTerminator.traits> {
let summary = "terminates a `forall` block";
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index 81a167d3514a3..f035eafa461c0 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -1816,21 +1816,26 @@ void ForallOp::getCanonicalizationPatterns(RewritePatternSet &results,
ForallOpReplaceConstantInductionVar>(context);
}
-/// Given the region at `index`, or the parent operation if `index` is None,
-/// return the successor regions. These are the regions that may be selected
-/// during the flow of control. `operands` is a set of optional attributes that
-/// correspond to a constant value for each operand, or null if that operand is
-/// not a constant.
void ForallOp::getSuccessorRegions(RegionBranchPoint point,
SmallVectorImpl<RegionSuccessor> ®ions) {
- // 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.
- if (point.isParent())
- regions.push_back(RegionSuccessor(&getRegion(), getRegionIterArgs()));
- else
- regions.push_back(
- RegionSuccessor(getOperation(), getOperation()->getResults()));
+ // There are two region branch points:
+ // 1. "parent": entering the forall op for the first time.
+ // 2. scf.in_parallel terminator
+ if (point.isParent()) {
+ // When first entering the forall op, the control flow typically branches
+ // into the forall body. (In parallel for multiple threads.)
+ 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()});
+ } 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()});
+ }
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
index 1ae6855351506..7ce5c0f9e3d5a 100644
--- a/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
+++ b/mlir/test/Analysis/DataFlow/test-dead-code-analysis.mlir
@@ -269,3 +269,17 @@ func.func @test_dca_doesnt_crash() -> () {
func.func @test_dca_doesnt_crash_2() -> () attributes {symbol = @notexistant} {
return
}
+
+func.func @test_forall_op_control_flow(%num_threads: index) {
+ // CHECK: test_forall_op_control_flow:
+ // CHECK: region #0
+ // CHECK: ^bb0 = live
+ // CHECK: region_preds: (all) predecessors:
+ // CHECK: scf.forall (%{{.*}}) in (%{{.*}}) {...} {tag = "test_forall_op_control_flow"}
+ // CHECK: op_preds: (all) predecessors:
+ // CHECK: scf.forall (%{{.*}}) in (%{{.*}}) {...} {tag = "test_forall_op_control_flow"}
+ // CHECK: scf.forall.in_parallel {...}
+ scf.forall (%arg0) in (%num_threads) {
+ } {tag = "test_forall_op_control_flow"}
+ return
+}
More information about the Mlir-commits
mailing list