Skip to content

NXP backend: added support for aten.pad with mode reflect - #21515

Open
novak-vaclav wants to merge 1 commit into
pytorch:mainfrom
nxp-upstream:feature/EIEX-888-add-pad-support
Open

NXP backend: added support for aten.pad with mode reflect#21515
novak-vaclav wants to merge 1 commit into
pytorch:mainfrom
nxp-upstream:feature/EIEX-888-add-pad-support

Conversation

@novak-vaclav

Copy link
Copy Markdown
Collaborator

Summary

Added support for aten.pad with mode reflect using new Neutron MLIR flow.

Test plan

tests can be manually run using pytest -c /dev/null backends/nxp/tests/

cc @robert-kalmar @JakeStevens @digantdesai @rascani @MartinPavella

Copilot AI lite review requested due to automatic review settings July 31, 2026 08:29
@novak-vaclav novak-vaclav added module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ release notes: nxp Changes to the NXP Neutron backend delegate labels Jul 31, 2026
@pytorch-bot

pytorch-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21515

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 New Failure, 9 Pending

As of commit 8c50a26 with merge base 4b4df96 (image):

NEW FAILURE - The following job has failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 31, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 31, 2026

Copy link
Copy Markdown

CLA Not Signed

@novak-vaclav
novak-vaclav force-pushed the feature/EIEX-888-add-pad-support branch from 9b9736c to 97731d1 Compare July 31, 2026 09:44
Copilot AI review requested due to automatic review settings July 31, 2026 09:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@novak-vaclav

Copy link
Copy Markdown
Collaborator Author

Fixed minor issues I found out about after creating the PR.

@novak-vaclav
novak-vaclav force-pushed the feature/EIEX-888-add-pad-support branch from 97731d1 to 76b13f3 Compare July 31, 2026 09:52
@novak-vaclav

Copy link
Copy Markdown
Collaborator Author

And rebased onto current main to run the tests on Neutron Software 3.2.0

Comment thread backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py Outdated
Comment thread backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py Outdated
Comment thread backends/nxp/neutron_partitioner.py Outdated
Comment thread backends/nxp/tests/ir/converter/node_converter/test_constant_pad_nd_converter.py Outdated
Comment thread backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py Outdated
Copilot AI review requested due to automatic review settings July 31, 2026 16:11
@novak-vaclav
novak-vaclav force-pushed the feature/EIEX-888-add-pad-support branch from 76b13f3 to fc14b2c Compare July 31, 2026 16:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings July 31, 2026 16:19
@novak-vaclav
novak-vaclav force-pushed the feature/EIEX-888-add-pad-support branch from fc14b2c to bc44109 Compare July 31, 2026 16:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@novak-vaclav
novak-vaclav force-pushed the feature/EIEX-888-add-pad-support branch from bc44109 to 6bf8da0 Compare July 31, 2026 16:26
Copilot AI review requested due to automatic review settings July 31, 2026 16:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@novak-vaclav

Copy link
Copy Markdown
Collaborator Author

Resolved comments from code review, resolved conflicts with main and rebased.

@roman-janik-nxp roman-janik-nxp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just move the comment, otherwise GJ!

Comment thread backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py Outdated
Copilot AI review requested due to automatic review settings August 3, 2026 15:32
@novak-vaclav
novak-vaclav force-pushed the feature/EIEX-888-add-pad-support branch from 6bf8da0 to f5b65ca Compare August 3, 2026 15:32
@novak-vaclav

Copy link
Copy Markdown
Collaborator Author

Fixed all found issues in code review, rebased onto main and when tests pass, I will merge.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py:33

  • PadConverter._get_mode() only looks at positional args. If aten.pad is emitted with keyword arguments (common in FX graphs), mode may be present in node.kwargs and this will incorrectly default to "constant", causing reflect padding to be treated as unsupported and decomposed.
    @staticmethod
    def _get_mode(node: Node) -> str:
        return node.args[2] if len(node.args) > 2 else "constant"

backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py:87

  • _convert_paddings_to_tflite() is annotated to return list[int], but it actually returns a list of 2-element pairs (rank x 2). Also, paddings_reversed becomes a list of NumPy row arrays, which then gets concatenated with Python lists; converting to plain list[list[int]] early avoids mixed element types and matches what MirrorPad expects.
    @staticmethod
    def _convert_paddings_to_tflite(
        paddings: Collection[int], input_tensor: tflite_model.Tensor
    ) -> list[int]:
        # Group `padding` by two elements per list.

