Transforms: reorder a broadcast constant by permuting, not reinterpreting - #22165
Draft
rascani wants to merge 1 commit into
Draft
Transforms: reorder a broadcast constant by permuting, not reinterpreting#22165rascani wants to merge 1 commit into
rascani wants to merge 1 commit into
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22165
Note: Links to docs will display an error until the docs builds have been completed. ✅ You can merge normally! (1 Unrelated Failure)As of commit 517882b with merge base 469debd ( FLAKY - The following job failed but was likely due to flakiness present on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
When RemovePermutesAroundElementwiseOps cancels the permutes around a region,
an operand that is a constant has no permute of its own to cancel, so the pass
compensates the constant directly: it applies the region's permutation to it.
For a constant of lower rank than the activation the pass emitted a view_copy.
Both ops copy, but they copy differently -- view_copy preserves flat order and
permute_copy reorders -- so a reshape stands in for the permutation only when
no non-unit axis moves:
b = torch.arange(4 * 8 * 8).reshape(4, 8, 8)
torch.ops.aten.view_copy.default(b, [8, 8, 4])[0, 0, :4] # [0, 1, 2, 3]
torch.ops.aten.permute_copy.default(b, [1, 2, 0])[0, 0, :4] # [0, 64, 128, 192]
Same shape, different values.
Broadcasting widens the constant to the region's rank before the permutation
applies, so widen it and ask where each non-unit axis lands. If those
destinations stay in order the permutation only shuffles unit axes, and a
single view_copy still expresses it -- the ordinary per-channel constant takes
this path and is unchanged. Otherwise the constant is widened and permuted.
Comparing extents rather than axis order would be wrong: two axes of the same
size can swap without the extents changing.
The old arithmetic also sliced the permuted shape back down to the constant's
rank, which is not where the surviving axes necessarily are. A rank-1
per-channel constant under [0, 2, 3, 1] wants (1, 1, C, 1) and the slice asked
for (1,), so export failed outright; it now lowers.
Checked by enumeration over every region rank 2..5, every permutation, every
constant rank below it and every extent combination in {1,2,3}: 15414
configurations, no disagreement with reshaping to region rank and permuting,
6810 of them still taking the single-view path.
Cadence is the exposed backend: it runs this pass twice in its default pipeline
and has no rank-matching pass. Arm reaches the pass at equal rank almost always,
because MatchArgRanksPass widens broadcast operands beforehand.
Authored with Claude Code.
rascani
force-pushed
the
transforms-constant-permute-fix
branch
from
August 25, 2026 22:41
d2f0e8b to
517882b
Compare
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
1 participant
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.
Summary
When RemovePermutesAroundElementwiseOps cancels a permute across a region, a broadcast constant of lower rank than the activation has to be reordered by the same permutation. The pass always emitted a view_copy for that. A view reinterprets strides rather than moving elements, so it is only a stand-in for the reorder when nothing but unit extents move. On a (4, 8, 8) bias under [0, 2, 3, 1] the two disagree elementwise at the same shape:
Keep the view where it is exact. Broadcasting widens the constant to the region's rank before the permutation applies, so compute the widened target shape and compare its non-unit extents with the original: if their order is unchanged the reorder is a pure reshape, and a single view still expresses it at no cost. Only when a non-unit extent actually moves does the constant get a widening view plus a permute.
A per-channel constant, the ordinary case, stays a lone view_copy. The rank-3 constant above is the one that gains a permute, because there a view is wrong.
The old shape arithmetic also sliced the permuted shape back down to the constant's rank, which is not where the surviving axes necessarily are. A rank-1 per-channel constant under [0, 2, 3, 1] wants (1, 1, C, 1) and the slice asked for (1,), so export failed outright; it now lowers.
Cadence is the exposed backend: it runs this pass twice in its default pipeline and has no rank-matching pass. Arm reaches the pass at equal rank almost always, because MatchArgRanksPass widens broadcast operands beforehand.
Authored with Claude Code.