diff --git a/exir/backend/canonical_partitioners/duplicate_constant_node_pass.py b/exir/backend/canonical_partitioners/duplicate_constant_node_pass.py index 07c3eb5e57a..478c2f35eea 100644 --- a/exir/backend/canonical_partitioners/duplicate_constant_node_pass.py +++ b/exir/backend/canonical_partitioners/duplicate_constant_node_pass.py @@ -40,6 +40,7 @@ def duplicate_constant_node( """ A pass to duplicate the attributes/constants node (the candidate_node) in the graph. Mostly used for duplicating light-weight data. If the data is too large, try tag it with "no_copy" to prevent high memory usage and make it as part of the output. + A mutated buffer is never copied, because a copy would not see its updates: this raises instead. Args: exported_program: the exported program to be modified. If constants nodes are copied, they will be added as new @@ -76,9 +77,23 @@ def duplicate_constant_node( raise RuntimeError( f"placeholder node for non-params, non-buffer, and non-tensor constants should not be tagged: {node} " ) - users = list(node.users.keys()) - for ith in range(1, len(node.users)): - copy_constant_or_attribute_fqn = node.name + f"_copy_{ith - 1}" + if ( + old_signature.inputs_to_buffers.get(node.name) + in old_signature.buffers_to_mutate.values() + ): + raise RuntimeError( + f"mutated buffer ({node}) cannot be copied, because a copy " + "would not see its updates. Tag it only when every node " + "that reads or writes it has the same tag, as " + "tag_mutated_buffer does, or leave it untagged." + ) + # The graph output keeps the original, which the output specs + # name, so then every other user reads a copy. + users = [user for user in node.users if user.op != "output"] + if len(users) == len(node.users): + users = users[1:] + for ith, user in enumerate(users): + copy_constant_or_attribute_fqn = node.name + f"_copy_{ith}" with exported_program.graph.inserting_before( constant_or_attribute_node ): @@ -96,26 +111,12 @@ def duplicate_constant_node( copied_constant_or_attribute_node.meta["val"] = ( constant_or_attribute_node.meta["val"] ) - new_args = tuple( - [ - ( - arg - if arg != constant_or_attribute_node - else copied_constant_or_attribute_node - ) - for arg in users[ith].args - ] + # This also reaches a constant that an op such as cat or + # index_put reads from a list. + user.replace_input_with( + constant_or_attribute_node, + copied_constant_or_attribute_node, ) - new_kwargs = { - key: ( - value - if value != constant_or_attribute_node - else copied_constant_or_attribute_node - ) - for key, value in users[ith].kwargs.items() - } - users[ith].args = new_args - users[ith].kwargs = new_kwargs # The copy keeps the original target, so it reads the # original tensor and needs no data of its own. new_input_specs.append( diff --git a/exir/backend/test/BUCK b/exir/backend/test/BUCK index 99913f65989..99a88f3b7fe 100644 --- a/exir/backend/test/BUCK +++ b/exir/backend/test/BUCK @@ -372,12 +372,15 @@ fbcode_target(_kind = runtime.python_test, "test_utils.py", ], deps = [ + ":backend_with_compiler_demo", ":op_partitioner_demo", "//caffe2:torch", "//executorch/exir:lib", "//executorch/exir/backend:backend_api", + "//executorch/exir/backend:compile_spec_schema", "//executorch/exir/backend:partitioner", "//executorch/exir/backend:utils", + "//executorch/exir/backend/test/demos/rpc:executor_backend_preprocess", "//executorch/exir/dialects:lib", "//executorch/extension/pybindings:portable_lib", # @manual "//executorch/runtime/executor/test:test_backend_compiler_lib", diff --git a/exir/backend/test/test_partitioner.py b/exir/backend/test/test_partitioner.py index b10cdcd436b..1efdba20c7a 100644 --- a/exir/backend/test/test_partitioner.py +++ b/exir/backend/test/test_partitioner.py @@ -658,32 +658,42 @@ def forward(self, x): def test_tag_mutated_buffer_detects_indirect_buffer_mutation(self) -> None: class MutableStateModule(torch.nn.Module): - def __init__(self): + def __init__(self, compute_new_value: bool): super().__init__() + self.compute_new_value = compute_new_value self.register_buffer("state", torch.zeros(1)) def forward(self, x): result = x + self.state - self.state.copy_(x) + self.state.copy_(x + 1 if self.compute_new_value else x) return result - edge = exir.to_edge( - torch.export.export(MutableStateModule(), (torch.ones(1),), strict=True) - ) - exported_program = edge.exported_program() - state_node = next( - node - for node in exported_program.graph.nodes - if exported_program.graph_signature.inputs_to_buffers.get(node.name) - == "state" - ) + # A new value that is a graph input is never in a partition, so the + # buffer then stays in the owning program. delegate_tag = "test_partition" - for user in state_node.users: - user.meta["delegation_tag"] = delegate_tag + for compute_new_value, expected_tag in ((True, delegate_tag), (False, None)): + with self.subTest(compute_new_value=compute_new_value): + edge = exir.to_edge( + torch.export.export( + MutableStateModule(compute_new_value), + (torch.ones(1),), + strict=True, + ) + ) + exported_program = edge.exported_program() + state_node = next( + node + for node in exported_program.graph.nodes + if exported_program.graph_signature.inputs_to_buffers.get(node.name) + == "state" + ) + for node in exported_program.graph.nodes: + if node.op == "call_function": + node.meta["delegation_tag"] = delegate_tag - tag_mutated_buffer(exported_program) + tag_mutated_buffer(exported_program) - self.assertEqual(state_node.meta.get("delegation_tag"), delegate_tag) + self.assertEqual(state_node.meta.get("delegation_tag"), expected_tag) def test_buffer_mutation1(self): class TestModule(torch.nn.Module): diff --git a/exir/backend/test/test_passes.py b/exir/backend/test/test_passes.py index 815a5b5bbed..697f8c78d74 100644 --- a/exir/backend/test/test_passes.py +++ b/exir/backend/test/test_passes.py @@ -86,3 +86,26 @@ def forward(self, x: torch.Tensor) -> torch.Tensor: self.assertEqual(len(copied_nodes), 1) self.assertNotEqual(user.kwargs["other"], buffer_node) + + def test_duplicate_constant_node_refuses_a_mutated_buffer(self) -> None: + class UpdateConst(torch.nn.Module): + def __init__(self): + super().__init__() + self.register_buffer("const", torch.zeros(2)) + + def forward(self, x): + y = x * self.const + self.const.add_(1) + return y + + program = exir.to_edge( + export(UpdateConst(), (torch.ones(2),), strict=True) + ).exported_program() + name = next( + node.name + for node in program.graph.nodes + if node.op == "placeholder" and is_buffer(program, node) + ) + + with self.assertRaisesRegex(RuntimeError, "would not see its updates"): + duplicate_constant_node(program, name) diff --git a/exir/backend/test/test_utils.py b/exir/backend/test/test_utils.py index ff11444930a..8da58e1cbd1 100644 --- a/exir/backend/test/test_utils.py +++ b/exir/backend/test/test_utils.py @@ -10,7 +10,18 @@ from executorch import exir from executorch.exir import to_edge from executorch.exir.backend.backend_api import to_backend -from executorch.exir.backend.partitioner import Partitioner, PartitionResult +from executorch.exir.backend.compile_spec_schema import CompileSpec +from executorch.exir.backend.partitioner import ( + DelegationSpec, + Partitioner, + PartitionResult, +) +from executorch.exir.backend.test.backend_with_compiler_demo import ( + BackendWithCompilerDemo, +) +from executorch.exir.backend.test.demos.rpc.executor_backend_preprocess import ( + ExecutorBackend, +) from executorch.exir.backend.test.op_partitioner_demo import AddMulPartitionerDemo from executorch.exir.backend.utils import ( format_delegated_graph, @@ -18,9 +29,14 @@ get_delegates, get_non_lowered_nodes, is_identical_graph, + tag_constant_data, + tag_mutated_buffer, ) from executorch.exir.dialects._ops import bind_pattern_to_op, ops as exir_ops +from executorch.extension.pybindings.portable_lib import ( # @manual + _load_for_executorch_from_buffer, +) from torch.export import export, ExportedProgram from torch.fx import symbolic_trace from torch.fx.passes.utils.matcher_utils import SubgraphMatcher @@ -390,3 +406,137 @@ def forward(self, a, x, b): payload_names, f"Delegate names mismatch: {delegate_names} vs {payload_names}", ) + + def test_to_backend_keeps_a_mutated_buffer_out_of_a_partial_delegate(self): + class UpdateState(torch.nn.Module): + def __init__(self, read: bool, returned: bool): + super().__init__() + self.read = read + self.returned = returned + self.register_buffer("state", torch.zeros(2)) + + def forward(self, x): + if self.read: + out = torch.sin(self.state) + (x + self.state) + else: + out = torch.sin(x) + self.state.copy_(x + x) + if self.returned: + return out, self.state + # In read mode, the last add reads the new value. + return out + self.state if self.read else out + + class TagNodes(Partitioner): + def __init__(self, tags): + super().__init__() + self.tags = tags + + def partition(self, program: ExportedProgram) -> PartitionResult: + for node in program.graph.nodes: + if node.name in self.tags: + node.meta["delegation_tag"] = self.tags[node.name] + tag_mutated_buffer(program) + spec = DelegationSpec( + BackendWithCompilerDemo.__name__, + [CompileSpec("max_value", bytes([4]))], + ) + return PartitionResult( + tagged_exported_program=program, + partition_tags={tag: spec for tag in self.tags.values()}, + ) + + # The delegate owns the buffer only when it reads and writes all of + # it. Otherwise the owning program keeps the one buffer: a copy in the + # delegate would never see the updates. + sin, add, writer = "aten_sin_default", "aten_add_tensor", "aten_add_tensor_2" + # Also the add that makes "out", and the add that reads the new value. + whole = {sin, add, "aten_add_tensor_1", writer, "aten_add_tensor_3"} + for read, returned, tags, owned_by_program in ( + (True, False, dict.fromkeys(whole, "tag0"), False), + (True, False, {sin: "tag0", add: "tag0", writer: "tag0"}, True), + (True, False, {sin: "tag0", add: "tag0"}, True), + (True, False, {sin: "tag0"}, True), + (True, False, {sin: "tag0", add: "tag1"}, True), + # Lowering cannot give a delegate a buffer whose new value is returned. + (True, True, {sin: "tag0", add: "tag0", writer: "tag0"}, True), + # Without readers, "add" is the writer. Some partitioners tag such a + # buffer themselves. + (False, False, {add: "tag0"}, True), + (False, False, {add: "tag0", "b_state": "tag0"}, False), + (False, True, {add: "tag0", "b_state": "tag0"}, True), + ): + with self.subTest(read=read, returned=returned, tags=tags): + x = torch.full((2,), 0.25) + lowered = to_edge( + export(UpdateState(read, returned), (x,), strict=True) + ).to_backend(TagNodes(tags)) + self.assertEqual( + [ + spec.arg.name + for spec in lowered.exported_program().graph_signature.input_specs + if spec.target == "state" + ], + ["b_state"] if owned_by_program else [], + ) + if tags == {sin: "tag0"}: + eager = UpdateState(read, returned) + module = _load_for_executorch_from_buffer( + lowered.to_executorch().buffer + ) + # The demo backend approximates sin with two Taylor terms. + for _ in range(2): + torch.testing.assert_close( + module.forward((x,))[0], eager(x), atol=1e-3, rtol=0 + ) + + def test_to_backend_copies_a_constant_read_by_ops_in_two_places(self): + class CatConst(torch.nn.Module): + def __init__(self, returned: bool): + super().__init__() + self.returned = returned + self.register_buffer("const", torch.ones(2)) + + def forward(self, x): + out = torch.cat([x, self.const]), torch.cat([self.const, x]) + return (*out, self.const) if self.returned else out + + class TagEachCat(Partitioner): + def partition(self, program: ExportedProgram) -> PartitionResult: + tags = {} + for node in program.graph.nodes: + if node.target == exir_ops.edge.aten.cat.default: + tag = f"tag{len(tags)}" + node.meta["delegation_tag"] = tag + tags[tag] = DelegationSpec(ExecutorBackend.__name__, []) + tag_constant_data(program) + return PartitionResult( + tagged_exported_program=program, partition_tags=tags + ) + + # cat reads the constant from a list. Each delegate gets its own copy, + # and the graph output keeps the original, which the output specs name. + for returned, constants in ( + (False, [["b_const"], ["b_const_copy_0"]]), + (True, [["b_const_copy_0"], ["b_const_copy_1"]]), + ): + with self.subTest(returned=returned): + lowered = to_edge( + export(CatConst(returned), (torch.ones(2),), strict=True) + ).to_backend(TagEachCat()) + program = lowered.exported_program() + self.assertEqual( + [ + [ + spec.arg.name + for spec in module.original_module.graph_signature.input_specs + if spec.target == "const" + ] + for _, module in program.graph_module.named_children() + ], + constants, + ) + if returned: + self.assertEqual( + program.graph.output_node().args[0][-1].name, "b_const" + ) + lowered.to_executorch() diff --git a/exir/backend/utils.py b/exir/backend/utils.py index fc8d0ceeb70..96ff686cc2a 100644 --- a/exir/backend/utils.py +++ b/exir/backend/utils.py @@ -23,6 +23,7 @@ from executorch.exir.dialects._ops import ops as exir_ops from executorch.exir.lowered_backend_module import create_submodule_from_nodes from tabulate import tabulate +from torch.export.graph_signature import OutputKind from torch.fx.experimental.symbolic_shapes import has_free_symbols from torch.fx.node import Node from torch.fx.passes.operator_support import OperatorSupportBase @@ -397,37 +398,66 @@ def tag_constant_data(edge_program: ExportedProgram) -> None: def tag_mutated_buffer(edge_program: ExportedProgram) -> None: """ - Util function for partitioners. This function tags the mutated buffer nodes - whose users all belong within the same partition. This should be called after tagging all other nodes. - Any buffer which is used as input to a subgraph, will be tagged with the same tag as that - subgraph. Throw error when buffers is used across different partitions. That is the - underlying data will be owned by multiple delegates. + Util function for partitioners. This function tags each mutated buffer + node whose readers (of the old or the new value) and writers all belong + within the same partition with the tag of that partition. This should be + called after tagging all other nodes. A buffer that is also read or + written outside that partition stays in the owning program and is passed + to the delegates as an input, because a delegate that owned it would + update its own copy, and the other users would not see the updates. A + buffer whose new value is also a user output stays in the owning program + too, and so does a buffer whose old value no node reads, unless the + partitioner already tagged it with its writers' tag. Any tag already on a + buffer that stays is removed, with a warning. """ # Cache signature lookups to avoid rebuilding dicts on every access. sig = edge_program.graph_signature buffers_map = sig.inputs_to_buffers - mutated_buffer_targets = set(sig.buffers_to_mutate.values()) + nodes = {node.name: node for node in edge_program.graph.nodes} + writers = {} + for output_name, buffer_name in sig.buffers_to_mutate.items(): + writers.setdefault(buffer_name, []).append(nodes[output_name]) + user_outputs = { + spec.arg.name + for spec in sig.output_specs + if spec.kind == OutputKind.USER_OUTPUT + } for node in edge_program.graph.nodes: + buffer_name = buffers_map.get(node.name) + if node.op != "placeholder" or buffer_name not in writers: + continue + # An op that reads the new value reads the buffer too. + new_value_readers = [ + user + for writer in writers[buffer_name] + for user in writer.users + if user.op != "output" + ] + user_tags = { + user.meta.get("delegation_tag", None) + for user in [*node.users, *writers[buffer_name], *new_value_readers] + } + # Lowering does not support a delegate that owns a buffer whose new + # value is also a user output. + returned = any(writer.name in user_outputs for writer in writers[buffer_name]) + # A buffer whose old value no node reads goes to a delegate only when + # the partitioner already gave it to the delegate that writes it. if ( - node.op == "placeholder" - and buffers_map.get(node.name) in mutated_buffer_targets + (node.users or node.meta.get("delegation_tag", None) in user_tags) + and len(user_tags) == 1 + and None not in user_tags + and not returned ): - user_tags = set() - for user in node.users: - user_tag = user.meta.get("delegation_tag", None) - if user_tag is not None: - user_tags.add(user_tag) - if len(user_tags) > 1: - logging.info( - f"The data node is used across multiple partitions, including {user_tags}. " - "If the data is too large and it's not preferred to copy, please tag the " - "constant node like node.['no_copy'] = True and they won't be copied." - ) - # Pick a deterministic consumer tag so a buffer shared across - # partitions is assigned reproducibly across runs. - if len(user_tags) > 0: - node.meta["delegation_tag"] = min(user_tags) + node.meta["delegation_tag"] = user_tags.pop() + continue + had_tag = node.meta.pop("delegation_tag", None) is not None + if had_tag or (node.users and user_tags != {None}): + logging.warning( + f"The mutated buffer {node.name} stays in the owning program: " + "it is read or written outside one partition, no node reads it, " + "or its new value is also returned." + ) def is_shape_dynamic(node: torch.fx.Node) -> bool: diff --git a/exir/emit/test/BUCK b/exir/emit/test/BUCK index 06f21991cdb..d0440dbac46 100644 --- a/exir/emit/test/BUCK +++ b/exir/emit/test/BUCK @@ -36,6 +36,7 @@ fbcode_target(_kind = runtime.python_test, "//executorch/exir/emit:lib", "//executorch/exir/passes:const_prop_pass", "//executorch/exir/passes:constant_prop_pass", + "//executorch/exir/passes:convert_constant_dim_order_pass", "//executorch/exir/passes:init_mutable_pass", "//executorch/exir/passes:propagate_device_pass", "//executorch/exir/tests:lib", diff --git a/exir/emit/test/test_emit.py b/exir/emit/test/test_emit.py index 8e465561b50..3c75954d86f 100644 --- a/exir/emit/test/test_emit.py +++ b/exir/emit/test/test_emit.py @@ -32,7 +32,7 @@ from executorch.exir.dialects._ops import ops as exir_ops from executorch.exir.emit import emit_program # noqa from executorch.exir.error import InternalError -from executorch.exir.passes import MemoryPlanningPass +from executorch.exir.passes import convert_constant_dim_order_pass, MemoryPlanningPass from executorch.exir.passes.constant_prop_pass import constant_prop_pass from executorch.exir.passes.init_mutable_pass import InitializedMutableBufferPass from executorch.exir.passes.sym_shape_eval_pass import ConstraintBasedSymShapeEvalPass @@ -2665,6 +2665,104 @@ def forward(self): expected_storage.append(j * 16 + i) self.assertEqual([int(v) for v in storage_values], expected_storage) + def test_constant_prop_pass_folds_without_a_zero_stride(self) -> None: + class ScaleByBuffer(nn.Module): + def __init__(self): + super().__init__() + self.register_buffer("scale", torch.tensor(2.0)) + + def forward(self, x): + # expand(1) of a 0-d tensor has stride (0,), yet PyTorch calls + # it contiguous. + return x * self.scale.expand(1) + + exported = constant_prop_pass( + export(ScaleByBuffer(), (torch.randn(4),), strict=True) + ) + self.assertEqual([t.stride() for t in exported.constants.values()], [(1,)]) + + def test_emit_user_constants_with_zero_stride(self) -> None: + def zero_stride(value): + # expand(1) of a 0-d tensor has stride (0,), yet PyTorch calls it + # contiguous. + return torch.tensor(value).expand(1) + + class AddConstants(nn.Module): + def __init__(self): + super().__init__() + self.weight = nn.Parameter(zero_stride(1.0), requires_grad=False) + self.register_buffer("bias", zero_stride(2.0)) + self.register_buffer("offset", zero_stride(3.0), persistent=False) + self.shift = zero_stride(4.0) + # Channels-last, with a zero stride on the size-1 batch dim. + self.register_buffer( + "image", + torch.randn(1, 3, 4, 4).as_strided((1, 3, 4, 4), (0, 1, 12, 3)), + ) + + def forward(self, x): + constants = self.weight + self.bias + self.offset + self.shift + return x + constants + self.image.contiguous() + + model = AddConstants() + x = torch.randn(4) + with self.assertNoLogs(convert_constant_dim_order_pass.logger): + edge = to_edge( + export(model, (x,), strict=True), + constant_methods={"get_scale": zero_stride(5.0)}, + ) + stored_image = edge.exported_program().state_dict["image"] + self.assertEqual(stored_image.stride(), (48, 1, 12, 3)) + program = edge.to_executorch() + + et_module = _load_for_executorch_from_buffer(program.buffer) + self.assertTrue(torch.allclose(model(x), et_module((x,))[0])) + self.assertEqual(et_module.run_method("get_scale", [])[0].tolist(), [5.0]) + + def test_emit_user_input_with_zero_stride(self) -> None: + class Double(nn.Module): + def forward(self, x): + return x * 2 + + # A zero stride on a size-1 dim, as in an expanded example input. PyTorch + # still calls it contiguous. + x = torch.randn(1, 4).as_strided((1, 4), (0, 1)) + program = to_edge(export(Double(), (x,), strict=True)).to_executorch() + + et_module = _load_for_executorch_from_buffer(program.buffer) + self.assertTrue(torch.allclose(x * 2, et_module((x,))[0])) + + def test_emit_constants_with_size_one_dims(self) -> None: + def channels_last(*sizes): + return torch.randn(*sizes).to(memory_format=torch.channels_last) + + class Model(nn.Module): + def __init__(self): + super().__init__() + # PyTorch ignores the stride of a size-1 dim, so it calls + # queries contiguous and frame channels-last, but their strides + # give other dim orders. image already has a supported one. + self.register_buffer("queries", torch.randn(1, 8, 4).transpose(0, 1)) + self.register_buffer("image", channels_last(2, 1, 4, 4)) + self.register_buffer( + "frame", torch.randn(48).as_strided((1, 3, 4, 4), (5, 1, 12, 3)) + ) + + def forward(self, keys, x, y): + return torch.bmm(self.queries, keys), x + self.image, y + self.frame + + model = Model() + inputs = ( + torch.randn(8, 4, 8), + channels_last(2, 1, 4, 4), + channels_last(1, 3, 4, 4), + ) + program = to_edge(export(model, inputs, strict=True)).to_executorch() + + et_module = _load_for_executorch_from_buffer(program.buffer) + for expected, actual in zip(model(*inputs), et_module(inputs)): + self.assertTrue(torch.allclose(expected, actual)) + def test_emit_device_info_propagated_to_serialized_tensor(self) -> None: """Verify that device info from PropagateDevicePass flows through the emitter into ExtraTensorInfo.device_type on serialized tensors.""" diff --git a/exir/passes/BUCK b/exir/passes/BUCK index 9deed4abcea..ef9144279ce 100644 --- a/exir/passes/BUCK +++ b/exir/passes/BUCK @@ -176,6 +176,7 @@ fbcode_target(_kind = runtime.python_library, ], deps = [ "//caffe2:torch", + "//executorch/exir:tensor", "//executorch/exir/dialects:lib", "//executorch/exir/dialects/edge:lib", "//executorch/exir/operator:util", @@ -506,6 +507,7 @@ fbcode_target(_kind = runtime.python_library, ], deps = [ "//caffe2:torch", + "//executorch/exir:tensor", ], ) diff --git a/exir/passes/constant_prop_pass.py b/exir/passes/constant_prop_pass.py index 44b0121268c..769a9a9c2c1 100644 --- a/exir/passes/constant_prop_pass.py +++ b/exir/passes/constant_prop_pass.py @@ -20,6 +20,7 @@ _schema_paired_args, ) from executorch.exir.passes.replace_aten_with_edge_pass import aten_to_edge +from executorch.exir.tensor import without_zero_strides from torch._export.utils import ( get_buffer, get_lifted_tensor_constant, @@ -288,19 +289,13 @@ def get_propagated_const_tensor_dict( # Execute the `node.target` and create a new propagated constant tensor. prop_constant_tensor = node.target(*args_data, **kwargs_data) - # ExecuTorch doesn't support zero strides, so we need to ensure the tensor is contiguous - # if it has any zero strides from broadcasting/expansion operations - if ( - isinstance(prop_constant_tensor, torch.Tensor) - and 0 in prop_constant_tensor.stride() - ): - prop_constant_tensor = prop_constant_tensor.contiguous() - # A folded constant must be a plain tensor that does not require # grad. no_grad is not enough when the op returns a view of a # parameter or the parameter itself. if isinstance(prop_constant_tensor, torch.Tensor): - prop_constant_tensor = prop_constant_tensor.detach() + prop_constant_tensor = without_zero_strides( + prop_constant_tensor + ).detach() const_node_to_tensor[node] = prop_constant_tensor # Only tensors can be lifted into placeholders. A non-tensor value read by diff --git a/exir/passes/convert_constant_dim_order_pass.py b/exir/passes/convert_constant_dim_order_pass.py index 36037961f62..8e1ac204b67 100644 --- a/exir/passes/convert_constant_dim_order_pass.py +++ b/exir/passes/convert_constant_dim_order_pass.py @@ -6,6 +6,7 @@ import logging import torch +from executorch.exir.tensor import dim_order_from_stride, without_zero_strides from torch.export import ExportedProgram from torch.export.graph_signature import InputKind @@ -22,6 +23,29 @@ def _should_transform(tensor: torch.Tensor) -> bool: ) +def _normalize(tensor: torch.Tensor) -> torch.Tensor: + """ + Returns the tensor with default or channels-last dim order and no zero + strides, copying it only if needed. + """ + if _should_transform(tensor): + return tensor.contiguous() + tensor = without_zero_strides(tensor) + ndim = tensor.dim() + dim_order = dim_order_from_stride(tensor.stride(), tuple(tensor.shape)) + if dim_order == tuple(range(ndim)) or ( + ndim in (4, 5) and dim_order == (0, *range(2, ndim), 1) + ): + return tensor + # PyTorch ignores the stride of a size-1 dim when it checks contiguity, so + # sizes (8, 1, 4) with strides (4, 32, 1) are contiguous, yet give dim + # order (1, 0, 2). + memory_format = ( + torch.contiguous_format if tensor.is_contiguous() else torch.channels_last + ) + return tensor.clone(memory_format=memory_format) + + def _update_placeholder_meta( exported_program: ExportedProgram, target: str, @@ -49,8 +73,11 @@ def _update_placeholder_meta( if placeholder_node is None: raise RuntimeError(f"Missing placeholder for {input_spec.arg.name}.") - strides = placeholder_node.meta["val"].stride() - placeholder_node.meta["val"] = placeholder_node.meta["val"].contiguous() + val = placeholder_node.meta["val"] + placeholder_node.meta["val"] = _normalize(val) + # Fixing a stride that PyTorch ignores does not change the layout. + if not _should_transform(val): + return logger.warning( "%s '%s' has strides %s, which are neither contiguous nor channels-last. " "to_edge stores it contiguous, so partitioners and delegates see the " @@ -58,7 +85,7 @@ def _update_placeholder_meta( "where it is created to avoid this change.", kind.name.replace("_", " ").capitalize(), target, - tuple(strides), + tuple(val.stride()), ) @@ -69,8 +96,11 @@ def convert_constant_dim_order_pass( Normalize lifted tensors to default or channels-last dim order. Tensor constants, buffers and parameters with other dim orders or striding - are converted to contiguous tensors. This pass acts in-place on the unlifted - exported program. + are converted to contiguous tensors. Those that PyTorch calls contiguous or + channels-last, but whose strides on size-1 dims give another dim order, are + copied into that layout, and zero strides are removed. User inputs with a + zero stride become contiguous, or stay channels-last; other inputs are not + changed. This pass acts in-place on the unlifted exported program. Args: exported_program: The ExportedProgram to transform. @@ -84,8 +114,11 @@ def convert_constant_dim_order_pass( ) for key, const in exported_program.constants.items(): - if isinstance(const, torch.Tensor) and _should_transform(const): - exported_program.constants[key] = const.contiguous() + if not isinstance(const, torch.Tensor): + continue + normalized = _normalize(const) + if normalized is not const: + exported_program.constants[key] = normalized # Also update the corresponding placeholder node meta value. This doesn't # get automatically updated during retracing as ExportPass uses the placeholder @@ -99,22 +132,28 @@ def convert_constant_dim_order_pass( _update_placeholder_meta(exported_program, key, kind) for key, param in exported_program.named_parameters(): - if _should_transform(param): + normalized = _normalize(param) + if normalized is not param: exported_program.state_dict[key] = torch.nn.Parameter( - param.contiguous(), requires_grad=param.requires_grad + normalized, requires_grad=param.requires_grad ) _update_placeholder_meta(exported_program, key, InputKind.PARAMETER) # Convert buffers. for key, buffer in exported_program.named_buffers(): - if ( - key not in non_persistent_buffer_names - and isinstance(buffer, torch.Tensor) - and _should_transform(buffer) - ): + if key in non_persistent_buffer_names or not isinstance(buffer, torch.Tensor): + continue + normalized = _normalize(buffer) + if normalized is not buffer: # Persistent buffers are stored in the state dict. - exported_program.state_dict[key] = buffer.contiguous() + exported_program.state_dict[key] = normalized _update_placeholder_meta(exported_program, key, InputKind.BUFFER) + # An expanded example input has a zero stride, which ExecuTorch cannot + # store either. Constants are already normalized, so this only changes inputs. + for node in exported_program.graph.nodes: + if node.op == "placeholder" and isinstance(node.meta.get("val"), torch.Tensor): + node.meta["val"] = without_zero_strides(node.meta["val"]) + return exported_program diff --git a/exir/program/_program.py b/exir/program/_program.py index 442a2658fdf..7394e17151b 100644 --- a/exir/program/_program.py +++ b/exir/program/_program.py @@ -826,8 +826,8 @@ def _generate_edge_program( program = remove_unused_parameters_pass(program) if not config._skip_dim_order: - # Normalize constants, buffers and parameters before partitioning, so a - # delegate sees the layout the program stores, and before + # Normalize constants, buffers, parameters and inputs before + # partitioning, so a delegate sees the layout the program stores, and before # MemoryFormatOpsPass rewrites preserve-format copies, which cannot # represent arbitrary dim orders. program = convert_constant_dim_order_pass.convert_constant_dim_order_pass( @@ -884,8 +884,8 @@ def _generate_edge_program( # Normalize tensor constants introduced by the preceding passes. With # _skip_dim_order this is the only call, so it fixes the layout of - # parameters, buffers and constants, but ops computed from them keep the - # old strides. + # parameters, buffers, constants and inputs, but ops computed from them + # keep the old strides. edge_program = convert_constant_dim_order_pass.convert_constant_dim_order_pass( edge_program ) diff --git a/exir/tensor.py b/exir/tensor.py index 50fcd46fa53..f44ca6d9070 100644 --- a/exir/tensor.py +++ b/exir/tensor.py @@ -118,6 +118,25 @@ def __lt__(self, other): return tuple(typing.cast(Tuple[bytes], sorted_dims)) +def without_zero_strides(tensor: torch.Tensor) -> torch.Tensor: + """ + Returns the tensor, or a copy of it if it has a zero stride, which + ExecuTorch cannot store. The copy is channels-last if the tensor is, and + contiguous otherwise. + + Tensor.contiguous() is not enough: PyTorch ignores the stride of a size-1 + dim, and every stride of an empty tensor, when it checks contiguity, so it + can return a tensor with a zero stride unchanged. + """ + if 0 not in tensor.stride(): + return tensor + if not tensor.is_contiguous() and tensor.is_contiguous( + memory_format=torch.channels_last + ): + return tensor.clone(memory_format=torch.channels_last) + return tensor.clone(memory_format=torch.contiguous_format) + + def stride_from_dim_order(sizes: List[int], dim_order: List[int]) -> List[int]: """ Converts dim order to stride using sizes @@ -210,7 +229,7 @@ def nbytes(self) -> int: def from_tensor(cls, tensor: torch.Tensor, const: bool = False) -> TensorSpec: if const: # for non-contigous tensors, convert to a contiguous one - tensor = tensor.contiguous() + tensor = without_zero_strides(tensor.contiguous()) # Weights cannot be views during emission or serialization if tensor.nbytes != tensor.untyped_storage().nbytes(): tensor = tensor.clone() diff --git a/exir/tests/test_tensor.py b/exir/tests/test_tensor.py index d6307d716e1..1ce281cb455 100644 --- a/exir/tests/test_tensor.py +++ b/exir/tests/test_tensor.py @@ -24,6 +24,7 @@ scalar_type_enum, stride_from_dim_order, TensorSpec, + without_zero_strides, ) @@ -384,6 +385,23 @@ def test_dim_order_from_stride_with_symbolic_sizes(self) -> None: ) self.assertEqual(shape_env.guards, []) + def test_without_zero_strides(self) -> None: + dense = torch.arange(4.0) + self.assertIs(without_zero_strides(dense), dense) + + for tensor, stride in [ + (torch.tensor(2.0).expand(1), (1,)), + (torch.zeros(3).expand(0, 3), (3, 1)), + ( + torch.randn(1, 3, 4, 4).as_strided((1, 3, 4, 4), (0, 1, 12, 3)), + (48, 1, 12, 3), + ), + ]: + self.assertIn(0, tensor.stride()) + copy = without_zero_strides(tensor) + self.assertEqual(copy.stride(), stride) + self.assertTrue(torch.equal(copy, tensor)) + def test_strides_from_dim_order(self) -> None: sizes = [] dim_order = [] diff --git a/pytest-windows.ini b/pytest-windows.ini index 26a0ee6dabc..92c580d781e 100644 --- a/pytest-windows.ini +++ b/pytest-windows.ini @@ -106,6 +106,7 @@ addopts = --deselect=exir/backend/test/test_lowered_backend_module.py::TestBackendAPI::test_shared_delegate_matches_separate_calls_at_runtime --deselect=exir/backend/test/test_lowered_backend_module.py::TestBackendAPI::test_runtime_rejects_a_shared_delegate_with_scratch --deselect=exir/backend/test/test_to_backend_multi_method.py::TestToBackendMultiMethod::test_multi_method_end_to_end + --deselect=exir/backend/test/test_utils.py::TestUtils::test_to_backend_keeps_a_mutated_buffer_out_of_a_partial_delegate --deselect=extension/llm/custom_ops/test_sdpa_with_kv_cache.py::SDPATestForSpeculativeDecode::test_sdpa_with_cache_seq_len_130 --deselect=devtools/inspector/tests/inspector_test.py::TestInspector::test_etrecord_populates_correct_edge_dialect_aot_intermediate_outputs --deselect=devtools/inspector/tests/inspector_test.py::TestInspector::test_etrecord_populates_correct_export_program_aot_intermediate_outputs