Skip to content

Fix IndexError in RemovePermutesAroundElementwiseOps traversal - #21625

Open
cceroici wants to merge 1 commit into
pytorch:mainfrom
cceroici:export-D115062769
Open

Fix IndexError in RemovePermutesAroundElementwiseOps traversal#21625
cceroici wants to merge 1 commit into
pytorch:mainfrom
cceroici:export-D115062769

Conversation

@cceroici

@cceroici cceroici commented Aug 6, 2026

Copy link
Copy Markdown

Summary:
_adapt_permute_across_view indexed the permutation without checking it was expressed in the view's input-rank space, so visit() crashed with IndexError: list index out of range instead of rejecting the subgraph.

  • Traversal adapts the permutation when it crosses a rank-changing view. If it then reaches a second squeeze/unsqueeze view whose input is still at the original rank, permute[index] is out of range — e.g. squeezing a trailing size-1 dim gives index == 3 for a rank-3 permutation.
  • Fix: validate the index / rank in all three branches of _adapt_permute_across_view and return None, the function's documented "not possible" signal. All three call sites already handle None by skipping the subgraph.
  • This mirrors the guard D105787161 added in permute_subgraph(), which only runs at apply time — traversal had no equivalent check.

Surfaced by D113424191: its _is_permutation_sink_view branch lets traversal continue past a layout-invariant flatten rather than aborting, which grows regions enough to reach a rank-mismatched view. That optimization is intentional and is preserved here; only the crash is fixed. The latent bug predates it.

Broke test_auth_u85 (ECAPA -> Ethos-U85) — ARM's RemovePermutesAroundElementwiseTosaOps subclasses this shared pass, so it runs inside ArmPassManager on every Ethos-U lowering. That test is CONTINUOUS_STATE, so it is omitted on diffs and could not have blocked the landing.

Authored with AI assistance (Claude Code).

Differential Revision: D115062769

Summary:
`_adapt_permute_across_view` indexed the permutation without checking it was expressed in the view's input-rank space, so `visit()` crashed with `IndexError: list index out of range` instead of rejecting the subgraph.

- Traversal adapts the permutation when it crosses a rank-changing view. If it then reaches a *second* squeeze/unsqueeze view whose input is still at the original rank, `permute[index]` is out of range — e.g. squeezing a trailing size-1 dim gives `index == 3` for a rank-3 permutation.
- Fix: validate the index / rank in all three branches of `_adapt_permute_across_view` and return `None`, the function's documented "not possible" signal. All three call sites already handle `None` by skipping the subgraph.
- This mirrors the guard D105787161 added in `permute_subgraph()`, which only runs at apply time — traversal had no equivalent check.

Surfaced by D113424191: its `_is_permutation_sink_view` branch lets traversal continue past a layout-invariant flatten rather than aborting, which grows regions enough to reach a rank-mismatched view. That optimization is intentional and is preserved here; only the crash is fixed. The latent bug predates it.

Broke `test_auth_u85` (ECAPA -> Ethos-U85) — ARM's `RemovePermutesAroundElementwiseTosaOps` subclasses this shared pass, so it runs inside `ArmPassManager` on every Ethos-U lowering. That test is `CONTINUOUS_STATE`, so it is omitted on diffs and could not have blocked the landing.

Authored with AI assistance (Claude Code).

Differential Revision: D115062769
@linux-foundation-easycla

Copy link
Copy Markdown

CLA Not Signed

@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 Aug 6, 2026
@meta-codesync

meta-codesync Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

@cceroici has exported this pull request. If you are a Meta employee, you can view the originating Diff in D115062769.

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. meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant