-
Notifications
You must be signed in to change notification settings - Fork 665
NXP backend: Resolve limitations of uncertain tensor formats #13942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d299bae
df3a601
d7558e9
904d36b
8255593
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
# Copyright (c) Meta Platforms, Inc. and affiliates. | ||
# Copyright 2025 NXP | ||
# All rights reserved. | ||
# | ||
# This source code is licensed under the BSD-style license found in the | ||
# LICENSE file in the root directory of this source tree. | ||
|
||
import torch | ||
|
||
from executorch.backends.nxp.backend.node_format_inference import ( | ||
NodeFormat, | ||
NXP_NODE_FORMAT, | ||
) | ||
from executorch.exir.dialects._ops import ops as exir_ops | ||
from executorch.exir.pass_base import ExportPass, PassResult | ||
|
||
|
||
class RemoveGetItemPass(ExportPass): | ||
""" | ||
This remove item is used to remove getitem operator for max_pool2d_with_indices.default operator, and replace it with a single operator, | ||
that extracts the first output. More specifically, we are only getting the first output from aten::maxpool2d operator. | ||
Before Pass: | ||
MaxPool2d ---> GetItem[max_values, max_indexes] | ||
After Pass: | ||
MaxPool2d -> max_values | ||
""" | ||
|
||
def call(self, graph_module: torch.fx.GraphModule): | ||
module = graph_module | ||
for node in module.graph.nodes: | ||
if node.op == "call_function": | ||
if ( | ||
node.target.__name__ == "aten.max_pool2d_with_indices.default" | ||
or node.target.__name__ == "aten.max.dim" | ||
): | ||
users = list(node.users.keys()) | ||
|
||
if len(users) != 1: | ||
if len(users) == 2 and node.target.__name__ == "aten.max.dim": | ||
# Two users is allowed for max.dim. For that case, | ||
# rather than removing the getitem node in this | ||
# pass, we handle the getitem nodes in the op's | ||
# visitor when serializing | ||
continue | ||
else: | ||
raise AssertionError( | ||
f"Invalid number of users for {node.target.__name__}: {len(users)}" | ||
) | ||
|
||
getitem_node = list(node.users.keys())[0] | ||
|
||
if getitem_node.target.__name__ != "getitem": | ||
raise AssertionError( | ||
f"Expected max node's user to be getitem, got {getitem_node.target.__name__}" | ||
) | ||
|
||
getitem_index = getitem_node.args[1] | ||
|
||
with module.graph.inserting_before(node): | ||
if ( | ||
node.target.__name__ | ||
== "aten.max_pool2d_with_indices.default" | ||
): | ||
if getitem_index != 0: | ||
raise AssertionError( | ||
f"Expected second argument of getitem node for {node.target.__name__} to be 0, got {getitem_index}. XNNPACK delegate currently only supports getting just the max values from the op but not getting the corresponding indices." | ||
) | ||
new_max_wd = module.graph.create_node( | ||
"call_function", | ||
exir_ops.edge.aten.max_pool2d.default, | ||
args=node.args, | ||
kwargs=node.kwargs, | ||
) | ||
|
||
else: | ||
if getitem_index != 0: | ||
raise AssertionError( | ||
f"Expected second argument of getitem node for {node.target.__name__} to be 0, got {getitem_index}. XNNPACK delegate currently only supports getting just the max values or getting both the max values and their corresponding indices from the op, but not getting the indices alone." | ||
) | ||
new_max_wd = module.graph.create_node( | ||
"call_function", | ||
exir_ops.edge.aten.amax.default, | ||
args=node.args, | ||
kwargs=node.kwargs, | ||
) | ||
|
||
# MODIFIED PART START | ||
# Make sure to preserve the inferred node format. | ||
new_max_wd.meta[NXP_NODE_FORMAT] = node.meta.get( | ||
NXP_NODE_FORMAT, NodeFormat.NONE | ||
) | ||
# MODIFIED PART END | ||
|
||
getitem_node.replace_all_uses_with(new_max_wd) | ||
|
||
module.graph.erase_node(getitem_node) | ||
module.graph.erase_node(node) | ||
|
||
graph_module.recompile() | ||
# Propagate metadata and retrace module | ||
graph_module = super().call(graph_module).graph_module | ||
|
||
return PassResult(graph_module, True) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ | |
|
||
logger = logging.getLogger(__name__) | ||
|
||
NXP_NODE_FORMAT = "nxp_node_format" # Key into the `meta` attribute of nodes, which is mapped to the inferred format. | ||
|
||
|
||
class NodeFormat(Enum): | ||
# Node's output in NCHW format | ||
|
@@ -43,8 +45,6 @@ class NodeFormatInference: | |
# are channels first but output is formatless). | ||
ops_that_can_change_tensor_format = {exir_ops.edge.aten.view_copy.default} | ||
|
||
_node_format_mapping: dict[Node, NodeFormat] | ||
|
||
_type_changed_during_last_run: bool | ||
|
||
# Mapping between Node and its ancestors (inputs) | ||
|
@@ -57,7 +57,6 @@ def __init__(self, edge_program: ExportedProgram): | |
self._edge_program = edge_program | ||
|
||
self._nodes = edge_program.graph.nodes | ||
self._node_format_mapping = {} | ||
self._node_inputs = { | ||
node: node.all_input_nodes for node in edge_program.graph.nodes | ||
} | ||
|
@@ -67,7 +66,7 @@ def __init__(self, edge_program: ExportedProgram): | |
|
||
self._type_changed_during_last_run = False | ||
|
||
def identify_node_formats(self) -> dict[Node, NodeFormat]: | ||
def identify_node_formats(self): | ||
self._type_changed_during_last_run = True | ||
|
||
# Re-run format inference until there are no changes | ||
|
@@ -77,7 +76,15 @@ def identify_node_formats(self) -> dict[Node, NodeFormat]: | |
for node in self._nodes: | ||
self._infer_format_of_nodes(node) | ||
|
||
return self._node_format_mapping | ||
for node in self._nodes: | ||
if self._get_node_op_type(node) is None: | ||
continue | ||
if not hasattr(node, "meta"): | ||
logging.warning(f"Node `{node}` does not have the `meta` attribute.") | ||
node.meta = {} | ||
if NXP_NODE_FORMAT not in node.meta: | ||
logging.warning(f"Node `{node}` does not have inferred format.") | ||
node.meta[NXP_NODE_FORMAT] = NodeFormat.NONE | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As we now perform the node format inference during partition (that is on the whole Edge Program), it is likely that some nodes wont have the format determined, as the NodeFormat inference algorithm does not know them. Right? We should make sure we stop the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you are correct, with a few caveats. I will update the code to not propagate the format through "unknown" operators. |
||
|
||
def _infer_format_of_nodes(self, node: Node): | ||
op_type = self._get_node_op_type(node) | ||
|
@@ -151,7 +158,7 @@ def _assign_format_to_node(self, node: Node, node_format: NodeFormat): | |
if old_node_format != node_format: | ||
self._type_changed_during_last_run = True | ||
|
||
self._node_format_mapping[node] = node_format | ||
node.meta[NXP_NODE_FORMAT] = node_format | ||
|
||
def _get_node_op_type(self, node: Node) -> str | None: | ||
""" | ||
|
@@ -252,8 +259,10 @@ def _node_produces_or_consumes_channels_first_format(self, node) -> bool: | |
for ancestor_node in input_nodes | ||
) | ||
|
||
def _get_node_format(self, node): | ||
return self._node_format_mapping.get(node, NodeFormat.NONE) | ||
def _get_node_format(self, node) -> NodeFormat: | ||
if not hasattr(node, "meta"): | ||
node.meta = {} | ||
return node.meta.get(NXP_NODE_FORMAT, NodeFormat.NONE) | ||
|
||
def _node_is_placeholder(self, node: Node): | ||
def _node_is_placeholder(self, node: Node) -> bool: | ||
return node.op == "placeholder" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we handle the aten.max.dim too? Is it a loftover from original pass?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it was in the original file. I wanted to make as few changes as possible, as it is not the main focus of this PR.