Why scf.forall op doesn't have RegionBranchOp interface?

MLIR
1

RegionBranchOp interface has provided a way to trace control flow between operation’s body region, which has widely used in dataflow framework, alias analysis and bufferization.

It seems strange to me that scf.forall/parallel op doesn’t have RegionBranchOp interface, which will cause analysis error(such as reaching definition analysis based on dense dataflow analysis).

Any help is appreciated, thanks.

2

Probably nobody needed it so far, so the interface was never implemented. Both ops should have it. However, note that the interface will not be as “useful” as for example on scf.for. The RegionBranchOpInterface implementation cannot model the fact that multiple loop iterations are running in parallel (or that there are multiple iterations at all). It will look like implementation of scf.execute_region.

One caveat: Both ops have special terminators that describe how the result value of the loop is constructed. So the “value propagation” aspect of the interface cannot be modeled.

3

Right, this likely applies: Why is “small feature” not available in MLIR?.

4

Thanks for your kindly reply. @mehdi_amini @matthias-springer

It seems “value propagation” cannot disable unless we modify all pass that use RegionBranchOpInterface.

For example, scf.forall will failed in sparse dataflow analysis. interface method ‘getSuccessorRegions’ implementation is hard to choose:

If return opeand values in ‘tensor.parallel_insert’, this will get a fault analysis state. e.g. the analysis state indicates all element in tensor have same value.

If return empty operandRange , dead code analysis will get a empty predessor state (llvm-project/DeadCodeAnalysis.cpp at 85452b5f9b5aba5bdf0259b7f0d7400362f95535 · llvm/llvm-project · GitHub), sparse analysis(llvm-project/SparseAnalysis.cpp at 85452b5f9b5aba5bdf0259b7f0d7400362f95535 · llvm/llvm-project · GitHub) will assert fail.

I think value in RegionSuccessor should have a ‘partial’ or ‘full’ bool state to indicate interface user. Is there a better way?

5

I’m not an expert on RegionBranchOpInterface but are you sure that it doesn’t work without value propagation?

getSuccessorEntryOperands is optional and the documentation of getSuccessorRegions suggests that operands are also optional.

I think something like this should work:

void ParallelOp::getSuccessorRegions(
    std::optional<unsigned> index, ArrayRef<Attribute> operands,
    SmallVectorImpl<RegionSuccessor> &regions) {
  if (!index) {
    regions.push_back(RegionSuccessor(&getRegion()));
    return;
  }
  regions.push_back(RegionSuccessor());
}

The terminator should implement RegionBranchTerminatorOpInterface and return empty successor operands. This may be why you see a failing assertion.

6

This is a good idea, thank you again for your suggestion.