Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][IR][NFC] Make replaceAllUsesWith non-templatized #84722

Conversation

matthias-springer
Copy link
Member

Turn RewriterBase::replaceAllUsesWith into a non-templatized implementation, so that it can be made virtual and be overridden in the ConversionPatternRewriter in a subsequent change.

This change is in preparation of adding dialect conversion support for replaceAllUsesWith.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Mar 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 11, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)

Changes

Turn RewriterBase::replaceAllUsesWith into a non-templatized implementation, so that it can be made virtual and be overridden in the ConversionPatternRewriter in a subsequent change.

This change is in preparation of adding dialect conversion support for replaceAllUsesWith.


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

1 Files Affected:

  • (modified) mlir/include/mlir/IR/PatternMatch.h (+6-4)
diff --git a/mlir/include/mlir/IR/PatternMatch.h b/mlir/include/mlir/IR/PatternMatch.h
index c1408c3f90a53b..2be1e2e2b40276 100644
--- a/mlir/include/mlir/IR/PatternMatch.h
+++ b/mlir/include/mlir/IR/PatternMatch.h
@@ -615,11 +615,13 @@ class RewriterBase : public OpBuilder {
   /// Find uses of `from` and replace them with `to`. Also notify the listener
   /// about every in-place op modification (for every use that was replaced).
   void replaceAllUsesWith(Value from, Value to) {
-    return replaceAllUsesWith(from.getImpl(), to);
+    for (OpOperand &operand : llvm::make_early_inc_range(from.getUses())) {
+      Operation *op = operand.getOwner();
+      modifyOpInPlace(op, [&]() { operand.set(to); });
+    }
   }
-  template <typename OperandType, typename ValueT>
-  void replaceAllUsesWith(IRObjectWithUseList<OperandType> *from, ValueT &&to) {
-    for (OperandType &operand : llvm::make_early_inc_range(from->getUses())) {
+  void replaceAllUsesWith(Block *from, Block *to) {
+    for (BlockOperand &operand : llvm::make_early_inc_range(from->getUses())) {
       Operation *op = operand.getOwner();
       modifyOpInPlace(op, [&]() { operand.set(to); });
     }

Copy link
Contributor

@ingomueller-net ingomueller-net left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, makes sense. I have contemplated that possibility already when I added the templated version.

@matthias-springer matthias-springer force-pushed the users/matthias-springer/replace_all_op_uses_notification branch from 7b93ec3 to 9d461b5 Compare April 2, 2024 01:45
Base automatically changed from users/matthias-springer/replace_all_op_uses_notification to main April 2, 2024 01:53
Turn `RewriterBase::replaceAllUsesWith` into a non-templatized implementation, so that it can be made virtual and be overridden in the `ConversionPatternRewriter` in a subsequent change.

This change is in preparation of adding dialect conversion support for `replaceAllUsesWith`.
@matthias-springer matthias-springer force-pushed the users/matthias-springer/replace_all_uses_with_nontemplate branch from e70c754 to 48279f3 Compare April 2, 2024 01:55
@matthias-springer matthias-springer merged commit 9067f54 into main Apr 2, 2024
3 of 4 checks passed
@matthias-springer matthias-springer deleted the users/matthias-springer/replace_all_uses_with_nontemplate branch April 2, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants