Skip to content

Add pass to replace input dim order clones with permutations.#21241

Open
MartinPavella wants to merge 2 commits into
pytorch:mainfrom
nxp-upstream:nxg01483/EIEX-1008-handle-non-contiguous-inputs
Open

Add pass to replace input dim order clones with permutations.#21241
MartinPavella wants to merge 2 commits into
pytorch:mainfrom
nxp-upstream:nxg01483/EIEX-1008-handle-non-contiguous-inputs

Conversation

@MartinPavella

Copy link
Copy Markdown
Collaborator

Summary

This pass replaces dim_order_clone and to_dim_order_copy nodes that consume model inputs by a sequence of permute nodes according to the proposal in #19299.

Fixes #20095.

Note: The PR is build on top of #21057 for testing purposes. This will be cleaned up before merge, so only the last commit is relevant for review.

Test plan

pytest backends/transforms/test/test_replace_channels_last_input_clones.py

The pass preserves the dim order of the inputs, but internally the graph will use only the contiguous dim order.
@MartinPavella MartinPavella self-assigned this Jul 23, 2026
@MartinPavella MartinPavella added the python Pull requests that update python code label Jul 23, 2026
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 23, 2026
@pytorch-bot

pytorch-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21241

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit e0c600c with merge base ad52be9 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@linux-foundation-easycla

Copy link
Copy Markdown

CLA Not Signed

@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@mcremon-meta

Copy link
Copy Markdown
Contributor

@MartinPavella if we're going to go all the way to removing dim_order ops, why not just skip them instead? You can set the _skip_dim_order flag to true, and those ops will never appear. The whole graph will be contiguous by construction. That is what we do for Cadence, e.g. here:

_skip_dim_order=True,
.

Related to this, that's one of the problems that I believe our fused-quant proposal would address. Hopefully we can discuss next week!

@MartinPavella

Copy link
Copy Markdown
Collaborator Author

@MartinPavella if we're going to go all the way to removing dim_order ops, why not just skip them instead? You can set the _skip_dim_order flag to true, and those ops will never appear. The whole graph will be contiguous by construction. That is what we do for Cadence, e.g. here:

_skip_dim_order=True,

.

Related to this, that's one of the problems that I believe our fused-quant proposal would address. Hopefully we can discuss next week!

@mcremon-meta thanks for the suggestion. In this PR our goal is to have a model that consumes channels last input data, which is then permuted to channels first by the channels_last.permute_copy operator. This permutation will then be canceled out with other channels_last.permute_copy operators that surround the channels last versions of compute operators, resulting in a model with channels last inputs and operators, and no runtime transpositions.

However, your suggestion might be applicable to another one of my PRs (#21057). In that one, our goal is to remove non-contiguous dim order from the model except for the inputs (so not quite what _skip_dim_order=True does, but close). Looking at #20096, I think we could do without #21057 and use the _skip_dim_order=True technique + #20096 instead. What do you think @AdrianLundell?

@AdrianLundell

Copy link
Copy Markdown
Collaborator

My only concern is that requiring _skip_dim_order=True may break scripts instantiating its own edge compile config and for the arm backend _skip_dim_order=False has been the expected choice previously. So I agree that setting it to false is a better solution, but if it is not too much work supporting False at least in a deprecation period would be preferred in my opinion.

As a heads up I go on leave from Monday for three weeks so I won't be available for more reviews during that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. python Pull requests that update python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Memory-layout infra] Handle non-contiguous inputs

3 participants