Skip to content

Commit dd8297a

Browse files
authored
Vulkan: remove redundant view_copy ops (pytorch#22898)
A `view_copy` onto the shape the tensor already has reaches the Vulkan runtime as a dispatch that reads every texel and writes it back unchanged. Adds `view_copy` to `RemoveRedundantOpsTransform` (its guard already requires the shapes to match), and runs the pass again after `SqueezeUnsqueezeInputs` / `FuseViewCopyTransform` / `ViewCopyToSqueezeUnsqueezePass`, which can collapse a chain into such a view after the only run that could have removed it. Removes 29 dispatches on rf-detr segmentation and 42 on rf-detr keypoint. Mali-G76, 6 interleaved rounds, median GPU time 789.3 -> 775.4 ms on segmentation, outputs bit-identical. Shape-changing views are kept: ops downstream read their argument's shape, and dropping them made a rank-4 consumer fail its broadcast check. Tests cover both cases. cc @SS-JIA @manuelcandales @digantdesai @cbilgin
1 parent 5be88d0 commit dd8297a

6 files changed

Lines changed: 274 additions & 9 deletions

File tree

‎backends/vulkan/_passes/remove_redundant_ops.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ class RemoveRedundantOpsTransform(ExportPass):
3636
exir_ops.edge.aten.expand_copy.default,
3737
# copy.default(self, src): no-op when src dtype/shape matches self.
3838
exir_ops.edge.aten.copy.default,
39+
# view_copy.default: a view onto the same shape. These survive into the
40+
# partition and dispatch a shader that reads every texel and writes it
41+
# back unchanged. Only same-shape views are safe to drop here: a view
42+
# that changes the rank must keep its node, because the ops downstream
43+
# read their argument's shape.
44+
exir_ops.edge.aten.view_copy.default,
3945
}
4046

4147
# For these ops the meaningful input is args[1] (src), not args[0] (self).

‎backends/vulkan/serialization/vulkan_graph_builder.py‎

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import logging
1010
import operator
1111
from types import NoneType
12-
from typing import cast, Dict, List, Optional, Union
12+
from typing import cast, Dict, List, Optional, Tuple, Union
1313

1414
import executorch.backends.vulkan.serialization.vulkan_graph_schema as vk_graph_schema
1515
import torch
@@ -32,6 +32,7 @@
3232
from executorch.exir.tensor import TensorSpec
3333
from torch._export.utils import get_buffer, get_param, is_buffer, is_param
3434
from torch.export import ExportedProgram
35+
from torch.export.graph_signature import OutputKind
3536
from torch.fx import Node
3637

3738
_ScalarType = Union[bool, int, float]
@@ -490,19 +491,80 @@ def process_call_function_node(self, node) -> None:
490491
def process_getattr_node(self, node: Node) -> None:
491492
self.create_node_value(node)
492493

494+
def _output_nodes_in_order(self, node: Node) -> List[Tuple[Optional[int], Node]]:
495+
"""
496+
The graph outputs in order, duplicates included, each paired with its
497+
position in the output signature.
498+
499+
`node.all_input_nodes` de-duplicates, so a graph that returns the same
500+
value twice (`return y, y`, or `return y, y.view(...)` once a redundant
501+
view is removed) would serialize one output id for two outputs and fail
502+
the runtime's argument count check.
503+
504+
The position is what distinguishes those repeats from a buffer mutation
505+
that happens to alias a user output: both reference the same node, so
506+
only the signature can say which slot is which. It is None when the
507+
signature does not line up with the output args, in which case the
508+
caller falls back to matching by name.
509+
"""
510+
args = node.args[0] if len(node.args) == 1 else node.args
511+
if isinstance(args, Node):
512+
args = (args,)
513+
output_specs = getattr(self.program.graph_signature, "output_specs", None)
514+
aligned = output_specs is not None and len(output_specs) == len(args)
515+
out_nodes: List[Tuple[Optional[int], Node]] = []
516+
for idx, arg in enumerate(args):
517+
if isinstance(arg, Node):
518+
out_nodes.append((idx if aligned else None, arg))
519+
return out_nodes
520+
521+
def _is_user_output_slot(self, idx: Optional[int]) -> Optional[bool]:
522+
"""
523+
Whether the output slot at `idx` is a user output, or None if unknown.
524+
"""
525+
if idx is None:
526+
return None
527+
output_specs = getattr(self.program.graph_signature, "output_specs", None)
528+
if output_specs is None:
529+
return None
530+
return output_specs[idx].kind == OutputKind.USER_OUTPUT
531+
493532
def process_output_node(self, node: Node) -> None:
494-
for out_node in node.all_input_nodes:
533+
# Names already emitted for a buffer mutation that aliases a user
534+
# output; see the fallback branch below.
535+
emitted_mutation_aliases: set[str] = set()
536+
for idx, out_node in self._output_nodes_in_order(node):
495537
if out_node not in self.node_to_value_ids:
496538
raise AssertionError(
497539
"Cannot find input to output node in node_to_value_ids. This means "
498540
"the output node is being serialized before its corresponding "
499541
"internal node which is not allowed."
500542
)
501-
# Mutable buffers outputs are not included as an output to the
502-
# delegate call. Skip marking them as an output.
543+
# A mutation whose buffer is aliased to its input is applied in
544+
# place, so its slot is not an output of the delegate call. Decide
545+
# per slot rather than per node: the mutation and a user output can
546+
# be the same node, and matching by name alone would either keep
547+
# both or drop both.
548+
#
549+
# Only the aliased case is filtered here. Without aliasing the
550+
# mutation still needs a slot to carry the new buffer value back,
551+
# so the old rule stands: dropping the slot would leave the graph
552+
# accepted but never updating the buffer, which is worse than the
553+
# argument-count rejection it would otherwise get.
503554
if out_node.name in self.buffer_mutation_inputs:
504-
if out_node.name not in self.buffer_mutation_user_outputs:
505-
continue
555+
is_user_output = self._is_user_output_slot(idx)
556+
if is_user_output is not None:
557+
if not is_user_output:
558+
continue
559+
else:
560+
if out_node.name not in self.buffer_mutation_user_outputs:
561+
continue
562+
# Without the signature to separate the slots, the mutation
563+
# and the user output are indistinguishable occurrences of
564+
# one node, and together they get a single slot.
565+
if out_node.name in emitted_mutation_aliases:
566+
continue
567+
emitted_mutation_aliases.add(out_node.name)
506568
elif is_mutable_buffer_node(out_node, self.program):
507569
continue
508570

‎backends/vulkan/test/test_serialization.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import executorch.backends.vulkan.custom_ops_lib # noqa: F401
1616
import torch
17+
from executorch.backends.vulkan.partitioner.vulkan_partitioner import VulkanPartitioner
1718
from executorch.backends.vulkan.serialization import (
1819
vulkan_graph_builder as graph_builder_module,
1920
)
@@ -34,6 +35,7 @@
3435
serialize_vulkan_graph,
3536
VulkanDelegateHeader,
3637
)
38+
from executorch.exir import to_edge_transform_and_lower
3739

3840

3941
class TestSerialization(unittest.TestCase):
@@ -168,6 +170,48 @@ def test_alias_buffer_mutations_preserves_shared_user_output(self) -> None:
168170
[builder.node_to_value_ids[mutation]],
169171
)
170172

173+
def test_mutation_aliasing_user_output_gets_one_slot(self) -> None:
174+
# A mutated buffer returned through a view: once the view is removed
175+
# the mutation and the user output are the same node. With aliasing on
176+
# the mutation is applied in place and only the user output takes a
177+
# slot; without it the mutation still needs a slot to carry the new
178+
# value back, and the graph is left to be rejected rather than silently
179+
# skipping the state update.
180+
class Model(torch.nn.Module):
181+
def __init__(self) -> None:
182+
super().__init__()
183+
self.register_buffer("state", torch.zeros(2, 48))
184+
185+
def forward(self, x):
186+
self.state.add_(x)
187+
return self.state.view(2, 48)
188+
189+
for alias, expected_slots in ((True, 1), (False, 2)):
190+
with self.subTest(alias_buffer_mutations=alias):
191+
captured: List[int] = []
192+
original = graph_builder_module.VkGraphBuilder.build_graph
193+
194+
def spy(builder_self, _original=original, _captured=captured):
195+
graph = _original(builder_self)
196+
_captured.append(len(graph.output_ids))
197+
return graph
198+
199+
graph_builder_module.VkGraphBuilder.build_graph = spy
200+
try:
201+
program = torch.export.export(
202+
Model(), (torch.ones(2, 48),), strict=True
203+
)
204+
to_edge_transform_and_lower(
205+
program,
206+
partitioner=[
207+
VulkanPartitioner({"alias_buffer_mutations": alias})
208+
],
209+
).to_executorch()
210+
finally:
211+
graph_builder_module.VkGraphBuilder.build_graph = original
212+
213+
self.assertEqual(captured, [expected_slots])
214+
171215
def _generate_random_const_tensors(self, num_tensors: int) -> List[torch.Tensor]:
172216
"""
173217
Helper function to generate `num_tensor` buffers of random sizes and random contents,

‎backends/vulkan/test/test_vulkan_delegate.py‎

Lines changed: 90 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,12 @@ def wrapper(*args, **kwargs):
6363

6464

6565
def lower_module(
66-
model: torch.nn.Module, sample_inputs: Tuple[torch.Tensor], dynamic_shapes=None
66+
model: torch.nn.Module,
67+
sample_inputs: Tuple[torch.Tensor],
68+
dynamic_shapes=None,
69+
compile_options=None,
6770
) -> EdgeProgramManager:
68-
compile_options = {}
71+
compile_options = dict(compile_options or {})
6972
if dynamic_shapes is not None:
7073
compile_options["require_dynamic_shapes"] = True
7174

@@ -251,6 +254,7 @@ def lower_module_and_test_output(
251254
test_inputs=None,
252255
first_output_only=False,
253256
expect_no_delegates=False,
257+
compile_options=None,
254258
):
255259
"""
256260
Helper testing function that takes a torch.nn.Module and lowers it to Vulkan with
@@ -262,7 +266,12 @@ def lower_module_and_test_output(
262266
model.eval()
263267
model(*sample_inputs)
264268

265-
edge_program = lower_module(model, sample_inputs, dynamic_shapes=dynamic_shapes)
269+
edge_program = lower_module(
270+
model,
271+
sample_inputs,
272+
dynamic_shapes=dynamic_shapes,
273+
compile_options=compile_options,
274+
)
266275

267276
et_program = edge_program.to_executorch()
268277

@@ -1314,6 +1323,84 @@ def forward(self, x):
13141323
sample_inputs,
13151324
)
13161325

1326+
def test_vulkan_backend_redundant_view_to_output(self):
1327+
# Returning a value and a redundant view of it: once the view is
1328+
# removed, both outputs are the same node. The serializer must still
1329+
# emit two output ids, or the runtime's argument count check fails.
1330+
class RedundantViewToOutputModule(torch.nn.Module):
1331+
def forward(self, x):
1332+
y = x + 1
1333+
return y, y.view(2, 48)
1334+
1335+
sample_inputs = (torch.randn(size=(2, 48), dtype=torch.float32),)
1336+
1337+
self.lower_module_and_test_output(
1338+
RedundantViewToOutputModule(),
1339+
sample_inputs,
1340+
)
1341+
1342+
def test_vulkan_backend_duplicate_output(self):
1343+
# The same guarantee without any view involved: `all_input_nodes`
1344+
# de-duplicates, so this loses an output slot too.
1345+
class DuplicateOutputModule(torch.nn.Module):
1346+
def forward(self, x):
1347+
y = x + 1
1348+
return y, y
1349+
1350+
sample_inputs = (torch.randn(size=(2, 48), dtype=torch.float32),)
1351+
1352+
self.lower_module_and_test_output(
1353+
DuplicateOutputModule(),
1354+
sample_inputs,
1355+
)
1356+
1357+
def test_vulkan_backend_buffer_mutation_aliasing_user_output(self):
1358+
# A mutated buffer returned through a redundant view, with the mutation
1359+
# aliased onto its input. Once the view is removed the buffer-mutation
1360+
# slot and the user output are the same node, but only the user output
1361+
# gets a slot in the delegate call, so keeping both would leave the
1362+
# serialized graph expecting one argument more than the call provides.
1363+
#
1364+
# Run it twice: the state has to carry over, which is what says the
1365+
# mutation is really being applied in place rather than quietly
1366+
# dropped along with its output slot.
1367+
class MutationAliasModule(torch.nn.Module):
1368+
def __init__(self):
1369+
super().__init__()
1370+
self.register_buffer("state", torch.zeros(2, 48))
1371+
1372+
def forward(self, x):
1373+
self.state.add_(x)
1374+
return self.state.view(2, 48)
1375+
1376+
sample_inputs = (torch.ones(size=(2, 48), dtype=torch.float32),)
1377+
test_inputs = [(torch.ones(size=(2, 48), dtype=torch.float32),)]
1378+
1379+
self.lower_module_and_test_output(
1380+
MutationAliasModule(),
1381+
sample_inputs,
1382+
test_inputs=test_inputs,
1383+
compile_options={"alias_buffer_mutations": True},
1384+
)
1385+
1386+
def test_vulkan_backend_view_chain_collapsing_to_input_shape(self):
1387+
# A chain of views whose net effect is the original shape. Fusing the
1388+
# chain leaves a view onto the shape it started from, which the
1389+
# redundant-op sweep then removes; the output count must survive both
1390+
# rewrites.
1391+
class ViewChainModule(torch.nn.Module):
1392+
def forward(self, x):
1393+
y = x + 1
1394+
z = y.view(4, 24).view(8, 12).view(2, 48)
1395+
return y, z
1396+
1397+
sample_inputs = (torch.randn(size=(2, 48), dtype=torch.float32),)
1398+
1399+
self.lower_module_and_test_output(
1400+
ViewChainModule(),
1401+
sample_inputs,
1402+
)
1403+
13171404
def test_vulkan_backend_view_int(self):
13181405
class ViewModule(torch.nn.Module):
13191406
def __init__(self):

‎backends/vulkan/test/test_vulkan_passes.py‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55

66
from executorch.backends.vulkan._passes.conv1d_as_conv2d import Conv1dAsConv2dPass
77
from executorch.backends.vulkan._passes.fuse_patterns import FusePatternsPass
8+
from executorch.backends.vulkan._passes.remove_redundant_ops import (
9+
RemoveRedundantOpsTransform,
10+
)
811

912
from executorch.exir import EdgeCompileConfig, EdgeProgramManager, to_edge
1013

@@ -877,3 +880,60 @@ def forward(self, x):
877880

878881
gm = ep.graph_module
879882
self.assertEqual(op_node_count(gm, "q8ta_pixel_shuffle.default"), 0)
883+
884+
def test_remove_same_shape_view_copy(self):
885+
"""A view onto the shape it already has is dropped."""
886+
887+
class SameShapeViewModule(torch.nn.Module):
888+
def forward(self, x):
889+
# Two views that cancel out. FuseViewCopyTransform collapses the
890+
# chain into a single view back to the original shape, which is
891+
# what leaves a redundant node behind in a real graph.
892+
return (x.view(4, 24).view(2, 48) + 1.0).view(2, 48)
893+
894+
model = SameShapeViewModule()
895+
inputs = (torch.rand(size=(2, 48), dtype=torch.float32),)
896+
program = torch.export.export(model, inputs)
897+
edge_program = to_edge(
898+
program, compile_config=EdgeCompileConfig(_check_ir_validity=False)
899+
)
900+
ep = edge_program._edge_programs["forward"]
901+
902+
before = op_node_count(ep.graph_module, "view_copy.default")
903+
self.assertGreater(before, 0)
904+
905+
RemoveRedundantOpsTransform().call(ep.graph_module)
906+
gm = ep.graph_module
907+
self.assertLess(op_node_count(gm, "view_copy.default"), before)
908+
909+
# Every view that survived changes the shape.
910+
for node in gm.graph.nodes:
911+
if get_target_canonical_name(node) == "view_copy.default":
912+
self.assertNotEqual(
913+
node.args[0].meta["val"].shape, node.meta["val"].shape
914+
)
915+
916+
self.assertTrue(torch.allclose(ep.module()(*inputs), model(*inputs)))
917+
918+
def test_rank_changing_view_copy_is_kept(self):
919+
"""A view that changes the shape must survive.
920+
921+
Its consumers read their argument's shape, so removing it makes them see
922+
the wrong rank even when both shapes occupy the same image extents.
923+
"""
924+
925+
class RankChangeModule(torch.nn.Module):
926+
def forward(self, x):
927+
return x.view(1, 2, 48) + 1.0
928+
929+
model = RankChangeModule()
930+
inputs = (torch.rand(size=(2, 48), dtype=torch.float32),)
931+
program = torch.export.export(model, inputs)
932+
edge_program = to_edge(
933+
program, compile_config=EdgeCompileConfig(_check_ir_validity=False)
934+
)
935+
ep = edge_program._edge_programs["forward"]
936+
937+
before = op_node_count(ep.graph_module, "view_copy.default")
938+
RemoveRedundantOpsTransform().call(ep.graph_module)
939+
self.assertEqual(op_node_count(ep.graph_module, "view_copy.default"), before)

‎backends/vulkan/vulkan_preprocess.py‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,12 @@ def preprocess( # noqa: C901
199199
SqueezeUnsqueezeInputs(),
200200
FuseViewCopyTransform(),
201201
ViewCopyToSqueezeUnsqueezePass(),
202+
# Again: the three passes above rewrite views, and collapsing a
203+
# chain of them can leave a view onto the shape it started from.
204+
# Those reach the runtime as a shader that reads every texel and
205+
# writes it back, and the earlier run happened before they were
206+
# created.
207+
RemoveRedundantOpsTransform(),
202208
],
203209
)
204210

0 commit comments

Comments
 (0)