Skip to content

Commit 1dbb8e7

Browse files
authored
Don't drop a permute that an earlier region already bypassed (#24067)
"Arm backend: Map linear rows to conv2d width" (9a81281) makes DecomposeLinearPass emit permutes around the 1x1 conv. In a gated MLP (`u * v`, where `v` also goes through a transpose), this lets RemovePermutesAroundElementwiseOps plan two regions that share one permute: - Region A treats the v-branch transpose as an incoming permute and bypasses it, so `mul` reads the transpose's producer directly. - Region B was planned before A was applied. It treats the same transpose as its outgoing permute, removes its own start permute and changes that producer's layout. `_subgraph_edges_are_current` only checked that the outgoing permute was still wired to its producer. A bypassed permute stays wired even though nothing uses it, so B was applied too. `mul` was left with mismatched operands (`[1,144,14] * [1,14,144]`), and PropagateViewCopyPermuteDownPass failed. The fix records the permutes each applied region bypasses. A later region whose outgoing (or rewritten) permute is in that set is skipped. Test plan: - New unit test `test_permute_bypassed_by_one_region_is_kept_by_another` passes. With the check disabled it fails with the same broadcast RuntimeError. - A gated-MLP model lowered through TosaPipelineINT failed with the same error before the fix and passes after. Internal mirror: D126717094.
1 parent 3e77093 commit 1dbb8e7

2 files changed

Lines changed: 47 additions & 1 deletion

File tree

‎backends/transforms/remove_permutes_around_elementwise_ops.py‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ def __init__(
120120
tuple[int, int, torch.fx.Node, torch.fx.Node] | None,
121121
] = {}
122122
self._pending_visits = None
123+
self._bypassed_permutes: set[torch.fx.Node] = set()
123124

124125
_VIEW_OPS = (
125126
exir_ops.edge.aten.view_copy.default,
@@ -417,6 +418,7 @@ def _adapt_permute_across_view(
417418
def call(self, graph_module: torch.fx.GraphModule) -> PassResult: # noqa: C901
418419
self._sq_unsq_cache.clear()
419420
self._interleave_cache.clear()
421+
self._bypassed_permutes.clear()
420422
subgraphs_found: list[RemovePermutesAroundElementwiseOps.Subgraph] = []
421423
processed_nodes: set[torch.fx.Node] = set()
422424
for node in graph_module.graph.nodes:
@@ -829,6 +831,7 @@ def permute_subgraph(self, subgraph: Subgraph) -> bool: # noqa: C901
829831
# Skip incoming permutes.
830832
for inp, out in subgraph.edges_in:
831833
assert inp.target in PERMUTE_COPY_TARGETS
834+
self._bypassed_permutes.add(inp)
832835
if len(inp.args) >= 1:
833836
out.replace_input_with(inp, cast(torch.fx.Node, inp.args[0]))
834837
else:
@@ -897,7 +900,13 @@ def _subgraph_edges_are_current(self, subgraph: Subgraph) -> bool:
897900
(inp, out) for inp, out, _ in subgraph.edges_out_to_update
898901
]
899902
for inp, out in outgoing:
900-
if out.target not in PERMUTE_COPY_TARGETS or out not in inp.users:
903+
# A bypassed permute stays wired to its producer, but an earlier
904+
# region now reads that producer directly and relies on its layout.
905+
if (
906+
out.target not in PERMUTE_COPY_TARGETS
907+
or out not in inp.users
908+
or out in self._bypassed_permutes
909+
):
901910
return False
902911

903912
for const_node, user_node in subgraph.constant_edges_in:

‎backends/transforms/test/test_permute_optimization_passes.py‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2500,6 +2500,43 @@ def test_chained_regions_absorb_into_last_permute(self) -> None:
25002500
"chained_regions_absorb_into_last_permute",
25012501
)
25022502

2503+
def test_permute_bypassed_by_one_region_is_kept_by_another(self) -> None:
2504+
"""Region A bypasses pb as an incoming permute, so mul reads tanh
2505+
directly. Region B would remove pb as its outgoing permute and change
2506+
tanh's layout, so it must be skipped."""
2507+
builder = GraphBuilder()
2508+
x_data = torch.randn(2, 3, 4)
2509+
y_data = torch.randn(2, 4, 3)
2510+
x = builder.placeholder("x", x_data)
2511+
y = builder.placeholder("y", y_data)
2512+
pa = builder.call_operator(
2513+
op=exir_ops.edge.aten.permute_copy.default, args=(x, [0, 2, 1])
2514+
)
2515+
pc = builder.call_operator(
2516+
op=exir_ops.edge.aten.permute_copy.default, args=(y, [0, 2, 1])
2517+
)
2518+
tanh = builder.call_operator(op=exir_ops.edge.aten.tanh.default, args=(pc,))
2519+
pb = builder.call_operator(
2520+
op=exir_ops.edge.aten.permute_copy.default, args=(tanh, [0, 2, 1])
2521+
)
2522+
mul = builder.call_operator(op=exir_ops.edge.aten.mul.Tensor, args=(pa, pb))
2523+
out = builder.call_operator(
2524+
op=exir_ops.edge.aten.permute_copy.default, args=(mul, [0, 2, 1])
2525+
)
2526+
builder.output([out])
2527+
original = builder.get_graph_module()
2528+
gm_before = copy.deepcopy(original)
2529+
2530+
result = cast(PassResult, RemovePermutesAroundElementwiseOps()(original))
2531+
2532+
self.assertTrue(result.modified)
2533+
validate_numerics(
2534+
gm_before,
2535+
result.graph_module,
2536+
[x_data, y_data],
2537+
"permute_bypassed_by_one_region_is_kept_by_another",
2538+
)
2539+
25032540

25042541
class LayoutPermuteVisibilityTest(unittest.TestCase):
25052542
"""The data-movement passes must see both permute dialects.

0 commit comments

Comments
 (0)