[all-commits] [llvm/llvm-project] 138df2: [mlir] Revamp `RegionBranchOpInterface` successor ...

[Markus Böck via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 138df298208a095dc9bb9e5d1e3c67231b0abd77
      https://github.com/llvm/llvm-project/commit/138df298208a095dc9bb9e5d1e3c67231b0abd77
  Author: Markus Böck <markus.bock+llvm at nextsilicon.com>
  Date:   2023-08-10 (Thu, 10 Aug 2023)

  Changed paths:
    M flang/include/flang/Optimizer/Dialect/FIROps.td
    M flang/lib/Optimizer/Dialect/FIROps.cpp
    M mlir/include/mlir/Analysis/DataFlow/DeadCodeAnalysis.h
    M mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
    M mlir/include/mlir/Dialect/Async/IR/AsyncOps.td
    M mlir/include/mlir/Dialect/SCF/IR/SCFOps.td
    M mlir/include/mlir/Dialect/Transform/IR/TransformOps.td
    M mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
    M mlir/lib/Analysis/AliasAnalysis/LocalAliasAnalysis.cpp
    M mlir/lib/Analysis/DataFlow/DeadCodeAnalysis.cpp
    M mlir/lib/Analysis/DataFlow/SparseAnalysis.cpp
    M mlir/lib/Dialect/Affine/IR/AffineOps.cpp
    M mlir/lib/Dialect/Async/IR/Async.cpp
    M mlir/lib/Dialect/Bufferization/Transforms/BufferDeallocation.cpp
    M mlir/lib/Dialect/Bufferization/Transforms/BufferViewFlowAnalysis.cpp
    M mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
    M mlir/lib/Dialect/SCF/IR/SCF.cpp
    M mlir/lib/Dialect/Shape/IR/Shape.cpp
    M mlir/lib/Dialect/Transform/IR/TransformOps.cpp
    M mlir/lib/Dialect/Vector/IR/VectorOps.cpp
    M mlir/lib/Interfaces/ControlFlowInterfaces.cpp
    M mlir/test/lib/Dialect/Test/TestDialect.cpp
    M mlir/test/lib/Dialect/Test/TestOps.td
    M mlir/unittests/Interfaces/ControlFlowInterfacesTest.cpp

  Log Message:
  -----------
  [mlir] Revamp `RegionBranchOpInterface` successor mechanism

The `RegionBranchOpInterface` had a few fundamental issues caused by the API design of `getSuccessorRegions`.

It always required passing values for the `operands` parameter. This is problematic as the operands parameter actually changes meaning depending on which predecessor `index` is referring to. If coming from a region, you'd have to find a `RegionBranchTerminatorOpInterface` in that region, get its operand count, and then create a `SmallVector` of that size.
This is not only inconvenient, but also error-prone, which has lead to a bug in the implementation of a previously existing `getSuccessorRegions` overload.

Additionally, this made the method dual-use, trying to serve two different use-cases: 1) Trying to determine possible control flow edges between regions and 2) Trying to determine the region being branched to based on constant operands.

This patch fixes these issues by changing the interface methods and adding new ones:
* The `operands` argument of `getSuccessorRegions` has been removed. The method is now only responsible for returning possible control flow edges between regions.
* An optional `getEntrySuccessorRegions` method has been added. This is used to determine which regions are branched to from the parent op based on constant operands of the parent op. By default, it calls `getSuccessorRegions`. This is analogous to `getSuccessorForOperands` from `BranchOpInterface`.
* Add `getSuccessorRegions` to `RegionBranchTerminatorOpInterface`. This is used to get the possible successors of the terminator based on constant operands. By default, it calls the containing `RegionBranchOpInterface`s `getSuccessorRegions` method.
* `getSuccessorEntryOperands` was renamed to `getEntrySuccessorOperands` for consistency.

Differential Revision: https://reviews.llvm.org/D157506