[Mlir-commits] [mlir] [mlir][IR] Fix overload resolution on MSVC build (PR #84589)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

#<!-- -->82629 added additional overloads to `replaceAllUsesWith` and `replaceUsesWithIf`. This caused a build breakage with MSVC when called with ops that can implicitly convert to `Value`.

```
external/llvm-project/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp(881): error C2666: 'mlir::RewriterBase::replaceAllUsesWith': 2 overloads have similar conversions
external/llvm-project/mlir/include\mlir/IR/PatternMatch.h(631): note: could be 'void mlir::RewriterBase::replaceAllUsesWith(mlir::Operation *,mlir::ValueRange)'
external/llvm-project/mlir/include\mlir/IR/PatternMatch.h(626): note: or       'void mlir::RewriterBase::replaceAllUsesWith(mlir::ValueRange,mlir::ValueRange)'
external/llvm-project/mlir/include\mlir/IR/PatternMatch.h(616): note: or       'void mlir::RewriterBase::replaceAllUsesWith(mlir::Value,mlir::Value)'
external/llvm-project/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp(882): note: while trying to match the argument list '(mlir::tensor::ExtractSliceOp, T)'
        with
        [
            T=mlir::Value
        ]
```

Note: The LLVM build bots (Linux and Windows) did not break, this seems to be an issue with `Tools\MSVC\14.29.30133\bin\HostX64\x64\cl.exe`.

This change renames the newly added overloads to `replaceAllOpUsesWith` and `replaceOpUsesWithIf`.

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


4 Files Affected:

- (modified) mlir/include/mlir/IR/PatternMatch.h (+13-7) 
- (modified) mlir/lib/Dialect/Linalg/Transforms/DecomposeLinalgOps.cpp (+2-2) 
- (modified) mlir/lib/IR/PatternMatch.cpp (+2-2) 
- (modified) mlir/lib/Transforms/Utils/RegionUtils.cpp (+1-1) 


``````````diff
diff --git a/mlir/include/mlir/IR/PatternMatch.h b/mlir/include/mlir/IR/PatternMatch.h
index e3500b3f9446d8..8d84ab6100007e 100644
--- a/mlir/include/mlir/IR/PatternMatch.h
+++ b/mlir/include/mlir/IR/PatternMatch.h
@@ -628,7 +628,10 @@ class RewriterBase : public OpBuilder {
     for (auto it : llvm::zip(from, to))
       replaceAllUsesWith(std::get<0>(it), std::get<1>(it));
   }
-  void replaceAllUsesWith(Operation *from, ValueRange to) {
+  // Note: This function cannot be called `replaceAllUsesWith` because the
+  // overload resolution, when called with an op that can be implicitly
+  // converted to a Value, would be ambiguous.
+  void replaceAllOpUsesWith(Operation *from, ValueRange to) {
     replaceAllUsesWith(from->getResults(), to);
   }
 
@@ -642,9 +645,12 @@ class RewriterBase : public OpBuilder {
   void replaceUsesWithIf(ValueRange from, ValueRange to,
                          function_ref<bool(OpOperand &)> functor,
                          bool *allUsesReplaced = nullptr);
-  void replaceUsesWithIf(Operation *from, ValueRange to,
-                         function_ref<bool(OpOperand &)> functor,
-                         bool *allUsesReplaced = nullptr) {
+  // Note: This function cannot be called `replaceOpUsesWithIf` because the
+  // overload resolution, when called with an op that can be implicitly
+  // converted to a Value, would be ambiguous.
+  void replaceOpUsesWithIf(Operation *from, ValueRange to,
+                           function_ref<bool(OpOperand &)> functor,
+                           bool *allUsesReplaced = nullptr) {
     replaceUsesWithIf(from->getResults(), to, functor, allUsesReplaced);
   }
 
@@ -652,9 +658,9 @@ class RewriterBase : public OpBuilder {
   /// the listener about every in-place op modification (for every use that was
   /// replaced). The optional `allUsesReplaced` flag is set to "true" if all
   /// uses were replaced.
-  void replaceUsesWithinBlock(Operation *op, ValueRange newValues, Block *block,
-                              bool *allUsesReplaced = nullptr) {
-    replaceUsesWithIf(
+  void replaceOpUsesWithinBlock(Operation *op, ValueRange newValues,
+                                Block *block, bool *allUsesReplaced = nullptr) {
+    replaceOpUsesWithIf(
         op, newValues,
         [block](OpOperand &use) {
           return block->getParentOp()->isProperAncestor(use.getOwner());
diff --git a/mlir/lib/Dialect/Linalg/Transforms/DecomposeLinalgOps.cpp b/mlir/lib/Dialect/Linalg/Transforms/DecomposeLinalgOps.cpp
index 1658ea67a46077..999359c7fa8724 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/DecomposeLinalgOps.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/DecomposeLinalgOps.cpp
@@ -370,8 +370,8 @@ DecomposeLinalgOp::matchAndRewrite(GenericOp genericOp,
       scalarReplacements.push_back(
           residualGenericOpBody->getArgument(num + origNumInputs));
     bool allUsesReplaced = false;
-    rewriter.replaceUsesWithinBlock(peeledScalarOperation, scalarReplacements,
-                                    residualGenericOpBody, &allUsesReplaced);
+    rewriter.replaceOpUsesWithinBlock(peeledScalarOperation, scalarReplacements,
+                                      residualGenericOpBody, &allUsesReplaced);
     assert(!allUsesReplaced &&
            "peeled scalar operation is erased when it wasnt expected to be");
   }
diff --git a/mlir/lib/IR/PatternMatch.cpp b/mlir/lib/IR/PatternMatch.cpp
index 0a88e40f73ec6c..4079ccc7567256 100644
--- a/mlir/lib/IR/PatternMatch.cpp
+++ b/mlir/lib/IR/PatternMatch.cpp
@@ -122,7 +122,7 @@ void RewriterBase::replaceOp(Operation *op, ValueRange newValues) {
     rewriteListener->notifyOperationReplaced(op, newValues);
 
   // Replace all result uses. Also notifies the listener of modifications.
-  replaceAllUsesWith(op, newValues);
+  replaceAllOpUsesWith(op, newValues);
 
   // Erase op and notify listener.
   eraseOp(op);
@@ -141,7 +141,7 @@ void RewriterBase::replaceOp(Operation *op, Operation *newOp) {
     rewriteListener->notifyOperationReplaced(op, newOp);
 
   // Replace all result uses. Also notifies the listener of modifications.
-  replaceAllUsesWith(op, newOp->getResults());
+  replaceAllOpUsesWith(op, newOp->getResults());
 
   // Erase op and notify listener.
   eraseOp(op);
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index eff8acdfb33d20..e25867b527b716 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -161,7 +161,7 @@ SmallVector<Value> mlir::makeRegionIsolatedFromAbove(
   rewriter.setInsertionPointToStart(newEntryBlock);
   for (auto *clonedOp : clonedOperations) {
     Operation *newOp = rewriter.clone(*clonedOp, map);
-    rewriter.replaceUsesWithIf(clonedOp, newOp->getResults(), replaceIfFn);
+    rewriter.replaceOpUsesWithIf(clonedOp, newOp->getResults(), replaceIfFn);
   }
   rewriter.mergeBlocks(
       entryBlock, newEntryBlock,

``````````

</details>


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