[mlir][linalg] Do not set insertion point inside padding function

[Groverkss]

Remove insertion point in rewriteAsPaddedOp. There is no gurantee that the sizes provided by the user are before the operation to pad. It's better to let the user handle where to insert the newly created operations, as long as they are after the origin operation to pad.

[llvmbot]

@llvm/pr-subscribers-mlir-linalg

@llvm/pr-subscribers-mlir

Author: Kunwar Grover (Groverkss)

Changes

Remove insertion point in rewriteAsPaddedOp. There is no gurantee that the sizes provided by the user are before the operation to pad. It's better to let the user handle where to insert the newly created operations, as long as they are after the origin operation to pad.

---

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

1 Files Affected:

• (modified) mlir/lib/Dialect/Linalg/Transforms/PadTilingInterface.cpp (-4)

diff --git a/mlir/lib/Dialect/Linalg/Transforms/PadTilingInterface.cpp b/mlir/lib/Dialect/Linalg/Transforms/PadTilingInterface.cpp
index 3e787a2ad0ef5..52ab92f180575 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/PadTilingInterface.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/PadTilingInterface.cpp
@@ -288,10 +288,6 @@ FailureOr<PadTilingInterfaceResult> linalg::rewriteAsPaddedOp(
     return failure();
   }
 
-  OpBuilder::InsertionGuard g(builder);
-  // Set IP after toPad because we also take the dims of toPad's output.
-  builder.setInsertionPointAfter(toPad);
-
   // 1. Get the loopUpperBounds from the TilingInterface.
   SmallVector<Range> iterationDomain = toPad.getIterationDomain(builder);
 

[fabianmcg]

The expectation that the caller is the one setting the insertion point is already how things like rewrite patterns work, so conceptually LGTM.

Please add a comment documenting this is the expectation, also there's a build error because there's a transform not setting the insertion point.

[Groverkss]

The expectation that the caller is the one setting the insertion point is already how things like rewrite patterns work, so conceptually LGTM.

Please add a comment documenting this is the expectation, also there's a build error because there's a transform not setting the insertion point.

Done, I'm guessing since you only had minor comments, it's okay to land. If there are any further comments, happy to address them in a post commit review.