Skip to content

Commit 1802bcf

Browse files
authored
Fix reinplace liveness for returned tensors (pytorch#23488)
The reinplace pass did not include graph outputs in reverse liveness analysis. When a model returned both an original tensor and the result of index_put, the pass could rewrite index_put to index_put_ and change the original output. Treating output nodes as readers keeps returned tensors live and prevents this incorrect rewrite while preserving valid in-place rewrites. Regression tests cover returned originals, nested outputs, later readers, immutable inputs, view-copy behavior, and repeated mutable-buffer executions. Validation: 20 tests passed in exir/tests/test_reinplace_pass.py; repository commit lint hook passed. This PR was authored with assistance from Codex (OpenAI AI assistant).
1 parent 5ef2767 commit 1802bcf

2 files changed

Lines changed: 112 additions & 2 deletions

File tree

‎exir/passes/reinplace.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ def reinplace_pass( # noqa: C901
337337
338338
Safety rules:
339339
* The mutated arg must not be used by any later node in the
340-
graph.
340+
graph or be returned as an unmodified output.
341341
* If the mutated arg is a placeholder (program input), it must
342342
be a *mutable* input — i.e., declared in
343343
`graph_signature.user_inputs_to_mutate` or
@@ -425,7 +425,7 @@ def reinplace_pass( # noqa: C901
425425

426426
for node in reversed(ep.graph.nodes):
427427
if node.op != "call_function" or node.target not in resolved:
428-
if node.op == "call_function":
428+
if node.op in ("call_function", "output"):
429429
seen_nodes.update(node.all_input_nodes)
430430
continue
431431

‎exir/tests/test_reinplace_pass.py‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,116 @@ def forward(
9090
)
9191
)
9292

93+
def test_returned_original_is_not_reinplaced(self) -> None:
94+
class ReturnsOldAndNew(torch.nn.Module):
95+
def forward(self, x, indices, values):
96+
original = x + 1
97+
updated = original.index_put((indices,), values)
98+
return original, updated
99+
100+
model = ReturnsOldAndNew()
101+
first = (torch.zeros(5), torch.tensor([0]), torch.tensor([9.0]))
102+
edge = to_edge(export(model, first, strict=True))
103+
et = edge.to_executorch(ExecutorchBackendConfig(run_reinplace_pass=True))
104+
self.assertEqual(len(_find_nodes(et.exported_program(), "index_put_")), 0)
105+
106+
loaded = _load_for_executorch_from_buffer(et.buffer)
107+
for inputs in (
108+
first,
109+
(torch.full((5,), 2.0), torch.tensor([3]), torch.tensor([-2.0])),
110+
):
111+
with self.subTest(inputs=inputs):
112+
for actual, expected in zip(loaded.forward(inputs), model(*inputs)):
113+
torch.testing.assert_close(actual, expected)
114+
115+
def test_nested_output_keeps_original_live(self) -> None:
116+
class ReturnsNested(torch.nn.Module):
117+
def forward(self, x, indices, values):
118+
original = x + 1
119+
updated = original.index_put((indices,), values)
120+
return original, (original, updated)
121+
122+
inputs = (torch.zeros(5), torch.tensor([0]), torch.tensor([9.0]))
123+
edge = to_edge(export(ReturnsNested(), inputs, strict=True))
124+
ep = reinplace_pass(edge.exported_program())
125+
self.assertEqual(len(_find_nodes(ep, "index_put_")), 0)
126+
actual = ep.module()(*inputs)
127+
expected = ReturnsNested()(*inputs)
128+
for result, reference in zip(
129+
(actual[0], *actual[1]), (expected[0], *expected[1])
130+
):
131+
torch.testing.assert_close(result, reference)
132+
133+
def test_output_view_copy_preserves_original(self) -> None:
134+
class ReturnsViewAndUpdated(torch.nn.Module):
135+
def forward(self, x, indices, values):
136+
original = x + 1
137+
view = original.view(-1)
138+
updated = original.index_put((indices,), values)
139+
return view, updated
140+
141+
inputs = (torch.zeros(5), torch.tensor([0]), torch.tensor([9.0]))
142+
edge = to_edge(export(ReturnsViewAndUpdated(), inputs, strict=True))
143+
et = edge.to_executorch(ExecutorchBackendConfig(run_reinplace_pass=True))
144+
# The view is materialized before the update, so it keeps the old value.
145+
self.assertEqual(len(_find_nodes(et.exported_program(), "index_put_")), 1)
146+
loaded = _load_for_executorch_from_buffer(et.buffer)
147+
for actual, expected in zip(
148+
loaded.forward(inputs), ReturnsViewAndUpdated()(*inputs)
149+
):
150+
torch.testing.assert_close(actual, expected)
151+
152+
def test_later_reader_keeps_original_live(self) -> None:
153+
class ReadsOriginalLater(torch.nn.Module):
154+
def forward(self, x, indices, values):
155+
original = x + 1
156+
updated = original.index_put((indices,), values)
157+
return updated, original * 2
158+
159+
inputs = (torch.zeros(5), torch.tensor([0]), torch.tensor([9.0]))
160+
edge = to_edge(export(ReadsOriginalLater(), inputs, strict=True))
161+
ep = reinplace_pass(edge.exported_program())
162+
self.assertEqual(len(_find_nodes(ep, "index_put_")), 0)
163+
for actual, expected in zip(
164+
ep.module()(*inputs), ReadsOriginalLater()(*inputs)
165+
):
166+
torch.testing.assert_close(actual, expected)
167+
168+
def test_immutable_input_is_not_reinplaced(self) -> None:
169+
class UpdatesCopy(torch.nn.Module):
170+
def forward(self, x, indices, values):
171+
return x.index_put((indices,), values)
172+
173+
inputs = (torch.zeros(5), torch.tensor([0]), torch.tensor([9.0]))
174+
edge = to_edge(export(UpdatesCopy(), inputs, strict=True))
175+
ep = reinplace_pass(edge.exported_program())
176+
self.assertEqual(len(_find_nodes(ep, "index_put_")), 0)
177+
torch.testing.assert_close(ep.module()(*inputs), UpdatesCopy()(*inputs))
178+
torch.testing.assert_close(inputs[0], torch.zeros(5))
179+
180+
def test_mutable_buffer_reinplace_across_executions(self) -> None:
181+
class UpdatesBuffer(torch.nn.Module):
182+
def __init__(self):
183+
super().__init__()
184+
self.register_buffer("state", torch.zeros(5))
185+
186+
def forward(self, indices, values):
187+
self.state.index_put_((indices,), values)
188+
return self.state
189+
190+
first = (torch.tensor([0]), torch.tensor([1.0]))
191+
edge = to_edge(export(UpdatesBuffer(), first, strict=True))
192+
et = edge.to_executorch(ExecutorchBackendConfig(run_reinplace_pass=True))
193+
self.assertEqual(len(_find_nodes(et.exported_program(), "index_put_")), 1)
194+
195+
loaded = _load_for_executorch_from_buffer(et.buffer)
196+
reference = UpdatesBuffer()
197+
for inputs in (first, (torch.tensor([2]), torch.tensor([3.0]))):
198+
with self.subTest(inputs=inputs):
199+
torch.testing.assert_close(
200+
loaded.forward(inputs)[0], reference(*inputs)
201+
)
202+
93203
def test_cant_reinplace(self) -> None:
94204
"""Test that index_put on a mutable buffer that is viewed later is not safe."""
95205

0 commit comments

Comments
 (0)