Skip to content

Commit 5ef2767

Browse files
authored
Preserve in-place result aliases and lifetimes during memory planning (pytorch#23526)
### Summary With `run_reinplace_pass=True`, an in-place result can remain live after the memory planner considers its storage dead. A subsequent allocation then overwrites it. This reproduces with `cache = torch.zeros_like(x).index_put((indices,), values); scratch = x + 10; return cache + scratch, cache` and caused incorrect, non-repeatable Whisper static-prefill logits and KV caches. Propagate in-place result aliases in `SpecPropPass`, before downstream outputs and getitems capture their specs. Include those aliased specs in lifetime collection instead of skipping in-place nodes. Shared TensorSpecs remain deduplicated for allocation. The four regression cases cover greedy and naive planning, with the cache either returned or used only as an intermediate; all four fail before this fix and pass afterward. In-place nodes are no longer skipped in `collect_specs_from_nodes`: if aliasing is skipped for a return (no matching `Tensor(a!)` annotation, or the mutated input can't be resolved), that result now gets its own planned buffer, where before it was never allocated. `MemoryPlanningPass.run` now also verifies, before planning, that every write-aliased in-place result shares its mutated input's TensorSpec, and raises `InternalError` otherwise. Without this check, an in-place op introduced after `SpecPropPass` would silently get its own planned buffer that the kernel never writes. The check covers only returns that the schema declares as aliasing an input, and skips the same cases aliasing skips, so ops without alias annotations are unaffected. A fifth test breaks the alias before planning and confirms the check raises. This addresses allocation lifetimes after a legal in-place rewrite. It is independent of pytorch#23488, which prevents an illegal rewrite when the original value is also returned. ### Test plan Local validation in a Python 3.12 environment with the native runtime built from source: **131 tests passed** across `executorch.exir.tests.test_memory_planning`, `test_banked_memory_planning`, `test_reinplace_pass`, and `test_remove_view_copy`; all seven `TestPasses.test_spec_prop_pass*` cases passed; and all 33 Cadence `backends/cadence/aot/tests/test_memory_passes.py` tests passed, so the new check does not trip in Cadence's planner. `lintrunner` on the changed files and `git diff --check` passed. The minimal runtime reproducer returns both tensors exactly as eager PyTorch after the fix, including on repeated calls. For `x=arange(8)`, indices `[0,3]`, and values `[9,17]`, its first result changes from the incorrect `[20,22,24,26,28,30,32,34]` to `[19,11,12,30,14,15,16,17]`. Also validated cached pretrained `openai/whisper-tiny`, FP32, through the Transformers exporter and XNNPACK, with four CPU threads. Default and reinplace outputs match bit-for-bit across encoder, dynamic-cache prefill/decode, and static-cache prefill/decode, including repeat executions. Static-prefill maximum absolute logit error against eager is `4.39e-5` for both variants. The corrected static-prefill arena is 31,219,232 bytes versus 33,899,072 bytes without reinplace. Local static-prefill A/B/B/A median latencies after the fix (milliseconds): | A1 default | B1 reinplace | B2 reinplace | A2 default | | ---: | ---: | ---: | ---: | | 232.730 | 26.247 | 25.989 | 232.396 | This is an 8.90× component speedup from enabling reinplace with correct outputs, not a full-transcription measurement. Each block was a fresh process with ten warmups and 150 measured calls, fixed inputs restored outside timing, and four pinned guest CPU cores. Other components had byte-identical A/B artifacts. The original comparison used PyTorch 2.13.0+cpu and the same installed ExecuTorch 1.5.0 native runtime for both variants. Authored with AI assistance using Codex at the submitter's direction.
1 parent 2c36163 commit 5ef2767

9 files changed

Lines changed: 298 additions & 160 deletions

File tree

‎exir/BUCK‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,19 @@ fbcode_target(_kind = runtime.python_library,
168168
],
169169
)
170170