backends/nxp/neutron_partitioner.py:559

  • Docstring has a mismatched bracket in the return-type description (Optional[Callable[[torch.fx.Node], bool]]]:). This makes the rendered/parsed type unclear.
        Returns:
            List[torch._ops.OpOverload]: a list of ops that should not be decomposed.
            Optional[Callable[[torch.fx.Node], bool]]]: an optional filter, called for each node in the
            graph, that lets a node be decomposed even though its op is in the list above. A node is kept
            (not decomposed) only if the filter returns True for it; if it returns False, the node is decomposed.

Comment thread backends/nxp/tests/models.py
Copilot AI review requested due to automatic review settings August 4, 2026 08:39
@novak-vaclav
novak-vaclav force-pushed the feature/EIEX-888-add-pad-support branch from f5b65ca to 8c50a26 Compare August 4, 2026 08:39
@novak-vaclav

Copy link
Copy Markdown
Collaborator Author

Github CI seemed to fail for some unrelated reason, so I rebased onto main and will wait till tests pass.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (4)

backends/nxp/tests/models.py:370

  • PadModule (and similarly PadConvModule) annotates value as float | None, but callers/tests may pass an int (and the previous API allowed float | int | None). Consider widening the annotation to float | int | None to match actual usage and avoid type-checker friction.
class PadModule(torch.nn.Module):
    def __init__(
        self,
        paddings: Collection[int],
        mode: str = "constant",
        value: float | None = None,
    ):

backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py:32

  • aten.pad mode can be provided via keyword arguments in FX (node.kwargs.get('mode')), not only positionally. With the current implementation, a node like pad(x, pad, mode='reflect') captured in kwargs could be misclassified as constant, making the converter incorrectly report unsupported and causing decomposition instead of delegation. Update _get_mode to check node.kwargs first (and fall back to positional/default).
    def _get_mode(node: Node) -> str:
        return node.args[2] if len(node.args) > 2 else "constant"

backends/nxp/backend/ir/converter/node_converters/ops_converters/pad_converter.py:78

  • Two issues here: (1) the return annotation says list[int], but the function returns a 2D structure (list[list[int]] / array of pairs), which is what MirrorPad expects. (2) this code will raise (or produce invalid padding) when len(paddings) is odd or when len(paddings)/2 > input_tensor.rank (negative complement). It would be safer to validate these conditions in _is_supported_in_IR (return False) or raise a clearer error before hitting NumPy reshape / negative rank math.
    def _convert_paddings_to_tflite(
        paddings: Collection[int], input_tensor: tflite_model.Tensor
    ) -> list[int]:
        # Group `padding` by two elements per list.
        paddings_grouped = np.array(paddings).reshape(-1, 2)

        # In TFLite, `padding` order is reversed.
        paddings_reversed = list(reversed(paddings_grouped))

        # Add complementary zero pairs to `padding` to match input tensor rank.
        zero_pair_compl = [[0, 0]] * (input_tensor.rank - len(paddings_reversed))
        padding_tfl = zero_pair_compl + paddings_reversed

backends/nxp/neutron_partitioner.py:560

  • Docstring type markup has a bracket mismatch: Optional[Callable[[torch.fx.Node], bool]]]: has an extra ]. This makes the docstring harder to read and can confuse generated docs; please fix the bracket structure.
    ) -> tuple[list[torch._ops.OpOverload], Callable[[torch.fx.Node], bool] | None]:
        """
        Method to determine which operators SHOULD NOT be decomposed to simpler edge ops.

        Returns:
            List[torch._ops.OpOverload]: a list of ops that should not be decomposed.
            Optional[Callable[[torch.fx.Node], bool]]]: an optional filter, called for each node in the
            graph, that lets a node be decomposed even though its op is in the list above. A node is kept
            (not decomposed) only if the filter returns True for it; if it returns False, the node is decomposed.
        """

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: nxp Issues related to NXP Neutron NPU delegation and code under backends/nxp/ release notes: nxp Changes to the NXP Neutron backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants