[mlir][vector] Update ApplyVectorReductionToContractPatternsOp #136699
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR removes the use of `populateSinkVectorOpsPatterns` from the definition of: * `transform.apply_patterns.vector.reduction_to_contract` As of llvm#131462, there is now a dedicated transform op for this pattern: * `transform.apply_patterns.vector.sink_op` Given that, we should now use the new TD op directly instead of relying on unrelated TD ops to include it. NOTE TO DOWNSTREAM USERS: Please add the following to your TD scripts: * `transform.apply_patterns.vector.sink_op` See the updated test for an example.
banach-space
requested review from
ftynse,
nicolasvasilache and
dcaballe
as code owners
|
@llvm/pr-subscribers-mlir-sme @llvm/pr-subscribers-mlir-vector Author: Andrzej Warzyński (banach-space) ChangesThis PR removes the use of
As of #131462, there is now a dedicated transform op for this pattern:
Given that, we should now use the new TD op directly instead of relying NOTE TO DOWNSTREAM USERS:
See the updated test for an example. Full diff: https://github.com/llvm/llvm-project/pull/136699.diff 3 Files Affected:
diff --git a/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp b/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp
index 12dcf768dd928..adc1dd437326f 100644
--- a/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp
+++ b/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp
@@ -67,10 +67,6 @@ void transform::ApplyFoldElementwiseToVectorPatternsOp::populatePatterns(
void transform::ApplyVectorReductionToContractPatternsOp::populatePatterns(
RewritePatternSet &patterns) {
vector::populateVectorReductionToContractPatterns(patterns);
-
- // TODO: As we now have a dedicated transform for
- // `populateSinkVectorOpsPatterns` we can remove it from here.
- vector::populateSinkVectorOpsPatterns(patterns);
}
void transform::ApplyLowerCreateMaskPatternsOp::populatePatterns(
diff --git a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir
index 29b0bc0c19606..ad7dbb9f7e126 100644
--- a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir
+++ b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir
@@ -79,6 +79,7 @@ module attributes {transform.with_named_sequence} {
transform.apply_patterns.vector.lower_masked_transfers
transform.apply_patterns.vector.transfer_permutation_patterns
transform.apply_patterns.vector.reduction_to_contract
+ transform.apply_patterns.vector.sink_ops
} : !transform.any_op
// Step 5: Lower vector.contract to vector.outerproduct. Also drop unit
diff --git a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir
index 78815a38612e9..71798a6affbbc 100644
--- a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir
+++ b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir
@@ -94,6 +94,7 @@ module attributes {transform.with_named_sequence} {
transform.apply_patterns.vector.lower_masked_transfers
transform.apply_patterns.vector.transfer_permutation_patterns
transform.apply_patterns.vector.reduction_to_contract
+ transform.apply_patterns.vector.sink_ops
} : !transform.any_op
// Step 5: Lower vector.contract to vector.outerproduct. Also drop unit
|
|
@llvm/pr-subscribers-mlir-linalg Author: Andrzej Warzyński (banach-space) ChangesThis PR removes the use of
As of #131462, there is now a dedicated transform op for this pattern:
Given that, we should now use the new TD op directly instead of relying NOTE TO DOWNSTREAM USERS:
See the updated test for an example. Full diff: https://github.com/llvm/llvm-project/pull/136699.diff 3 Files Affected:
diff --git a/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp b/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp
index 12dcf768dd928..adc1dd437326f 100644
--- a/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp
+++ b/mlir/lib/Dialect/Vector/TransformOps/VectorTransformOps.cpp
@@ -67,10 +67,6 @@ void transform::ApplyFoldElementwiseToVectorPatternsOp::populatePatterns(
void transform::ApplyVectorReductionToContractPatternsOp::populatePatterns(
RewritePatternSet &patterns) {
vector::populateVectorReductionToContractPatterns(patterns);
-
- // TODO: As we now have a dedicated transform for
- // `populateSinkVectorOpsPatterns` we can remove it from here.
- vector::populateSinkVectorOpsPatterns(patterns);
}
void transform::ApplyLowerCreateMaskPatternsOp::populatePatterns(
diff --git a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir
index 29b0bc0c19606..ad7dbb9f7e126 100644
--- a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir
+++ b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/matmul.mlir
@@ -79,6 +79,7 @@ module attributes {transform.with_named_sequence} {
transform.apply_patterns.vector.lower_masked_transfers
transform.apply_patterns.vector.transfer_permutation_patterns
transform.apply_patterns.vector.reduction_to_contract
+ transform.apply_patterns.vector.sink_ops
} : !transform.any_op
// Step 5: Lower vector.contract to vector.outerproduct. Also drop unit
diff --git a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir
index 78815a38612e9..71798a6affbbc 100644
--- a/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir
+++ b/mlir/test/Integration/Dialect/Linalg/CPU/ArmSME/multi-tile-matmul-mixed-types.mlir
@@ -94,6 +94,7 @@ module attributes {transform.with_named_sequence} {
transform.apply_patterns.vector.lower_masked_transfers
transform.apply_patterns.vector.transfer_permutation_patterns
transform.apply_patterns.vector.reduction_to_contract
+ transform.apply_patterns.vector.sink_ops
} : !transform.any_op
// Step 5: Lower vector.contract to vector.outerproduct. Also drop unit
|
Hardcode84
approved these changes
Apr 22, 2025
banach-space
deleted the
andrzej/vector/update_vec_reduction_to_contract
branch
banach-space
added a commit
to banach-space/llvm-project
that referenced
this pull request
Apr 23, 2025
Updates a test that I forgot to update in llvm#136699. Failing bot: * https://lab.llvm.org/buildbot/#/builders/143/builds/7166
banach-space
added a commit
that referenced
this pull request
Apr 23, 2025
Updates a test that I forgot to update in #136699. Failing bot: * https://lab.llvm.org/buildbot/#/builders/143/builds/7166
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…136699) This PR removes the use of `populateSinkVectorOpsPatterns` from the definition of: * `transform.apply_patterns.vector.reduction_to_contract` As of llvm#131462, there is now a dedicated transform op for this pattern: * `transform.apply_patterns.vector.sink_op` Given that, we should now use the new TD op directly instead of relying on unrelated TD ops to include it. NOTE TO DOWNSTREAM USERS: Please add the following to your TD scripts: * `transform.apply_patterns.vector.sink_op` See the updated test for an example.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
Updates a test that I forgot to update in llvm#136699. Failing bot: * https://lab.llvm.org/buildbot/#/builders/143/builds/7166
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…136699) This PR removes the use of `populateSinkVectorOpsPatterns` from the definition of: * `transform.apply_patterns.vector.reduction_to_contract` As of llvm#131462, there is now a dedicated transform op for this pattern: * `transform.apply_patterns.vector.sink_op` Given that, we should now use the new TD op directly instead of relying on unrelated TD ops to include it. NOTE TO DOWNSTREAM USERS: Please add the following to your TD scripts: * `transform.apply_patterns.vector.sink_op` See the updated test for an example.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
Updates a test that I forgot to update in llvm#136699. Failing bot: * https://lab.llvm.org/buildbot/#/builders/143/builds/7166
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
…136699) This PR removes the use of `populateSinkVectorOpsPatterns` from the definition of: * `transform.apply_patterns.vector.reduction_to_contract` As of llvm#131462, there is now a dedicated transform op for this pattern: * `transform.apply_patterns.vector.sink_op` Given that, we should now use the new TD op directly instead of relying on unrelated TD ops to include it. NOTE TO DOWNSTREAM USERS: Please add the following to your TD scripts: * `transform.apply_patterns.vector.sink_op` See the updated test for an example.
IanWood1
pushed a commit
to IanWood1/llvm-project
that referenced
this pull request
May 6, 2025
Updates a test that I forgot to update in llvm#136699. Failing bot: * https://lab.llvm.org/buildbot/#/builders/143/builds/7166
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the use of
populateSinkVectorOpsPatternsfrom thedefinition of:
transform.apply_patterns.vector.reduction_to_contractAs of #131462, there is now a dedicated transform op for this pattern:
transform.apply_patterns.vector.sink_opGiven that, we should now use the new TD op directly instead of relying
on unrelated TD ops to include it.
NOTE TO DOWNSTREAM USERS:
Please add the following to your TD scripts:
transform.apply_patterns.vector.sink_opSee the updated test for an example.