Skip to content

Add MaxPool1D decomposition pass support (#17022) - #21446

Open
Ninja91 wants to merge 1 commit into
mainfrom
export-D91760459
Open

Add MaxPool1D decomposition pass support (#17022)#21446
Ninja91 wants to merge 1 commit into
mainfrom
export-D91760459

Conversation

@Ninja91

@Ninja91 Ninja91 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary:

Implement DecomposeMaxPool1dPass to enable MaxPool1D support on ARM backend
by decomposing max_pool1d into unsqueeze_copy → max_pool2d → squeeze_copy.

Implementation Strategy

Decomposition Approach (Optimal for TOSA/Vela)

The pass decomposes max_pool1d into max_pool2d via unsqueeze_copy/squeeze_copy
operations:

  1. unsqueeze_copy(dim=2): (N, C, L) → (N, C, 1, L) - add height dimension
  2. max_pool2d: with adapted params [k]→[1,k], [s]→[1,s], [p]→[0,p], [d]→[1,d]
  3. squeeze_copy(dims=[2]): (N, C, 1, L_out) → (N, C, L_out) - remove height dimension

Why This Approach is Optimal

  1. unsqueeze_copy and squeeze_copy map to TOSA RESHAPE which is zero-cost in Vela:

    • Classified as memory_only_ops (Reshape, Squeeze, ExpandDims, Identity)
    • Bypassed entirely when conditions met (NPU-produced, single consumer)
    • Tensor equivalence enables memory aliasing (same address)
  2. TFA Pipeline Placement (before quantization):

    • unsqueeze_copy.default is in _one_to_one_shared_input_qspec
    • squeeze_copy.dims is added to _one_to_one_shared_input_qspec
    • max_pool2d is in _one_to_one_shared_input_or_input_act_qspec
    • All get proper SharedQuantizationSpec from the annotator automatically
  3. Quantization Handling:

    • Clear qparams on intermediate unsqueeze_copy and squeeze_copy ops (let annotator fill them)
    • Preserve original meta on max_pool2d for proper tracing
    • MAX_POOL2D doesn't need zero-point handling (unlike AVG_POOL2D)

TOSA/Vela Constraints Validated

  • U55: Stride ≤3 ✓, Kernel ≤256x256 ✓
  • U85: Extended stride support via accumulator save/restore
  • Dilation: Handled by separate DecomposeMaxPool2dPass if needed

Reviewed By: 3l1

Differential Revision: D91760459

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani

Summary:

Implement DecomposeMaxPool1dPass to enable MaxPool1D support on ARM backend
by decomposing max_pool1d into unsqueeze_copy → max_pool2d → squeeze_copy.

## Implementation Strategy

### Decomposition Approach (Optimal for TOSA/Vela)
The pass decomposes max_pool1d into max_pool2d via unsqueeze_copy/squeeze_copy
operations:
1. unsqueeze_copy(dim=2): (N, C, L) → (N, C, 1, L) - add height dimension
2. max_pool2d: with adapted params [k]→[1,k], [s]→[1,s], [p]→[0,p], [d]→[1,d]
3. squeeze_copy(dims=[2]): (N, C, 1, L_out) → (N, C, L_out) - remove height dimension

### Why This Approach is Optimal

1. **unsqueeze_copy and squeeze_copy map to TOSA RESHAPE** which is zero-cost in Vela:
   - Classified as memory_only_ops (Reshape, Squeeze, ExpandDims, Identity)
   - Bypassed entirely when conditions met (NPU-produced, single consumer)
   - Tensor equivalence enables memory aliasing (same address)

2. **TFA Pipeline Placement (before quantization)**:
   - unsqueeze_copy.default is in _one_to_one_shared_input_qspec
   - squeeze_copy.dims is added to _one_to_one_shared_input_qspec
   - max_pool2d is in _one_to_one_shared_input_or_input_act_qspec
   - All get proper SharedQuantizationSpec from the annotator automatically

3. **Quantization Handling**:
   - Clear qparams on intermediate unsqueeze_copy and squeeze_copy ops (let annotator fill them)
   - Preserve original meta on max_pool2d for proper tracing
   - MAX_POOL2D doesn't need zero-point handling (unlike AVG_POOL2D)

### TOSA/Vela Constraints Validated
- U55: Stride ≤3 ✓, Kernel ≤256x256 ✓
- U85: Extended stride support via accumulator save/restore
- Dilation: Handled by separate DecomposeMaxPool2dPass if needed

Reviewed By: 3l1

Differential Revision: D91760459
Copilot AI review requested due to automatic review settings July 28, 2026 22:30
@Ninja91
Ninja91 requested a review from digantdesai as a code owner July 28, 2026 22:30
@pytorch-bot

pytorch-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown

🔗 Helpful Links

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

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

❌ 2 New Failures, 2 Unrelated Failures

As of commit 501e2ea with merge base 32f56be (image):

NEW FAILURES - The following jobs have failed:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

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 28, 2026
@meta-codesync

meta-codesync Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

@Ninja91 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D91760459.

@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

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

This PR adds Arm-backend support for aten.max_pool1d by introducing a decomposition pass that rewrites MaxPool1D into an equivalent MaxPool2D sequence (unsqueeze_copy → max_pool2d → squeeze_copy) and wires it into the Arm pass pipelines and quantization annotator so the resulting intermediate ops are properly annotated.

Changes:

  • Add DecomposeMaxPool1dPass to rewrite aten.max_pool1d into 2D pooling plus view-like reshapes.
  • Register the new pass in the Arm pass manager and _passes package exports.
  • Update Arm quantization annotation to treat aten.squeeze_copy.dims as a shared-input qspec op; enable the previously xfailed TOSA INT MaxPool1D test.

Reviewed changes

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

Show a summary per file
File Description
backends/arm/test/ops/test_max_pool1d.py Un-xfails the TOSA INT MaxPool1D decomposition test.
backends/arm/quantizer/quantization_annotator.py Adds squeeze_copy.dims to shared-input qspec ops to support the new decomposition intermediates.
backends/arm/_passes/decompose_max_pool1d_pass.py Introduces the MaxPool1D→MaxPool2D decomposition pass.
backends/arm/_passes/arm_pass_manager.py Inserts the new decomposition pass into relevant Arm pipelines.
backends/arm/_passes/init.py Exports DecomposeMaxPool1dPass from the passes package.
Comments suppressed due to low confidence (1)

backends/arm/_passes/decompose_max_pool1d_pass.py:54

  • This decomposition assumes the max_pool1d input is 3D (N, C, L). However, max_pool1d can also be called with 2D inputs (C, L); in that case unsqueeze_copy(dim=2) produces a rank-3 tensor and max_pool2d will fail due to incorrect rank. Consider guarding on input rank (or explicitly handling 2D) before applying the rewrite.
        # Extract and normalize arguments
        x = args[0]
        kernel_size = _normalize_to_list(args[1])
        stride = _normalize_to_list(

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +15 to +26
def _normalize_to_list(
value: Optional[Union[int, List[int], tuple]],
default: Optional[List[int]] = None,
) -> List[int]:
"""Normalize parameter to list: handle None, int, tuple, list."""
if value is None:
if default is None:
raise ValueError("Value cannot be None without a default")
return default
if isinstance(value, int):
return [value]
return list(value)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. meta-exported module: arm Issues related to arm backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants