diff --git a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td --- a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td +++ b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td @@ -154,8 +154,9 @@ either correspond to a constant value for each operand of this operation, or null if that operand is not a constant. If `index` is valid, `operands` corresponds to the entry values of the region at - `index`. Only a region, i.e. a valid `index`, may use the parent - operation as a successor. This method allows for describing which + `index`. The parent operation, i.e. a null `index`, may specify itself + as successor, which indicates that the control flow may not enter any + region at all. This method allows for describing which regions may be executed when entering an operation, and which regions are executed after having executed another region of the parent op. The successor region must be non-empty. diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp --- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp +++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp @@ -2868,10 +2868,13 @@ regions.reserve(2); regions.push_back( RegionSuccessor(&getThenRegion(), getThenRegion().getArguments())); - // Don't consider the else region if it is empty. - if (!getElseRegion().empty()) + // If the "else" region is empty, branch bach into parent. + if (getElseRegion().empty()) { + regions.push_back(getResults()); + } else { regions.push_back( RegionSuccessor(&getElseRegion(), getElseRegion().getArguments())); + } return; } diff --git a/mlir/lib/Dialect/Bufferization/Transforms/BufferViewFlowAnalysis.cpp b/mlir/lib/Dialect/Bufferization/Transforms/BufferViewFlowAnalysis.cpp --- a/mlir/lib/Dialect/Bufferization/Transforms/BufferViewFlowAnalysis.cpp +++ b/mlir/lib/Dialect/Bufferization/Transforms/BufferViewFlowAnalysis.cpp @@ -93,11 +93,11 @@ for (RegionSuccessor &entrySuccessor : entrySuccessors) { // Wire the entry region's successor arguments with the initial // successor inputs. - assert(entrySuccessor.getSuccessor() && - "Invalid entry region without an attached successor region"); registerDependencies( regionInterface.getSuccessorEntryOperands( - entrySuccessor.getSuccessor()->getRegionNumber()), + entrySuccessor.isParent() + ? std::optional() + : entrySuccessor.getSuccessor()->getRegionNumber()), entrySuccessor.getSuccessorInputs()); } diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp --- a/mlir/lib/Dialect/SCF/IR/SCF.cpp +++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp @@ -534,13 +534,8 @@ /// Return operands used when entering the region at 'index'. These operands /// correspond to the loop iterator operands, i.e., those excluding the -/// induction variable. LoopOp only has one region, so 0 is the only valid value -/// for `index`. +/// induction variable. OperandRange ForOp::getSuccessorEntryOperands(std::optional index) { - assert(index && *index == 0 && "invalid region index"); - - // The initial operands map to the loop arguments after the induction - // variable. return getInitArgs(); } @@ -552,15 +547,9 @@ void ForOp::getSuccessorRegions(std::optional index, ArrayRef operands, SmallVectorImpl ®ions) { - // If the predecessor is the ForOp, branch into the body using the iterator - // arguments. - if (!index) { - regions.push_back(RegionSuccessor(&getLoopBody(), getRegionIterArgs())); - return; - } - - // Otherwise, the loop may branch back to itself or the parent operation. - assert(*index == 0 && "expected loop region"); + // Both the operation itself and the region may be branching into the body or + // back into the operation itself. It is possible for loop not to enter the + // body. regions.push_back(RegionSuccessor(&getLoopBody(), getRegionIterArgs())); regions.push_back(RegionSuccessor(getResults())); } @@ -1728,14 +1717,10 @@ void ForallOp::getSuccessorRegions(std::optional index, ArrayRef operands, SmallVectorImpl ®ions) { - // If the predecessor is ForallOp, branch into the body with empty arguments. - if (!index) { - regions.push_back(RegionSuccessor(&getRegion())); - return; - } - - // Otherwise, the loop should branch back to the parent operation. - assert(*index == 0 && "expected loop region"); + // Both the operation itself and the region may be branching into the body or + // back into the operation itself. It is possible for loop not to enter the + // body. + regions.push_back(RegionSuccessor(&getRegion())); regions.push_back(RegionSuccessor()); } @@ -2031,9 +2016,12 @@ } else { // If the condition isn't constant, both regions may be executed. regions.push_back(RegionSuccessor(&getThenRegion())); - // If the else region does not exist, it is not a viable successor. + // If the else region does not exist, it is not a viable successor, so the + // control will go back to this operation instead. if (elseRegion) regions.push_back(RegionSuccessor(elseRegion)); + else + regions.push_back(RegionSuccessor()); return; } @@ -3040,15 +3028,10 @@ void ParallelOp::getSuccessorRegions( std::optional index, ArrayRef operands, SmallVectorImpl ®ions) { - // If the predecessor is ParallelOp, branch into the body with empty - // arguments. - if (!index) { - regions.push_back(RegionSuccessor(&getRegion())); - return; - } - - assert(*index == 0 && "expected loop region"); - // Otherwise, the loop should branch back to the parent operation. + // Both the operation itself and the region may be branching into the body or + // back into the operation itself. It is possible for loop not to enter the + // body. + regions.push_back(RegionSuccessor(&getRegion())); regions.push_back(RegionSuccessor()); } diff --git a/mlir/test/Analysis/DataFlow/test-next-access.mlir b/mlir/test/Analysis/DataFlow/test-next-access.mlir --- a/mlir/test/Analysis/DataFlow/test-next-access.mlir +++ b/mlir/test/Analysis/DataFlow/test-next-access.mlir @@ -70,10 +70,7 @@ func.func @loop(%arg0: memref, %arg1: f32, %arg2: index, %arg3: index, %arg4: index) -> f32 { %c0 = arith.constant 0.0 : f32 // CHECK: name = "pre" - // CHECK-SAME: next_access = {{\[}}["loop"], "unknown"] - // The above is not entirely correct when the loop has 0 iterations, but - // the region control flow specificaiton is currently incapable of - // specifying that. + // CHECK-SAME: next_access = {{\[}}["outside", "loop"], "unknown"] memref.load %arg0[%arg4] {name = "pre"} : memref %l = scf.for %i = %arg2 to %arg3 step %arg4 iter_args(%ia = %c0) -> (f32) { // CHECK: name = "loop" @@ -90,10 +87,7 @@ // CHECK-LABEL: @conditional func.func @conditional(%cond: i1, %arg0: memref) { // CHECK: name = "pre" - // CHECK-SAME: next_access = {{\[}}["then"]] - // The above is not entirely correct when the condition is false, but - // the region control flow specificaiton is currently incapable of - // specifying that. + // CHECK-SAME: next_access = {{\[}}["post", "then"]] memref.load %arg0[] {name = "pre"}: memref scf.if %cond { // CHECK: name = "then" @@ -126,10 +120,7 @@ func.func @dead_conditional(%arg0: memref) { %false = arith.constant 0 : i1 // CHECK: name = "pre" - // CHECK-SAME: next_access = ["unknown"] - // The above is not entirely correct when the condition is false, but - // the region control flow specificaiton is currently incapable of - // specifying that. + // CHECK-SAME: next_access = {{\[}}["post"]] memref.load %arg0[] {name = "pre"}: memref scf.if %false { // CHECK: name = "then" @@ -301,10 +292,7 @@ // CHECK-LABEL: @recursive_call func.func @recursive_call(%arg0: memref, %cond: i1) { // CHECK: name = "pre" - // CHECK-SAME: next_access = {{\[}}["pre"]] - // The above is not entirely correct when the condition is false, but - // the region control flow specificaiton is currently incapable of - // specifying that. + // CHECK-SAME: next_access = {{\[}}["post", "pre"]] memref.load %arg0[] {name = "pre"} : memref scf.if %cond { func.call @recursive_call(%arg0, %cond) : (memref, i1) -> ()