Fix IndexError when a squeeze view is reached upstream (#21620) - #21620
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21620
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit c709c73 with merge base 74f1ce8 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@mcremon-meta has exported this pull request. If you are a Meta employee, you can view the originating Diff in D115002472. |
This PR needs a
|
Summary: `RemovePermutesAroundElementwiseOps._adapt_permute_across_view` assumes the permutation it is handed lives in the view's *input-rank* space. That holds for the two downstream call sites, but `visit()` also calls it when a squeeze/unsqueeze view is reached by *upstream* traversal, where the permutation is expressed at the view's *output* rank. For a squeeze view the squeezed position is then out of range for the permutation, and `permute[index]` raises `IndexError: list index out of range`, aborting the whole `ArmPassManager` pipeline during lowering. This is the same class of bug as D105787161, which added a guard in `permute_subgraph()` — but that guard runs after traversal, so it never gets a chance to fire on this path. Bail out of `_adapt_permute_across_view` (return `None`) when the permutation rank does not match the view's input rank. The existing `visit()` call sites already treat `None` as "cannot adapt" and drop the subgraph, so the pass degrades to leaving the permutes in place instead of crashing or silently mis-adapting the permutation. Reviewed By: aliafzal Differential Revision: D115002472
838fb82 to
de2efc1
Compare
Summary: `RemovePermutesAroundElementwiseOps._adapt_permute_across_view` assumes the permutation it is handed lives in the view's *input-rank* space. That holds for the two downstream call sites, but `visit()` also calls it when a squeeze/unsqueeze view is reached by *upstream* traversal, where the permutation is expressed at the view's *output* rank. For a squeeze view the squeezed position is then out of range for the permutation, and `permute[index]` raises `IndexError: list index out of range`, aborting the whole `ArmPassManager` pipeline during lowering. This is the same class of bug as D105787161, which added a guard in `permute_subgraph()` — but that guard runs after traversal, so it never gets a chance to fire on this path. Bail out of `_adapt_permute_across_view` (return `None`) when the permutation rank does not match the view's input rank. The existing `visit()` call sites already treat `None` as "cannot adapt" and drop the subgraph, so the pass degrades to leaving the permutes in place instead of crashing or silently mis-adapting the permutation. Reviewed By: aliafzal Differential Revision: D115002472
de2efc1 to
c709c73
Compare
Summary:
RemovePermutesAroundElementwiseOps._adapt_permute_across_viewassumes thepermutation it is handed lives in the view's input-rank space. That holds for
the two downstream call sites, but
visit()also calls it when asqueeze/unsqueeze view is reached by upstream traversal, where the permutation
is expressed at the view's output rank. For a squeeze view the squeezed
position is then out of range for the permutation, and
permute[index]raisesIndexError: list index out of range, aborting the wholeArmPassManagerpipeline during lowering.
This is the same class of bug as D105787161, which added a guard in
permute_subgraph()— but that guard runs after traversal, so it never gets achance to fire on this path.
Bail out of
_adapt_permute_across_view(returnNone) when the permutationrank does not match the view's input rank. The existing
visit()call sitesalready treat
Noneas "cannot adapt" and drop the subgraph, so the passdegrades to leaving the permutes in place instead of crashing or silently
mis-adapting the permutation.
Reviewed By: aliafzal
Differential Revision: D115002472