Skip to content

Commit 206f7ce

Browse files
ThomasJannaudfacebook-github-bot
authored andcommitted
Preserve distinct mutation output names in graph signatures (#24105)
Summary: Problem `ExportGraphSignature.buffers_to_mutate` and `user_inputs_to_mutate` index writes by output node name. No-op folding and CSE can merge two writes into one node, so one of the mutation entries is lost from the signature. Solution Rebuild the signature from `output_specs` (`_get_updated_graph_signature` in `exir/_program_utils.py`). Copy a computed result only when two writes collide in the same mutation map, and keep positional user outputs and ordinary graph uses shared. Select ATen, edge or dim-order cloning from the program's verifier. Preserve value/debug metadata and FX provenance on the copies without inheriting allocation or custom-kernel placement annotations. Mutation output copies keep independent storage, as required by the existing output pointer contract. This is the ExecuTorch half of D125489522, split out so that ExecuTorch owners can review it on its own. The Helios half (removing the Helios-side duplicated-output splitter that this change makes unnecessary) stays in D125489522, stacked on top. fbcode and xplat copies are identical. {F2001970137} Differential Revision: D127072149
1 parent d533adf commit 206f7ce

6 files changed

Lines changed: 356 additions & 6 deletions

File tree

‎exir/BUCK‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,7 @@ fbcode_target(_kind = runtime.python_library,
296296
],
297297
deps = [
298298
"//caffe2:torch",
299+
"//executorch/exir/dialects:lib",
299300
],
300301
)
301302

‎exir/_program_utils.py‎

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@
77
# pyre-unsafe
88

99
import torch
10+
from executorch.exir.dialects._ops import ops as exir_ops
1011
from torch.export.exported_program import (
1112
ConstantArgument,
1213
ExportGraphSignature,
1314
InputSpec,
1415
OutputSpec,
1516
)
17+
from torch.export.graph_signature import OutputKind
18+
from torch.fx.traceback import NodeSource, NodeSourceAction
1619

1720

1821
def _get_updated_range_constraints(gm, old_range_constraints=None):
@@ -54,9 +57,12 @@ def get_shape_env(gm):
5457
def _get_updated_graph_signature(
5558
old_signature: ExportGraphSignature,
5659
new_gm: torch.fx.GraphModule,
60+
*,
61+
use_edge_ops: bool = False,
62+
use_dim_order: bool = False,
5763
) -> ExportGraphSignature:
5864
"""
59-
Update the graph signature's user_input/user_outputs.
65+
Update the positional signature, disambiguating mutation writes in-place.
6066
"""
6167
new_input_specs = []
6268
i = 0
@@ -87,6 +93,53 @@ def _get_updated_graph_signature(
8793
output_node = new_gm.graph.output_node()
8894
assert output_node.op == "output"
8995

96+
output_args = output_node.args[0]
97+
assert isinstance(output_args, (tuple, list))
98+
assert len(output_args) == len(
99+
old_signature.output_specs
100+
), "Number of outputs changed after transformation"
101+
seen_mutations: set[tuple[OutputKind, torch.fx.Node]] = set()
102+
new_output_args = list(output_args)
103+
for i, (node, spec) in enumerate(zip(output_args, old_signature.output_specs)):
104+
if spec.kind not in (
105+
OutputKind.BUFFER_MUTATION,
106+
OutputKind.USER_INPUT_MUTATION,
107+
):
108+
continue
109+
assert isinstance(node, torch.fx.Node)
110+
mutation = (spec.kind, node)
111+
if mutation in seen_mutations:
112+
# Mutation dictionaries are keyed by node name, while user outputs
113+
# are positional and may share a node. Copy only colliding writes.
114+
with new_gm.graph.inserting_before(output_node):
115+
clone = new_gm.graph.call_function(
116+
(
117+
(
118+
exir_ops.edge.dim_order_ops._clone_dim_order.default
119+
if use_dim_order
120+
else exir_ops.edge.aten.clone.default
121+
)
122+
if use_edge_ops
123+
else torch.ops.aten.clone.default
124+
),
125+
args=(node,),
126+
)
127+
clone.meta = {
128+
key: node.meta[key]
129+
for key in ("val", "stack_trace", "nn_module_stack", "debug_handle")
130+
if key in node.meta
131+
}
132+
clone.meta["from_node"] = [
133+
NodeSource(
134+
node, "_get_updated_graph_signature", NodeSourceAction.CREATE
135+
)
136+
]
137+
new_output_args[i] = clone
138+
seen_mutations.add(mutation)
139+
if new_output_args != list(output_args):
140+
output_node.args = (tuple(new_output_args),) + output_node.args[1:]
141+
new_gm.recompile()
142+
90143
new_output_specs = []
91144
for i, node in enumerate(output_node.args[0]):
92145
assert i < len(

‎exir/pass_manager.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,10 @@ def __call__( # noqa: C901
225225
new_graph_signature = _get_updated_graph_signature(
226226
exported_program.graph_signature,
227227
res.graph_module,
228+
use_edge_ops=exported_program.dialect != "ATEN",
229+
use_dim_order=getattr(
230+
exported_program.verifier(), "use_dim_order", False
231+
),
228232
)
229233
# ExportedProgram.__init__ runs this, but this path mutates
230234
# the program in place and never reconstructs it. Duplicate

‎exir/program/_program.py‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,14 @@ def _update_exported_program_graph_module(
206206
root=gm,
207207
graph=gm.graph,
208208
graph_signature=_get_updated_graph_signature(
209-
exported_program.graph_signature, gm
209+
exported_program.graph_signature,
210+
gm,
211+
use_edge_ops=exported_program.dialect != "ATEN",
212+
use_dim_order=getattr(
213+
(override_verifiers or [exported_program.verifier])[0](),
214+
"use_dim_order",
215+
False,
216+
),
210217
),
211218
state_dict=exported_program.state_dict,
212219
range_constraints=_get_updated_range_constraints(
@@ -711,7 +718,10 @@ def _to_edge(ep, config: EdgeCompileConfig) -> "ExirExportedProgram":
711718
root=new_gm,
712719
graph=new_gm.graph,
713720
graph_signature=_get_updated_graph_signature(
714-
new_ep.exported_program.graph_signature, new_gm
721+
new_ep.exported_program.graph_signature,
722+
new_gm,
723+
use_edge_ops=True,
724+
use_dim_order=not config._skip_dim_order,
715725
),
716726
state_dict=new_ep.exported_program.state_dict,
717727
range_constraints=new_ep.exported_program.range_constraints,
@@ -851,7 +861,12 @@ def _generate_edge_program(
851861
edge_program = ExportedProgram(
852862
root=gm,
853863
graph=gm.graph,
854-
graph_signature=_get_updated_graph_signature(program.graph_signature, gm),
864+
graph_signature=_get_updated_graph_signature(
865+
program.graph_signature,
866+
gm,
867+
use_edge_ops=True,
868+
use_dim_order=not config._skip_dim_order,
869+
),
855870
state_dict=program.state_dict,
856871
range_constraints=program.range_constraints,
857872
module_call_graph=program.module_call_graph,

‎exir/tests/targets.bzl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ def define_common_targets(is_fbcode = False):
220220
":models",
221221
"//caffe2:torch",
222222
"//caffe2/functorch:functorch_src",
223+
"//executorch/exir:_program_utils",
223224
"//executorch/exir:graph_module",
224225
"//executorch/exir:lib",
225226
"//executorch/exir:memory",

0 commit comments

Comments
 (0)