Fix ReshapeFusion dropping allowzero on inferred 0-sized intermediate dims#28349
Merged
justinchuby merged 1 commit intomainfrom May 5, 2026
Merged
Fix ReshapeFusion dropping allowzero on inferred 0-sized intermediate dims#28349justinchuby merged 1 commit intomainfrom
justinchuby merged 1 commit intomainfrom
Conversation
… dims FuseContiguousReshapes collapses a chain of Reshape/Squeeze/Unsqueeze nodes into a single Reshape whose shape data is taken verbatim from the fully-inferred output shape of the last node in the chain. The new node is created without an allowzero attribute, so it defaults to allowzero=0. When that inferred shape contains a literal 0 dim (legitimate when the original chain used allowzero=1, or when intermediate tensors had zero-sized dimensions), the fused Reshape misinterprets the 0 as 'copy the corresponding dim from the input tensor' -- but the input here is the original input of the *first* reshape in the chain, with unrelated dims. The result is a silently wrong output shape (and a benign-looking MergeShapeInfo warning at graph load). Setting allowzero=1 on the fused node would fix this but requires opset >= 14, which the transformer cannot assume (it accepts Reshape opset 5+). Bail out of fusion conservatively when the inferred shape contains any literal 0 dim. Adds a regression test that builds a Reshape -> Reshape chain whose inferred intermediate has zero-sized dims and asserts that the fusion no longer collapses it (and that the inferred output shape stays correct). Fixes #28348. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
justinchuby
approved these changes
May 4, 2026
justinchuby
reviewed
May 4, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a correctness bug in the Reshape fusion optimizer: when collapsing contiguous Reshape/Squeeze/Unsqueeze chains, the fused node could mis-handle literal 0 dimensions because it did not preserve allowzero semantics. In the optimizer stack, this prevents a silent shape corruption during graph rewriting.
Changes:
- Add a guard in
ReshapeFusion::FuseContiguousReshapesto skip fusion when the inferred fused shape contains any literal0dimension. - Add a regression test covering a
Reshape -> Reshapechain where the second reshape usesallowzero=1and the correct output shape contains zeros.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
onnxruntime/core/optimizer/reshape_fusion.cc |
Adds a conservative early-exit to avoid unsafe contiguous reshape fusion for inferred zero dims. |
onnxruntime/test/optimizer/graph_transform_test.cc |
Adds a regression test asserting the unsafe reshape chain is not fused and inferred output shape stays correct. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
Will merge to fix the regression reported in #28348. Follow ups can be created if desired. |
Contributor
|
@titaiwangms thanks for creating the fix! |
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
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.
Description
ReshapeFusion::FuseContiguousReshapescollapses a chain ofReshape/Squeeze/Unsqueezenodes into a singleReshapewhose shape data is taken verbatim from the fully-inferred output shape of the last node in the chain. The new node is created without anallowzeroattribute, so it defaults toallowzero = 0.When that inferred shape contains a literal
0dim (legitimate when the original chain usedallowzero=1, or when intermediate tensors had zero-sized dimensions), the fusedReshapemisinterprets the0as "copy the corresponding dim from the input tensor" — but the input here is the original input of the first reshape in the chain, with unrelated dims. The result is a silently wrong output shape (and a benign-lookingMergeShapeInfowarning at graph load).Repro (before the fix)
Output on
main(40c9f85f69):Fix
Setting
allowzero=1on the fused node would also work but requires opset >= 14, which this transformer cannot assume (it acceptsReshapeopset 5+). Bail out of fusion conservatively whenshape_valuecontains any literal0dim.Test
Adds
ReshapeFusionContiguousReshapesWithZeroDimthat builds the bug repro programmatically and asserts:(0, 0, 3)The existing happy-path test
ReshapeFusion_Contiguous_Reshape(added in #22494) is unaffected — its inferred output shape(2, 1, 64, 32)contains no zero dims, so the new guard does not trigger.Provenance
FuseContiguousReshapeswas introduced in #22494 (Feb 2025). The bug has been latent inmainsince then.Motivation and Context
Found while reviewing microsoft/onnxscript#2907 — the rewriter rule under test there is semantically correct, but its numerical-equivalence check using ORT as the oracle fails because of this fusion bug.
Fixes #28348.