[Mlir-commits] [clang] [flang] [mlir] [mlir][Interfaces] Single interface method to query constant region-based CF (PR #193486)

[Mehdi Amini]

================
@@ -198,31 +205,39 @@ def RegionBranchOpInterface : OpInterface<"RegionBranchOpInterface"> {
       }]
     >,
     InterfaceMethod<[{
-        Returns the potential region successors when first executing the op.
+        Returns the potential region successors when branching from `point`.
+
+        This is a "constants-aware" variant of `getSuccessorRegions`. The
+        `operandConstants` parameter bundles up the constant values of the
+        operands of the parent (region branch) op and any of its region
+        branch terminators. Based on these, the implementation may filter out
+        certain successors that are statically known not to be taken.
 
-        Unlike `getSuccessorRegions`, this method also passes along the
-        constant operands of this op. Based on these, the implementation may
-        filter out certain successors. By default, simply dispatches to
-        `getSuccessorRegions`. `operands` contains an entry for every
-        operand of this op, with a null attribute if the operand has no constant
-        value.
+        By default, this method simply dispatches to `getSuccessorRegions`,
+        ignoring the constant operand information.
 
-        Note: The control flow does not necessarily have to enter any region of
-        this op.
+        Implementations should handle the case where no constant information
+        is available for a given branch point (i.e.,
+        `operandConstants.getOperandConstants(point)` returns an empty range)
+        by falling back to the no-constants overload.
 
         Example: In the above example, this method may return two region
-        region successors: the single region of the `scf.for` op and the
-        `scf.for` operation (that implements this interface). If %lb, %ub, %step
-        are constants and it can be determined the loop does not have any
-        iterations, this method may choose to return only this operation.
-        Similarly, if it can be determined that the loop has at least one
-        iteration, this method may choose to return only the region of the loop.
+        region successors for the "parent" branching point: the single region
+        of the `scf.for` op and the `scf.for` operation (that implements this
+        interface). If %lb, %ub, %step are constants and it can be determined
+        the loop does not have any iterations, this method may choose to return
+        only this operation. Similarly, if it can be determined that the loop
+        has at least one iteration, this method may choose to return only the
+        region of the loop.
       }],
-      "void", "getEntrySuccessorRegions",
-      (ins "::llvm::ArrayRef<::mlir::Attribute>":$operands,
-           "::llvm::SmallVectorImpl<::mlir::RegionSuccessor> &":$regions), [{}],
+      "void", "getSuccessorRegionsWithConstants",
+      (ins "::mlir::RegionBranchPoint":$point,
+           "const ::mlir::RegionBranchPointOperandConstants &"
+             :$operandConstants,
+           "::llvm::SmallVectorImpl<::mlir::RegionSuccessor> &":$regions),
+      [{}],
       /*defaultImplementation=*/[{
-        $_op.getSuccessorRegions(mlir::RegionBranchPoint::parent(), regions);
+        $_op.getSuccessorRegions(point, regions);
       }]
----------------
joker-eph wrote:

Why isn't the default implementation for `getSuccessorRegions` calling into `getSuccessorRegionsWithConstants` with null constants? 

Seems like as a user I would want to be able to implement just `getSuccessorRegionsWithConstants` and not both?

Also, taking this further why both methods should exist when one is just the same with null constants?

https://github.com/llvm/llvm-project/pull/193486