[PATCH] D77727: Only insert memref_cast when needed during canonicalization.
Alex Zinenko via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 8 09:12:51 PDT 2020
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
More information about the llvm-commits
mailing list