[mlir][linalg] Use hasPureTensorSemantics in TransposeMatmul methods. #146438
Conversation
The issue is triggered by llvm@ee070d0 that checks `TensorLikeType` when downstream projects use the pattern without registering bufferization::BufferizationDialect. The registeration is needed because the interface implementation for builtin types locate at `BufferizationDialect::initialize()`. However, we do not need to fix it by the registeration. The proper fix is using the linalg method, i.e., hasPureTensorSemantics. Signed-off-by: hanhanW <[email protected]>
|
cc @andrey-golubev (I can't add you as a reviewer.) |
|
@llvm/pr-subscribers-mlir-linalg @llvm/pr-subscribers-mlir Author: Han-Chung Wang (hanhanW) ChangesThe issue is triggered by ee070d0 that checks Full diff: https://github.com/llvm/llvm-project/pull/146438.diff 1 Files Affected:
diff --git a/mlir/lib/Dialect/Linalg/Transforms/TransposeMatmul.cpp b/mlir/lib/Dialect/Linalg/Transforms/TransposeMatmul.cpp
index e624f589917d1..233f180e48be0 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/TransposeMatmul.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/TransposeMatmul.cpp
@@ -38,7 +38,7 @@ FailureOr<Operation *> mlir::linalg::transposeMatmul(RewriterBase &rewriter,
matmulOp, "only matmul ops with non-extended semantics are supported");
}
- if (!bufferization::hasTensorSemantics(matmulOp))
+ if (!matmulOp.hasPureTensorSemantics())
return rewriter.notifyMatchFailure(
matmulOp, "only matmul ops with tensors are supported");
@@ -93,7 +93,7 @@ mlir::linalg::transposeBatchMatmul(RewriterBase &rewriter,
batchMatmulOp, "ops with user-defined maps are not supported");
}
- if (!bufferization::hasTensorSemantics(batchMatmulOp))
+ if (!batchMatmulOp.hasPureTensorSemantics())
return rewriter.notifyMatchFailure(
batchMatmulOp, "only matmul ops with tensors are supported");
|
|
The functionality is well tested in transpose-matmul.mlir. I'm not able to reproduce it using the transform dialect. My guess is that the interpreter pass already loads all the upstream dialects. I can only reproduce it with writing a new c++ pass. If it is a requirement, I can do so. But my feeling is that people will delete it in the future. |
There was a problem hiding this comment.
LGTM (I wonder how I didn't catch this when introducing a patch...). Could you add a test for this case so that the regression is covered?
Edit: saw your note regarding the reproduction in a different-scope scenario. Up to you then whether you want a test or not. I guess we could have some precautionary note saying "don't delete this test as it tests a different setup".
The issue is triggered by ee070d0 that checks
TensorLikeTypewhen downstream projects use the pattern without registering bufferization::BufferizationDialect. The registeration is needed because the interface implementation for builtin types locate atBufferizationDialect::initialize(). However, we do not need to fix it by the registeration. The proper fix is using the linalg method, i.e., hasPureTensorSemantics.