171+
fbcode_target(_kind = runtime.python_library,
172+
name = "inplace_aliasing",
173+
srcs = [
174+
"inplace_aliasing.py",
175+
],
176+
deps = [
177+
":error",
178+
":tensor",
179+
"//caffe2:torch",
180+
"//executorch/exir/operator:convert",
181+
],
182+
)
183+
171184
fbcode_target(_kind = runtime.python_library,
172185
name = "memory_planning",
173186
srcs = [
@@ -177,6 +190,7 @@ fbcode_target(_kind = runtime.python_library,
177190
":control_flow",
178191
":delegate",
179192
":error",
193+
":inplace_aliasing",
180194
":memory",
181195
":schema",
182196
":tensor",

‎exir/emit/_emitter.py‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -638,8 +638,8 @@ def _emit_spec(self, spec: ValueSpec) -> _EmitterValue:
638638
"""Given the provided spec constructs the corresponding EValue from it and then emits it.
639639
640640
If `spec` was already emitted earlier (e.g., because two FX
641-
nodes share the same TensorSpec object — typically because the
642-
planner's `_alias_inplace_result_specs` aliased an in-place
641+
nodes share the same TensorSpec object — typically because
642+
`alias_inplace_result_specs` aliased an in-place
643643
op's result onto its mutated input), reuse the existing
644644
value_id. This keeps the invariant "one TensorSpec ↔ one
645645
Value" so downstream emit doesn't create duplicate Values for
@@ -2093,9 +2093,8 @@ def _is_buffer(node: Node, graph_signature: ExportGraphSignature) -> bool:
20932093

20942094
# Populate spec2id_dict so downstream `_emit_spec` calls (e.g.,
20952095
# for in-place op result FX nodes whose spec was aliased onto
2096-
# this placeholder's spec by the planner's
2097-
# `_alias_inplace_result_specs`) reuse this placeholder's
2098-
# value_id rather than creating a new Value.
2096+
# this placeholder's spec by `alias_inplace_result_specs`) reuse
2097+
# this placeholder's value_id rather than creating a new Value.
20992098
if isinstance(spec, TensorSpec):
21002099
self.emitter_state.spec2id_dict[spec] = value.id
21012100

‎exir/inplace_aliasing.py‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
# All rights reserved.
3+
#
4+
# This source code is licensed under the BSD-style license found in the
5+
# LICENSE file in the root directory of this source tree.
6+
7+
# pyre-strict
8+
9+
import logging
10+
from typing import List, Optional
11+
12+
import torch
13+
from executorch.exir.error import internal_assert, InternalError
14+
from executorch.exir.operator.convert import (
15+
is_inplace_variant,
16+
output_to_aliased_input_map,
17+
unwrap_op_overload,
18+
)
19+
from executorch.exir.tensor import TensorSpec
20+
21+
22+
def is_inplace_node(node: torch.fx.Node) -> bool:
23+
if node.op != "call_function":
24+
return False
25+
target = node.target
26+
if not isinstance(target, torch._ops.OpOverload) and not isinstance(
27+
getattr(target, "_op", None), torch._ops.OpOverload
28+
):
29+
return False
30+
op = unwrap_op_overload(target)
31+
return is_inplace_variant(op._schema.name, op._schema.overload_name)
32+
33+
34+
def alias_inplace_result_specs(node: torch.fx.Node) -> None: # noqa: C901
35+
"""Alias an in-place op's result TensorSpec(s) onto the corresponding
36+
input's spec.
37+
38+
In-place ops (schema kind == inplace) mutate one or more of their
39+
inputs and return tensors that alias them, declared via the
40+
``Tensor(a!)`` schema annotation. To make the memory planner treat
41+
result and aliased input as one storage, we copy the input's spec
42+
object onto the output's ``node.meta["spec"]`` slot during spec
43+
propagation, before consumers capture it.
44+
45+
Output→input correspondence is computed via
46+
``output_to_aliased_input_map``, which matches each return's
47+
write-alias set against the inputs that share it.
48+
49+
Gating:
50+
51+
- Only runs for in-place nodes (caller checks ``is_inplace_node``).
52+
- Multi-output in-place ops are supported when each return's alias
53+
set matches exactly one input's alias set.
54+
- Falls through silently when alias info is absent or unparseable,
55+
preserving the original spec. ``logging.debug`` records each
56+
early-return reason so silent regressions are observable.
57+
"""
58+
target = node.target
59+
op = unwrap_op_overload(target)
60+
61+
schema = op._schema
62+
out_to_in = output_to_aliased_input_map(schema)
63+
if not out_to_in:
64+
logging.debug(
65+
f"alias_inplace_result_specs: schema for {op} declares no "
66+
f"write-aliased outputs matching an input; skipping."
67+
)
68+
return
69+
70+
# Normalize the current spec container into a list for uniform
71+
# handling. Caller guarantees this node was identified as an
72+
# in-place op with a meta spec (see `is_inplace_node`), so an
73+
# unrecognized container shape is a real bug — assert loudly so it
74+
# surfaces in tests rather than silently disabling aliasing.
75+
current = node.meta.get("spec")
76+
if isinstance(current, TensorSpec):
77+
out_specs_list: List[Optional[TensorSpec]] = [current]
78+
return_container_kind = "scalar"
79+
elif isinstance(current, (list, tuple)):
80+
out_specs_list = list(current)
81+
return_container_kind = type(current).__name__
82+
else:
83+
raise InternalError(
84+
f"alias_inplace_result_specs: in-place node {node.name} "
85+
f"({op}) has unrecognized spec container of type "
86+
f"{type(current).__name__!r}; expected TensorSpec, list, "
87+
f"or tuple."
88+
)
89+
90+
# Compute new spec for each return; None means "keep original".
91+
# Mutated inputs are usually positional (the `Tensor(a!)` `self`
92+
# arg), but custom ops may pass them via kwargs — fall back to
93+
# `node.kwargs[arg_name]` in that case.
94+
replacements: List[Optional[TensorSpec]] = [None] * len(out_specs_list)
95+
96+
for out_idx, in_idx in out_to_in.items():
97+
if out_idx >= len(out_specs_list):
98+
logging.debug(
99+
f"alias_inplace_result_specs: schema for {op} declares "
100+
f"return {out_idx} but spec container has only "
101+
f"{len(out_specs_list)} entries; skipping this return."
102+
)
103+
continue
104+
in_node = _resolve_mutated_input(node, schema, in_idx)
105+
if in_node is None:
106+
continue
107+
# NOTE: alias unconditionally — including when the input is a
108+
# placeholder (named buffer / mutable input). Skipping
109+
# placeholders would leave a dangling out-arg on in-place op
110+
# instructions whose self is a buffer; the runtime kernel
111+
# writes to that out-arg's storage rather than mutating the
112+
# buffer, producing incorrect results. The companion change in
113+
# `_emit_spec` (emit/_emitter.py) deduplicates by spec identity
114+
# so this aliasing doesn't produce two Values for the same FQN.
115+
in_spec = in_node.meta.get("spec")
116+
if not isinstance(in_spec, TensorSpec):
117+
continue
118+
replacements[out_idx] = in_spec
119+
120+
if not any(r is not None for r in replacements):
121+
return
122+
123+
# Assemble the new spec container, preserving the original shape.
124+
new_list = [
125+
replacements[i] if replacements[i] is not None else out_specs_list[i]
126+
for i in range(len(out_specs_list))
127+
]
128+
if return_container_kind == "scalar":
129+
if isinstance(new_list[0], TensorSpec):
130+
node.meta["spec"] = new_list[0]
131+
elif return_container_kind == "list":
132+
node.meta["spec"] = list(new_list)
133+
elif return_container_kind == "tuple":
134+
node.meta["spec"] = tuple(new_list)
135+
136+
137+
def _resolve_mutated_input(
138+
node: torch.fx.Node, schema: torch.FunctionSchema, in_idx: int
139+
) -> Optional[torch.fx.Node]:
140+
"""Return the node passed as argument ``in_idx`` of an in-place op,
141+
preferring positional args and falling back to kwargs by name (custom
142+
ops may pass ``Tensor(a!)`` args via kwargs)."""
143+
in_node: object
144+
if in_idx < len(node.args):
145+
in_node = node.args[in_idx]
146+
else:
147+
arg_name = (
148+
schema.arguments[in_idx].name if in_idx < len(schema.arguments) else None
149+
)
150+
if arg_name is None or arg_name not in node.kwargs:
151+
logging.debug(
152+
f"_resolve_mutated_input: schema for {node.target} "
153+
f"expects mutated input at position {in_idx} "
154+
f"(name={arg_name!r}) but it is supplied neither "
155+
"positionally nor via kwargs; skipping."
156+
)
157+
return None
158+
in_node = node.kwargs[arg_name]
159+
return in_node if isinstance(in_node, torch.fx.Node) else None
160+
161+
162+
def verify_inplace_result_aliases(graph_module: torch.fx.GraphModule) -> None:
163+
"""Assert that every write-aliased in-place result shares its mutated
164+
input's TensorSpec.
165+
166+
``SpecPropPass`` establishes this aliasing before consumers capture the
167+
result spec. An in-place op added after spec propagation would otherwise
168+
get its own buffer that the kernel never writes.
169+
"""
170+
for module in graph_module.modules():
171+
if not isinstance(module, torch.fx.GraphModule):
172+
continue
173+
for node in module.graph.nodes:
174+
if not is_inplace_node(node):
175+
continue
176+
schema = unwrap_op_overload(node.target)._schema
177+
result_specs = node.meta.get("spec")
178+
if isinstance(result_specs, TensorSpec):
179+
result_specs = [result_specs]
180+
if not isinstance(result_specs, (list, tuple)):
181+
continue
182+
for out_idx, in_idx in output_to_aliased_input_map(schema).items():
183+
in_node = _resolve_mutated_input(node, schema, in_idx)
184+
if in_node is None or out_idx >= len(result_specs):
185+
continue
186+
in_spec = in_node.meta.get("spec")
187+
if not isinstance(in_spec, TensorSpec):
188+
continue
189+
internal_assert(
190+
result_specs[out_idx] is in_spec,
191+
f"In-place node {node.name} ({node.target}) result "
192+
f"{out_idx} does not share the TensorSpec of its mutated "
193+
f"input {in_node.name}. Run SpecPropPass after introducing "
194+
"in-place ops and before memory planning.",
195+
)

0 commit comments

Comments
 (0)