Fix GemmTransposeFusion identity transpose handling - #32429
Fix GemmTransposeFusion identity transpose handling#32429Sylvester Kaczmarek (sylvesterkaczmarek) wants to merge 3 commits into
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
Add output-side coverage for preserved identity and folded matrix transposes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Restricts Gemm transpose fusion to genuine rank-2 matrix transposes, preserving identity transposes.
Changes:
- Validates transpose permutations for Gemm inputs and outputs.
- Adds regression coverage for A and B input transposes.
File summaries
| File | Summary |
|---|---|
onnxruntime/test/optimizer/gemm_transpose_fusion_test.cc |
Tests identity and matrix transposes for A/B inputs; output-side cases remain uncovered. |
onnxruntime/core/optimizer/gemm_transpose_fusion.cc |
Guards input and output fusion based on transpose permutation. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| output_node_ptr->OpType() == "Transpose" && | ||
| IsMatrixTranspose(*output_node_ptr)) { |
|
Added the missing output-side regression coverage. The tests now verify that an identity output transpose is preserved and that a true matrix transpose is folded, including the expected transA/transB values and reversed Gemm input order in the folded case. Full ONNX Runtime CI is left to the PR because this sparse local checkout does not contain a complete build tree. |
Description
Fixes #32417 and #32418.
GemmTransposeFusioncurrently treats anyTransposedirectly connected to aGemminput or output as a matrix transpose and togglestransA/transB. That is only valid when the transpose actually swaps the two matrix axes.For an identity transpose with
perm=[0, 1], the current rewrite removes the transpose and flips the corresponding Gemm transpose attribute, changing the graph semantics. Non-square cases can fail after optimization, while square cases can silently produce different numeric results.This change only folds rank-2 matrix transposes with
perm=[1, 0], or a missingpermwhere ONNX's default reverse-axis behavior is equivalent to[1, 0]for Gemm matrices. The same guard is applied independently to A, B, and output transposes.Tests
Adds regression coverage for identity and real matrix transposes on both Gemm A and B inputs. Identity transposes remain intact;
[1, 0]transposes continue to fuse.