Skip to content

Commit 6829cff

Browse files
Conarnarfacebook-github-bot
authored andcommitted
Keep delegate-consumed mutable buffers above the delegate (#21507)
Summary: `tag_constant_data` freezes a const/param/buffer into a delegate when all of its users are partitioned. It recognized a *mutated* buffer only when a direct user's node name is a key in `buffers_to_mutate`. That misses the case where a buffer both feeds a delegate and is mutated *inside* it: the mutation is produced by a `getitem` off the `call_delegate`, so the buffer's only direct user is the delegate node (whose name is not a `buffers_to_mutate` key), and the buffer is wrongly frozen as constant data instead of staying a method-owned mutable buffer. Additionally treat a placeholder as a mutated buffer when its FQN is a `buffers_to_mutate` target, so a buffer a delegate updates in place stays owned above the delegate. This is the case anticipated by `test_not_delegate_mutable_buffers` ("consider when the delegate can consume the mutable buffer") — e.g. a backend that embeds a pre-compiled engine which updates a KV cache in place. Authored with AI assistance (Claude Code). Differential Revision: D114288921
1 parent 644990e commit 6829cff

2 files changed

Lines changed: 186 additions & 13 deletions

File tree

exir/backend/test/test_partitioner.py

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -620,6 +620,158 @@ def partition(
620620
]
621621
self.assertEqual(len(copy_node), 1)
622622

623+
def test_delegate_consumes_mutable_buffer(self) -> None:
624+
"""The case anticipated by test_not_delegate_mutable_buffers: a delegate
625+
both reads AND updates a mutable buffer, so the buffer's only user is the
626+
already-fused call_delegate and its mutation is produced by a getitem off
627+
that delegate (as when a backend embeds a pre-compiled engine before
628+
partitioning). tag_constant_data must keep the buffer owned above the
629+
delegate -- it recognizes it because its FQN is a buffers_to_mutate
630+
target, not because a direct user is the mutation producer (the direct
631+
user is the call_delegate, whose name is not a mutation key).
632+
"""
633+
634+
class DelegatedMutableModule(torch.nn.Module):
635+
def __init__(self):
636+
super().__init__()
637+
self.register_buffer("my_state", torch.zeros(1))
638+
639+
def forward(self, x):
640+
y = x + self.my_state
641+
self.my_state.add_(x)
642+
return y
643+
644+
edge = exir.to_edge(
645+
torch.export.export(
646+
DelegatedMutableModule(), (torch.zeros(1),), strict=True
647+
)
648+
)
649+
self.assertGreater(
650+
len(edge.exported_program().graph_signature.buffers_to_mutate), 0
651+
)
652+
653+
# Fuse both adds (buffer read + mutation producer) into ONE call_delegate,
654+
# WITHOUT tagging constant data yet -- so afterwards my_state feeds the
655+
# fused call_delegate and its mutation is a getitem off it.
656+
class FuseAddsPartitioner(Partitioner):
657+
def __init__(self):
658+
super().__init__()
659+
self.delegation_spec = DelegationSpec(
660+
ExecutorBackend.__name__,
661+
[CompileSpec(key, value) for key, value in self.spec.items()],
662+
)
663+
664+
def partition(
665+
self, edge_exported_program: ExportedProgram
666+
) -> PartitionResult:
667+
partition_tags = {}
668+
for node in edge_exported_program.graph.nodes:
669+
if node.op == "call_function" and node.target in [
670+
exir_ops.edge.aten.add.Tensor
671+
]:
672+
node.meta["delegation_tag"] = "tag0"
673+
partition_tags["tag0"] = self.delegation_spec
674+
return PartitionResult(
675+
tagged_exported_program=edge_exported_program,
676+
partition_tags=partition_tags,
677+
)
678+
679+
lowered = edge.to_backend(FuseAddsPartitioner())
680+
lowered_ep = lowered.exported_program()
681+
682+
# Sanity: my_state now feeds a call_delegate whose getitem is the mutation.
683+
gs = lowered_ep.graph_signature
684+
self.assertIn("my_state", set(gs.buffers_to_mutate.values()))
685+
mutate_producers = [
686+
name for name, buf in gs.buffers_to_mutate.items() if buf == "my_state"
687+
]
688+
self.assertEqual(len(mutate_producers), 1)
689+
self.assertTrue(mutate_producers[0].startswith("getitem"))
690+
691+
# A backend that embeds a pre-compiled multi-output engine tags the
692+
# delegate node itself, so the mutable buffer feeds a *tagged* delegate
693+
# whose getitem is the mutation. tag_constant_data's freeze loop would
694+
# pull such a buffer into the delegate unless it recognizes it as a
695+
# mutation target.
696+
for node in lowered_ep.graph.nodes:
697+
if node.op == "call_function" and "call_delegate" in str(node.target):
698+
node.meta["delegation_tag"] = "tag0"
699+
700+
tag_constant_data(lowered_ep)
701+
for node in lowered_ep.graph.nodes:
702+
if (
703+
node.op == "placeholder"
704+
and gs.inputs_to_buffers.get(node.name) == "my_state"
705+
):
706+
self.assertIsNone(
707+
node.meta.get("delegation_tag"),
708+
"mutable buffer consumed by a delegate must stay above it, "
709+
"not be frozen into the delegate as constant data",
710+
)
711+
712+
def test_delegate_consumes_buffer_keeps_only_mutated_above(self) -> None:
713+
"""Companion to test_delegate_consumes_mutable_buffer: a delegate that
714+
consumes both a mutated and a non-mutated buffer must keep only the
715+
*mutated* one above the delegate; the non-mutated buffer is still frozen
716+
into the delegate as constant data. Guards that the mutation-target check
717+
in tag_constant_data is surgical and leaves plain constants untouched.
718+
"""
719+
720+
class MixedModule(torch.nn.Module):
721+
def __init__(self):
722+
super().__init__()
723+
self.register_buffer("my_state", torch.zeros(2))
724+
self.register_buffer("frozen_w", torch.ones(2))
725+
726+
def forward(self, x):
727+
y = x + self.my_state + self.frozen_w
728+
self.my_state.add_(x)
729+
return y
730+
731+
edge = exir.to_edge(
732+
torch.export.export(MixedModule(), (torch.zeros(2),), strict=True)
733+
)
734+
735+
class FuseAddsPartitioner(Partitioner):
736+
def __init__(self):
737+
super().__init__()
738+
self.delegation_spec = DelegationSpec(
739+
ExecutorBackend.__name__,
740+
[CompileSpec(key, value) for key, value in self.spec.items()],
741+
)
742+
743+
def partition(
744+
self, edge_exported_program: ExportedProgram
745+
) -> PartitionResult:
746+
partition_tags = {}
747+
for node in edge_exported_program.graph.nodes:
748+
if node.op == "call_function" and node.target in [
749+
exir_ops.edge.aten.add.Tensor
750+
]:
751+
node.meta["delegation_tag"] = "tag0"
752+
partition_tags["tag0"] = self.delegation_spec
753+
return PartitionResult(
754+
tagged_exported_program=edge_exported_program,
755+
partition_tags=partition_tags,
756+
)
757+
758+
lowered_ep = edge.to_backend(FuseAddsPartitioner()).exported_program()
759+
for node in lowered_ep.graph.nodes:
760+
if node.op == "call_function" and "call_delegate" in str(node.target):
761+
node.meta["delegation_tag"] = "tag0"
762+
tag_constant_data(lowered_ep)
763+
764+
gs = lowered_ep.graph_signature
765+
tag_by_buffer = {}
766+
for node in lowered_ep.graph.nodes:
767+
if node.op == "placeholder" and node.name in gs.inputs_to_buffers:
768+
tag_by_buffer[gs.inputs_to_buffers[node.name]] = node.meta.get(
769+
"delegation_tag"
770+
)
771+
# mutated buffer stays above the delegate; non-mutated buffer is frozen in.
772+
self.assertIsNone(tag_by_buffer.get("my_state"))
773+
self.assertEqual(tag_by_buffer.get("frozen_w"), "tag0")
774+
623775
def test_buffer_mutation1(self):
624776
class TestModule(torch.nn.Module):
625777
def __init__(self):

exir/backend/utils.py

Lines changed: 34 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,37 @@ def format_delegated_graph(graph_module: torch.fx.GraphModule) -> str:
342342
return graph_format_str
343343

344344

345+
def _find_mutated_buffers(
346+
edge_program: ExportedProgram,
347+
params_map: Dict[str, str],
348+
buffers_map: Dict[str, str],
349+
constants_map: Dict[str, str],
350+
buffers_to_mutate: Dict[str, str],
351+
) -> Set[torch.fx.Node]:
352+
"""Return the const/param/buffer placeholder nodes that are mutated (and so
353+
must not be frozen into a delegate as constant data).
354+
355+
A buffer is mutated when a direct user produces a graph-signature buffer
356+
mutation, or when its FQN is a mutation target -- the latter covers a buffer
357+
mutated inside a delegate, whose mutation is produced by a getitem off the
358+
call_delegate rather than by a direct user of the buffer placeholder.
359+
"""
360+
mutated_buffer_targets = set(buffers_to_mutate.values())
361+
mutated_buffer: Set[torch.fx.Node] = set()
362+
for node in edge_program.graph.nodes:
363+
if node.op != "placeholder" or not (
364+
node.name in params_map
365+
or node.name in buffers_map
366+
or node.name in constants_map
367+
):
368+
continue
369+
if any(user.name in buffers_to_mutate for user in node.users) or (
370+
buffers_map.get(node.name) in mutated_buffer_targets
371+
):
372+
mutated_buffer.add(node)
373+
return mutated_buffer
374+
375+
345376
def tag_constant_data(edge_program: ExportedProgram) -> None:
346377
"""
347378
Util function for partitioners. This function tags the const/param/buffers nodes
@@ -357,19 +388,9 @@ def tag_constant_data(edge_program: ExportedProgram) -> None:
357388
constants_map = sig.inputs_to_lifted_tensor_constants
358389
buffers_to_mutate = sig.buffers_to_mutate
359390

360-
mutated_buffer = set()
361-
for node in edge_program.graph.nodes:
362-
if node.op == "placeholder" and (
363-
node.name in params_map
364-
or node.name in buffers_map
365-
or node.name in constants_map
366-
):
367-
for node_user in node.users:
368-
if node_user.name in buffers_to_mutate:
369-
logging.info(
370-
"The buffer node is a mutated buffer node, which is not constant."
371-
)
372-
mutated_buffer.add(node)
391+
mutated_buffer = _find_mutated_buffers(
392+
edge_program, params_map, buffers_map, constants_map, buffers_to_mutate
393+
)
373394

374395
for node in edge_program.graph.nodes:
375396
# go through const/param/buffer nodes, if all users of const/param/buffer nodes are partitioned then partition

0 commit comments

Comments
 (0)