Skip to content

Commit

Permalink
[mlir][IR] Fix overload resolution on MSVC build (llvm#84589)
Browse files Browse the repository at this point in the history
llvm#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`.
  • Loading branch information
matthias-springer authored Mar 11, 2024
1 parent d4569d4 commit f1aa783
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 12 deletions.
20 changes: 13 additions & 7 deletions mlir/include/mlir/IR/PatternMatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -648,7 +648,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);
}

Expand All @@ -662,19 +665,22 @@ 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);
}

/// Find uses of `from` within `block` and replace them with `to`. Also notify
/// 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());
Expand Down
4 changes: 2 additions & 2 deletions mlir/lib/Dialect/Linalg/Transforms/DecomposeLinalgOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down
4 changes: 2 additions & 2 deletions mlir/lib/IR/PatternMatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion mlir/lib/Transforms/Utils/RegionUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit f1aa783

Please sign in to comment.