[Mlir-commits] [mlir] [MLIR] Don't drop attached discardable attributes (PR #111261)

[Mehdi Amini]

================
@@ -4337,11 +4337,16 @@ LogicalResult PackOp::canonicalize(PackOp packOp, PatternRewriter &rewriter) {
       dest =
           rewriter.create<tensor::CastOp>(loc, newDestType, packOp.getDest());
     }
-    Value newOp = rewriter.create<tensor::PackOp>(
-        loc, source, dest, packOp.getInnerDimsPos(), packOp.getMixedTiles(),
-        packOp.getPaddingValue(), packOp.getOuterDimsPerm());
+    auto clonedPackOp = cast<PackOp>(rewriter.clone(*packOp));
----------------
joker-eph wrote:

The `Operation::getDiscardableAttrDictionary()` method seems like a more direct implementation than `getPrunedAttributeList()` (why isn't this one living in mlir/IR by the way??)

That said, I didn't notice in the review that you're cloning the op here: why is that? Why aren't you just modifying it in place? Since you were doing `rewriter.startOpModification` I was assuming that this is what is happening.
When we can avoid recreating an operation and destroying the original, it's just more efficient.

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