[Mlir-commits] [mlir] [mlir][Interfaces] Use `areTypesCompatible` instead of `mayForwardTypeToSuccessor` (PR #216471)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

This is a follow-up for #<!-- -->215036. The new `mayForwardTypeToSuccessor` interface method is not needed. The existing `areTypesCompatible` interface method can be used. If a type is not supported by a branch op, `areTypesCompatible(x, x)` can return "false".

Assisted-by: Cursor


---
Full diff: https://github.com/llvm/llvm-project/pull/216471.diff


3 Files Affected:

- (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td (+10-10) 
- (modified) mlir/include/mlir/Interfaces/ControlFlowInterfaces.td (+3-16) 
- (modified) mlir/lib/Transforms/Utils/RegionUtils.cpp (+2-3) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
index e670e6699e57d..86b9e6f58ea31 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMOps.td
@@ -769,8 +769,8 @@ def LLVM_InvokeOp
     LLVMFunctionType getCalleeFunctionType();
 
     /// Successor operands are restricted to LLVM-compatible types.
-    bool mayForwardTypeToSuccessor(unsigned index, Type type) {
-      return isCompatibleType(type);
+    bool areTypesCompatible(Type lhs, Type rhs) {
+      return lhs == rhs && isCompatibleType(lhs);
     }
   }];
 }
@@ -1130,8 +1130,8 @@ def LLVM_BrOp : LLVM_TerminatorOp<"br",
   ];
   let extraClassDeclaration = [{
     /// Successor operands are restricted to LLVM-compatible types.
-    bool mayForwardTypeToSuccessor(unsigned index, Type type) {
-      return isCompatibleType(type);
+    bool areTypesCompatible(Type lhs, Type rhs) {
+      return lhs == rhs && isCompatibleType(lhs);
     }
   }];
 }
@@ -1173,8 +1173,8 @@ def LLVM_CondBrOp
   }]>, LLVM_TerminatorPassthroughOpBuilder];
   let extraClassDeclaration = [{
     /// Successor operands are restricted to LLVM-compatible types.
-    bool mayForwardTypeToSuccessor(unsigned index, Type type) {
-      return isCompatibleType(type);
+    bool areTypesCompatible(Type lhs, Type rhs) {
+      return lhs == rhs && isCompatibleType(lhs);
     }
   }];
 }
@@ -1297,8 +1297,8 @@ def LLVM_SwitchOp
     }
 
     /// Successor operands are restricted to LLVM-compatible types.
-    bool mayForwardTypeToSuccessor(unsigned index, Type type) {
-      return isCompatibleType(type);
+    bool areTypesCompatible(Type lhs, Type rhs) {
+      return lhs == rhs && isCompatibleType(lhs);
     }
   }];
 }
@@ -1961,8 +1961,8 @@ def LLVM_IndirectBrOp : LLVM_TerminatorOp<"indirectbr",
   ];
   let extraClassDeclaration = [{
     /// Successor operands are restricted to LLVM-compatible types.
-    bool mayForwardTypeToSuccessor(unsigned index, Type type) {
-      return isCompatibleType(type);
+    bool areTypesCompatible(Type lhs, Type rhs) {
+      return lhs == rhs && isCompatibleType(lhs);
     }
   }];
 }
diff --git a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
index d6aac2114e467..080829238a153 100644
--- a/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
+++ b/mlir/include/mlir/Interfaces/ControlFlowInterfaces.td
@@ -93,27 +93,14 @@ def BranchOpInterface : OpInterface<"BranchOpInterface"> {
     InterfaceMethod<[{
         This method is called to compare types along control-flow edges. By
         default, the types are checked as equal.
+
+        Note: Operations that do not support a certain type for successor
+        operands at all should return "false" if `lhs` / `rhs` is that type.
       }],
       "bool", "areTypesCompatible",
       (ins "::mlir::Type":$lhs, "::mlir::Type":$rhs), [{}],
        [{ return lhs == rhs; }]
     >,
-    InterfaceMethod<[{
-        Returns true if a value of the given type may be appended to the
-        forwarded operands of the successor at the given index, i.e. whether
-        the operation could pass such a value along the corresponding
-        control-flow edge. Transformations that thread new values across a
-        branch, such as block merging, must check this before appending to
-        `getSuccessorOperands`: an operation whose successor operands are
-        constrained to a subset of types (the LLVM dialect terminators, for
-        example, only forward LLVM-compatible values) would otherwise be
-        rewritten into an operation that no longer verifies. The default
-        implementation accepts any type.
-      }],
-      "bool", "mayForwardTypeToSuccessor",
-      (ins "unsigned":$index, "::mlir::Type":$type), [{}],
-      /*defaultImplementation=*/[{ return true; }]
-    >,
   ];
 
   let verify = [{
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index b63a50f6af0a1..349a03f897441 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -834,15 +834,14 @@ LogicalResult BlockMergeCluster::addToCluster(BlockEquivalenceData &blockData) {
 /// Returns true if the predecessor terminators of the given block can have
 /// their operands updated by appending values of the given types: each must
 /// implement BranchOpInterface and be willing to forward every one of the
-/// types to the block.
+/// types to the block (`areTypesCompatible(T, T)`).
 static bool ableToUpdatePredOperands(Block *block, ArrayRef<Type> types) {
   for (auto it = block->pred_begin(), e = block->pred_end(); it != e; ++it) {
     auto branch = dyn_cast<BranchOpInterface>((*it)->getTerminator());
     if (!branch)
       return false;
-    unsigned succIndex = it.getSuccessorIndex();
     for (Type type : types)
-      if (!branch.mayForwardTypeToSuccessor(succIndex, type))
+      if (!branch.areTypesCompatible(type, type))
         return false;
   }
   return true;

``````````

</details>


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