Skip to content

Commit 775a436

Browse files
committed
[ET-VK] Rewrite conv1d only when the weight and the batch are ours to use
Two ways the rewrite could reach a graph it does not own, both from review. The weight is reshaped in place, so a weight with a second consumer is left behind at rank 4 while that consumer still expects rank 3. The comment claimed the rank check in _eligible() covered this; it does not. It stops the second conv1d being rewritten, which is precisely the problem: the conv keeps its 1-D arguments and loses its 1-D weight. Only rewrite when this convolution is the weight's sole consumer. A symbolic batch was compared against 1, which reads the value it was traced with rather than the values the graph accepts. A dimension free to be 4 at runtime could pass a test for 1 and reach a conv2d that rejects batched input. Require a batch that is already a plain 1. Tests move to test_vulkan_passes.py, where the graph can be inspected without a device. The delegate test only compared outputs, which an unrewritten conv1d satisfies just as well, so it could not have failed if the pass stopped firing. The shared-weight test fails against the previous pass. The symbolic-batch test does not: export specializes a size-1 dimension, so a batch that is both symbolic and hinted 1 is not constructible here, and a batch hinted higher is rejected either way. It pins the guard rather than reproducing the defect.
1 parent ba7afe9 commit 775a436

2 files changed

Lines changed: 116 additions & 4 deletions

File tree

‎backends/vulkan/_passes/conv1d_as_conv2d.py‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,25 @@ def _eligible(self, node: torch.fx.Node) -> bool:
7272
# The weight has to be a constant so it can be reshaped in place.
7373
if not is_param_node(self._exported_program, weight_node):
7474
return False
75+
# ...and it has to belong to this convolution alone. The reshape is a
76+
# mutation of shared state: a second consumer would keep reading the
77+
# weight it was built against and find it rank 4, whether that consumer
78+
# is another conv1d this pass then declines to rewrite, or an op that
79+
# has nothing to do with convolution.
80+
if len(weight_node.users) != 1:
81+
return False
7582
transposed, groups = node.args[6], node.args[8]
7683
if transposed or groups != 1:
7784
return False
7885
# conv2d does not support batched input at all, so a rewrite there
7986
# would turn a working conv1d into a throw.
80-
if in_shape[0] != 1:
87+
#
88+
# A symbolic batch is rejected rather than compared. Comparing it would
89+
# read the value it happened to be traced with, so a dimension free to
90+
# be 4 at runtime would pass a test for 1 and reach a conv2d that
91+
# cannot serve it. Only a batch that is already a plain 1 is provably 1
92+
# for every input the graph accepts.
93+
if not isinstance(in_shape[0], int) or in_shape[0] != 1:
8194
return False
8295
# Depthwise and pointwise 1-D convs keep their own shaders.
8396
if w_shape[0] < _IM2COL_MIN_C_OUT or w_shape[2] == 1:
@@ -98,8 +111,9 @@ def _rewrite(self, graph: torch.fx.Graph, node: torch.fx.Node) -> None:
98111
out_4d_shape = [out_shape[0], out_shape[1], 1, out_shape[2]]
99112

100113
# The weight tensor is contiguous, so inserting a singleton dim is a
101-
# pure metadata change. A weight shared by several convs is only
102-
# reshaped once; the rank check in _eligible() skips it afterwards.
114+
# pure metadata change. It is safe to do in place only because
115+
# _eligible() has established this convolution is the weight's sole
116+
# consumer.
103117
weight = get_param_tensor(self._exported_program, weight_node)
104118
assert weight is not None
105119
set_param_tensor(

‎backends/vulkan/test/test_vulkan_passes.py‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import unittest
2-
from typing import Optional, Tuple
2+
from typing import List, Optional, Tuple
33

44
import torch
55

6+
from executorch.backends.vulkan._passes.conv1d_as_conv2d import Conv1dAsConv2dPass
67
from executorch.backends.vulkan._passes.fuse_patterns import FusePatternsPass
78

89
from executorch.exir import EdgeCompileConfig, EdgeProgramManager, to_edge
@@ -86,7 +87,104 @@ def op_node_count(graph_module: torch.fx.GraphModule, canonical_op_name: str) ->
8687
return count
8788

8889

90+
def run_conv1d_as_conv2d(
91+
model: torch.nn.Module,
92+
sample_inputs: Tuple[torch.Tensor],
93+
dynamic_shapes=None,
94+
) -> torch.fx.GraphModule:
95+
"""Exports `model` to edge and runs Conv1dAsConv2dPass over it."""
96+
program = torch.export.export(
97+
model, sample_inputs, dynamic_shapes=dynamic_shapes, strict=True
98+
)
99+
edge_program = to_edge(
100+
program,
101+
compile_config=EdgeCompileConfig(_skip_dim_order=False, _check_ir_validity=False),
102+
)
103+
exported = edge_program.exported_program()
104+
105+
conv_pass = Conv1dAsConv2dPass()
106+
conv_pass._exported_program = exported
107+
return conv_pass(exported.graph_module).graph_module
108+
109+
110+
def conv_input_ranks(graph_module: torch.fx.GraphModule) -> List[int]:
111+
"""The rank of the input to every convolution in the graph."""
112+
ranks = []
113+
for node in graph_module.graph.nodes:
114+
if get_target_canonical_name(node) == "convolution.default":
115+
ranks.append(len(node.args[0].meta["val"].shape))
116+
return ranks
117+
118+
89119
class TestVulkanPasses(unittest.TestCase):
120+
def test_conv1d_as_conv2d_rewrites_eligible_conv(self):
121+
# Output equality cannot show this: an unrewritten conv1d computes the
122+
# same numbers. What distinguishes the two is the rank the convolution
123+
# is asked to work at, so that is what is asserted.
124+
class Conv1dModule(torch.nn.Module):
125+
def __init__(self):
126+
super().__init__()
127+
self.conv = torch.nn.Conv1d(32, 128, kernel_size=3, padding=1)
128+
129+
def forward(self, x):
130+
return self.conv(x)
131+
132+
gm = run_conv1d_as_conv2d(
133+
Conv1dModule(), (torch.randn(1, 32, 64),)
134+
)
135+
136+
self.assertEqual(conv_input_ranks(gm), [4])
137+
# Two views: one lifting the input to 4-D, one lowering the output back.
138+
self.assertEqual(op_node_count(gm, "view_copy.default"), 2)
139+
140+
def test_conv1d_as_conv2d_rejects_shared_weight(self):
141+
# The rewrite reshapes the weight in place. A second consumer would go
142+
# on reading a tensor that has silently become rank 4, so a shared
143+
# weight has to be left alone.
144+
class SharedWeightModule(torch.nn.Module):
145+
def __init__(self):
146+
super().__init__()
147+
self.weight = torch.nn.Parameter(torch.randn(128, 32, 3))
148+
149+
def forward(self, x, y):
150+
return (
151+
torch.nn.functional.conv1d(x, self.weight, padding=1),
152+
torch.nn.functional.conv1d(y, self.weight, padding=1),
153+
)
154+
155+
gm = run_conv1d_as_conv2d(
156+
SharedWeightModule(), (torch.randn(1, 32, 64), torch.randn(1, 32, 64))
157+
)
158+
159+
self.assertEqual(conv_input_ranks(gm), [3, 3])
160+
self.assertEqual(op_node_count(gm, "view_copy.default"), 0)
161+
162+
def test_conv1d_as_conv2d_rejects_symbolic_batch(self):
163+
# A batch free to vary at runtime is not a batch of 1, whatever value it
164+
# was traced with, and conv2d refuses batched input. The guard is on the
165+
# dimension being a plain integer rather than on its traced value, so
166+
# that a comparison never decides this from the hint alone.
167+
#
168+
# Traced at 2 because export specializes a size-1 dimension: the batch
169+
# has to be genuinely symbolic here for the graph to be the one the
170+
# guard exists for.
171+
class Conv1dModule(torch.nn.Module):
172+
def __init__(self):
173+
super().__init__()
174+
self.conv = torch.nn.Conv1d(32, 128, kernel_size=3, padding=1)
175+
176+
def forward(self, x):
177+
return self.conv(x)
178+
179+
gm = run_conv1d_as_conv2d(
180+
Conv1dModule(),
181+
(torch.randn(2, 32, 64),),
182+
dynamic_shapes={"x": {0: torch.export.Dim.AUTO}},
183+
)
184+
185+
self.assertEqual(conv_input_ranks(gm), [3])
186+
self.assertEqual(op_node_count(gm, "view_copy.default"), 0)
187+
90188
def test_fuse_torchao_quantized_embedding(self):
91189
"""A torchao-dialect 4-bit weight-only quantized embedding
92190
(torchao.dequantize_affine -> aten.embedding) should fuse into a single

0 commit comments

Comments
 (0)