Add pass to replace input dim order clones with permutations.

[MartinPavella]

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

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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:

• Unrelated MacOS MPS test jobs time out (30 min) due to stale cpp_extension lock file poisoning self-hosted runners

✅ No Failures

As of commit e0c600c with merge base ad52be9 ():
💚 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]

• ❌ - login: @MartinPavella / name: Martin Pavella. The commit (7a3280e, e0c600c) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please visit our EasyCLA portal and chat with our support bot.

[github-actions]

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]

@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:

executorch/backends/cadence/aot/compiler.py

Line 298 in 3389d51

_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]

@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:

executorch/backends/cadence/aot/compiler.py

Line 298 in 3389d51

_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]

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.

[MartinPavella]

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.

Support for _skip_dim_order=False (i.e. a solution which doesn't require _skip_dim_order=True) is already implemented in #21057. Relying on _skip_dim_order=True would render that PR meaningless and we would avoid introducing 1 pass. But if we want to be more flexible, we can just merge #21057 and support both states of _skip_dim_order.