[PATCH] D77727: Only insert memref_cast when needed during canonicalization.

[Alex Zinenko via Phabricator via llvm-commits]

ftynse added inline comments.


================
Comment at: mlir/lib/Dialect/StandardOps/IR/Ops.cpp:311
 
 namespace {
+// Create a memref_cast when needed to convert from newType to the type of
----------------
MLIR/LLVM prefer `static` to anonymous namespaces for functions


================
Comment at: mlir/lib/Dialect/StandardOps/IR/Ops.cpp:320
+  bool canSkipMemrefCast = true;
+  auto oldOp = oldValue.getDefiningOp();
+  auto uses = oldOp->getUses();
----------------
Either `auto *` or `Operation *`. 


================
Comment at: mlir/lib/Dialect/StandardOps/IR/Ops.cpp:330
+    for (auto it = uses.begin(); it != uses.end(); it++)
+      it->get().setType(newValue.getType());
+    rewriter.replaceOp(oldOp, {newValue});
----------------
I'm not certain `setType` is okay inside a rewrite pattern. Can you find another way?


================
Comment at: mlir/lib/Dialect/StandardOps/IR/Ops.cpp:335
+                                                oldValue.getType());
+    rewriter.replaceOp(oldOp, {castOp});
+  }
----------------
bondhugula wrote:
> You could use `replaceOpWithNewOp(oldOp, oldValue.getLoc(), ...)`
Nit: you don't need `oldValue.getLoc()`, the location will be extracted from oldOp.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77727/new/

https://reviews.llvm.org/D77727