Skip to content

Commit 9520b05

Browse files
authored
Keep output specs in sync in ExportedProgramPassBase (pytorch#22864)
Summary: A pass that replaces the graph's output node leaves `output_specs` naming the node that is no longer there. Nothing catches it at the time; it surfaces later as a signature that disagrees with the graph, and every pass author has to remember to fix it up by hand. `ExportedProgramPassBase.__call__` now realigns the specs with the graph between the pass and the postcondition check: ``` self.requires(exported_program) res = self.call(exported_program) signature_modified = _sync_output_specs(res.exported_program) self.ensures(res.exported_program) ``` Rewriting a spec to name the current output node is always safe. Changing an output between a node and a literal is not, so `_sync_output_specs` raises rather than guessing, and a length mismatch between outputs and specs raises too -- a pass that adds or removes an output is expected to maintain its own signature. Two consequences worth calling out: - A pass that only renames an output now reports `modified=True` even if it returned False, because the signature did change. That is what callers driving a pass to fixpoint need to see. - `ensures()` now receives the pass's result rather than the program that went in. The previous behaviour looks like an oversight -- a postcondition check that inspects the pre-pass program cannot check much -- and no pass in the tree relies on it: of the subclasses of `ExportedProgramPassBase`, exactly one overrides `ensures()`, and it calls `exported_program.validate()`, which wants the result. This was previously implemented as a `fused_quant`-local subclass. It is not specific to that package, so it moves here; the subclass goes away in the diff above this one. Reviewed By: mergennachin, DrJessop Differential Revision: D120259092 Pull Request resolved: pytorch#22864
1 parent c9c6819 commit 9520b05

2 files changed

Lines changed: 244 additions & 4 deletions

File tree

‎exir/pass_base.py‎

Lines changed: 93 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import traceback
1111
from abc import ABC, abstractmethod
1212
from contextlib import nullcontext
13-
from dataclasses import dataclass
13+
from dataclasses import dataclass, replace
1414
from typing import (
1515
Any,
1616
Callable,
@@ -38,6 +38,7 @@
3838
from torch._subclasses.fake_tensor import FakeTensor
3939
from torch._subclasses.functional_tensor import FunctionalTensor, FunctionalTensorMode
4040
from torch.export import ExportedProgram
41+
from torch.export.graph_signature import ConstantArgument
4142
from torch.fx import traceback as fx_traceback
4243
from torch.fx.experimental.proxy_tensor import PythonKeyTracer
4344
from torch.fx.graph import CodeGen
@@ -237,6 +238,65 @@ class ExportedProgramPassResult:
237238
modified: bool
238239

239240

241+
def _sync_output_specs(exported_program: ExportedProgram) -> bool:
242+
"""Realign ``output_specs`` with the graph's output node.
243+
244+
A pass that replaces an output node leaves the signature naming the old one,
245+
which later stages then disagree with. Rewriting the spec to match is always
246+
safe; changing an output between a node and a literal is not, so that raises
247+
rather than guessing.
248+
249+
Returns whether any spec changed.
250+
"""
251+
output_node = exported_program.graph_module.graph.output_node()
252+
outputs = output_node.args[0]
253+
assert isinstance(outputs, (tuple, list))
254+
255+
output_specs = exported_program.graph_signature.output_specs
256+
if len(outputs) != len(output_specs):
257+
raise ExportPassBaseError(
258+
f"Graph has {len(outputs)} outputs, but its signature has "
259+
f"{len(output_specs)} output specs"
260+
)
261+
262+
modified = False
263+
for output, output_spec in zip(outputs, output_specs):
264+
if isinstance(output_spec.arg, ConstantArgument):
265+
if isinstance(output, torch.fx.Node):
266+
raise ExportPassBaseError(
267+
f"Output {output.name} replaced a literal output; changing output "
268+
"representation is not supported"
269+
)
270+
if output_spec.arg.value != output:
271+
output_spec.arg = replace(output_spec.arg, value=output)
272+
modified = True
273+
continue
274+
if not isinstance(output, torch.fx.Node):
275+
raise ExportPassBaseError(
276+
f"Output {output_spec.arg.name} became a literal; changing output "
277+
"representation is not supported"
278+
)
279+
if output_spec.arg.name != output.name:
280+
output_spec.arg = replace(output_spec.arg, name=output.name)
281+
modified = True
282+
return modified
283+
284+
285+
def _rename_output_specs(exported_program: ExportedProgram, old: str, new: str) -> None:
286+
"""Rename ``old`` to ``new`` in ``output_specs`` only.
287+
288+
``ExportGraphSignature.get_replace_hook`` renames input specs alongside the
289+
output ones, so replacing a placeholder that is also returned renames its
290+
input spec out from under callers that still look it up by its old name
291+
(``delete_constant_placeholder``, for one).
292+
"""
293+
for output_spec in exported_program.graph_signature.output_specs:
294+
if isinstance(output_spec.arg, ConstantArgument):
295+
continue
296+
if output_spec.arg.name == old:
297+
output_spec.arg = replace(output_spec.arg, name=new)
298+
299+
240300
class ExportedProgramPassBase(ABC):
241301
"""
242302
Base interface for implementing passes that operate on ExportedProgram.
@@ -245,12 +305,41 @@ class ExportedProgramPassBase(ABC):
245305
def __call__(self, exported_program: ExportedProgram) -> ExportedProgramPassResult:
246306
"""
247307
Runs the precondition check, the pass itself, and the postcondition check.
308+
309+
Prefer the node replacement APIs (``replace_all_uses_with``,
310+
``replace_input_with``) in ``call``: a replace hook keeps the signature
311+
valid as the graph changes. Output specs are realigned with the graph
312+
afterwards regardless, so ``ensures`` sees a self-consistent program.
248313
"""
249314

250315
self.requires(exported_program)
251-
res = self.call(exported_program)
252-
self.ensures(exported_program)
253-
return res
316+
graph_module = exported_program.graph_module
317+
graph = graph_module.graph
318+
hook_modified = False
319+
320+
def tracking_hook(old: torch.fx.Node, new: str, user: torch.fx.Node) -> None:
321+
# Bound to the graph the hook was installed on: GraphModule.__deepcopy__
322+
# carries _replace_hooks over to the copy, and rewriting the copy must
323+
# not touch this program. The signature is read now rather than
324+
# captured because a pass can swap _graph_signature for a fresh object
325+
# while it runs.
326+
if user.graph is not graph or user.op != "output" or old.name == new:
327+
return
328+
nonlocal hook_modified
329+
hook_modified = True
330+
_rename_output_specs(exported_program, old.name, new)
331+
332+
with graph_module._set_replace_hook(tracking_hook):
333+
res = self.call(exported_program)
334+
result_graph_module = res.exported_program.graph_module
335+
if tracking_hook in result_graph_module._replace_hooks:
336+
result_graph_module._unregister_replace_node_hook(tracking_hook)
337+
signature_modified = _sync_output_specs(res.exported_program)
338+
self.ensures(res.exported_program)
339+
return ExportedProgramPassResult(
340+
res.exported_program,
341+
res.modified or hook_modified or signature_modified,
342+
)
254343

255344
@abstractmethod
256345
def call(self, exported_program: ExportedProgram) -> ExportedProgramPassResult:

‎exir/tests/test_pass_infra.py‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
# pyre-strict
99

10+
import copy
1011
import unittest
1112

1213
import executorch.exir as exir
@@ -579,3 +580,153 @@ def placeholder(
579580
new_input = self._find_input_node(new_graph_module)
580581

581582
self.assertNotEqual(self._symbolic_input_shape(new_input), original_snapshot)
583+
584+
585+
class ExportedProgramPassBaseOutputSpecTest(unittest.TestCase):
586+
"""__call__ realigns output specs with the graph before ensures() runs."""
587+
588+
class _Model(torch.nn.Module):
589+
def forward(self, x: torch.Tensor) -> torch.Tensor:
590+
return x + x
591+
592+
def _program(self) -> ExportedProgram:
593+
return to_edge(export(self._Model(), (torch.randn(2, 2),))).exported_program()
594+
595+
def test_replacing_the_output_node_updates_the_signature(self) -> None:
596+
class ReplaceOutputPass(ExportedProgramPassBase):
597+
def call(self, ep: ExportedProgram) -> ExportedProgramPassResult:
598+
graph = ep.graph_module.graph
599+
output_node = graph.output_node()
600+
(old,) = output_node.args[0]
601+
with graph.inserting_before(output_node):
602+
new = graph.call_function(
603+
exir_ops.edge.aten.mul.Tensor, (old.args[0], old.args[1])
604+
)
605+
new.meta = dict(old.meta)
606+
output_node.args = ((new,),)
607+
return ExportedProgramPassResult(ep, True)
608+
609+
program = self._program()
610+
result = ReplaceOutputPass()(program)
611+
612+
(spec,) = result.exported_program.graph_signature.output_specs
613+
graph_output_name = result.exported_program.graph.output_node().args[0][0].name
614+
self.assertEqual(spec.arg.name, graph_output_name)
615+
result.exported_program.validate()
616+
617+
def test_a_signature_only_change_is_reported_as_modified(self) -> None:
618+
"""A pass that renames the output reports modified even if it says False."""
619+
620+
class RenameOutputPass(ExportedProgramPassBase):
621+
def call(self, ep: ExportedProgram) -> ExportedProgramPassResult:
622+
ep.graph.output_node().args[0][0].name = "renamed_output"
623+
return ExportedProgramPassResult(ep, False)
624+
625+
result = RenameOutputPass()(self._program())
626+
627+
self.assertTrue(result.modified)
628+
(spec,) = result.exported_program.graph_signature.output_specs
629+
self.assertEqual(spec.arg.name, "renamed_output")
630+
631+
def test_output_count_mismatch_is_rejected(self) -> None:
632+
class DropOutputPass(ExportedProgramPassBase):
633+
def call(self, ep: ExportedProgram) -> ExportedProgramPassResult:
634+
output_node = ep.graph.output_node()
635+
output_node.args = ((*output_node.args[0], output_node.args[0][0]),)
636+
return ExportedProgramPassResult(ep, True)
637+
638+
with self.assertRaisesRegex(ExportPassBaseError, "output specs"):
639+
DropOutputPass()(self._program())
640+
641+
def test_the_replace_hook_updates_the_signature_during_the_pass(self) -> None:
642+
"""A pass using the replacement APIs sees a valid signature as it runs."""
643+
644+
signature_during_pass = []
645+
646+
class ReplaceViaApiPass(ExportedProgramPassBase):
647+
def call(self, ep: ExportedProgram) -> ExportedProgramPassResult:
648+
graph = ep.graph_module.graph
649+
(old,) = graph.output_node().args[0]
650+
with graph.inserting_before(graph.output_node()):
651+
new = graph.call_function(
652+
exir_ops.edge.aten.mul.Tensor, (old.args[0], old.args[1])
653+
)
654+
new.meta = dict(old.meta)
655+
old.replace_all_uses_with(new)
656+
signature_during_pass.append(
657+
ep.graph_signature.output_specs[0].arg.name
658+
)
659+
return ExportedProgramPassResult(ep, True)
660+
661+
result = ReplaceViaApiPass()(self._program())
662+
663+
graph_output_name = result.exported_program.graph.output_node().args[0][0].name
664+
self.assertEqual(signature_during_pass, [graph_output_name])
665+
666+
def test_replacing_a_returned_buffer_leaves_input_specs_alone(self) -> None:
667+
"""The hook is output-only, so the replaced placeholder stays deletable."""
668+
669+
class TwoBuffers(torch.nn.Module):
670+
def __init__(self) -> None:
671+
super().__init__()
672+
self.register_buffer("a", torch.ones(2, 2))
673+
self.register_buffer("b", torch.ones(2, 2))
674+
675+
def forward(self, x: torch.Tensor) -> tuple[torch.Tensor, torch.Tensor]:
676+
return self.a, x + self.b
677+
678+
replaced_name = []
679+
680+
class ReplaceReturnedBufferPass(ExportedProgramPassBase):
681+
def call(self, ep: ExportedProgram) -> ExportedProgramPassResult:
682+
names = {
683+
target: name
684+
for name, target in ep.graph_signature.inputs_to_buffers.items()
685+
}
686+
placeholders = {
687+
node.name: node
688+
for node in ep.graph.nodes
689+
if node.op == "placeholder"
690+
}
691+
old = placeholders[names["a"]]
692+
old.replace_all_uses_with(placeholders[names["b"]])
693+
replaced_name.append(old.name)
694+
return ExportedProgramPassResult(ep, True)
695+
696+
program = to_edge(export(TwoBuffers(), (torch.randn(2, 2),))).exported_program()
697+
698+
result = ReplaceReturnedBufferPass()(program)
699+
700+
signature = result.exported_program.graph_signature
701+
self.assertIn(replaced_name[0], signature.inputs_to_buffers)
702+
result.exported_program.validate()
703+
704+
def test_mutating_a_copied_graph_leaves_the_original_signature_alone(self) -> None:
705+
"""GraphModule.__deepcopy__ carries the replace hook over to the copy."""
706+
707+
signature_after_copy_edit = []
708+
709+
class MutateACopyPass(ExportedProgramPassBase):
710+
def call(self, ep: ExportedProgram) -> ExportedProgramPassResult:
711+
graph_module = copy.deepcopy(ep.graph_module)
712+
graph = graph_module.graph
713+
(old,) = graph.output_node().args[0]
714+
with graph.inserting_before(graph.output_node()):
715+
new = graph.call_function(
716+
exir_ops.edge.aten.mul.Tensor, (old.args[0], old.args[1])
717+
)
718+
new.meta = dict(old.meta)
719+
old.replace_all_uses_with(new)
720+
signature_after_copy_edit.append(
721+
ep.graph_signature.output_specs[0].arg.name
722+
)
723+
return ExportedProgramPassResult(ep, False)
724+
725+
program = self._program()
726+
original_output_name = program.graph_signature.output_specs[0].arg.name
727+
728+
result = MutateACopyPass()(program)
729+
730+
self.assertEqual(signature_after_copy_edit, [original_output_name])
731+
self.assertFalse(result.modified)
732+
program.validate()

0 commit comments

Comments
 (0)