Fix quantized Conv1d same padding with even kernels in XNNPACK

[SakshamKapoor2911]

Fixes #20558.
Related to #20553.

Summary

Quantized nn.Conv1d(..., padding="same") with an even kernel exports with
asymmetric padding (unequal left/right amounts), which cannot be folded into
the convolution's symmetric padding field. The original pad-folding approach
in this PR was replaced with the explicit-PAD approach from #20553:

• InsertPadQDQPass — inserts implicit quantize/dequantize pairs after
  constant_pad_nd nodes in quantized contexts so they serialize as quantized
  static pads. Refactored with additional guard checks (pad_value, pad_amounts,
  negative amounts) and correct idempotency.
• ConvolutionConfig._get_act_deps — pulls zero-valued constant_pad_nd nodes
  (and their QDQ chain if InsertPadQDQPass already ran) into the convolution's
  partition for both 1D and 2D convs. The merged method replaces separate 1D
  and 2D implementations that existed on this branch.
• Regression test — quantized Conv1d even-kernel same-padding covering both
  symmetric and asymmetric pad cases, validated by numerical comparison.
• Removed unrelated init.py re-ordering and no-op conv1d_unsqueeze_pass
  changes.

With these changes, an even-kernel padding="same" conv1d graph:

dequant -> constant_pad_nd -> convolution

becomes (after XNNPACK preprocessing and partitioning):

[dequant -> pad -> q -> dq -> conv]  (single XNNPACK delegate)

Test plan

python -m pytest backends/xnnpack/test/ops/test_conv1d.py -q
# 7 passed (1 new regression test with 4 subTests)

python -m pytest backends/xnnpack/test/ops/test_conv2d.py -q
# 32 passed

python -m pytest backends/xnnpack/test/passes/test_insert_pad_qdq.py -q
# 3 passed

python -m pytest backends/xnnpack/test/ops/test_static_constant_pad.py -q
# 8 passed

lintrunner -a
# No lint issues

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

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❌ 1 Cancelled Job

As of commit e0975ed with merge base a6aaeff ():

CANCELLED JOB - The following job was cancelled. Please retry:

• trunk / test-models-macos-cpu (efficient_sam, portable) / macos-job (gh)
  ##[error]The operation was canceled.

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

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: SakshamKapoor2911 / name: SakshamKapoor2911 (e7636f5)

[Copilot]

Pull request overview

This PR fixes an XNNPACK delegation/runtime correctness issue for statically-quantized nn.Conv1d(padding="same") with even kernel sizes by folding the explicit temporal constant_pad_nd into asymmetric Conv2d padding during Conv1d→Conv2d lowering, and ensuring the serialized graph’s metadata matches the folded asymmetric padding.

Changes:

• Extend XNNPACK partitioning and Conv1d unsqueeze lowering to absorb temporal-only zero constant_pad_nd into the quantized Conv1d partition and fold it into asymmetric xnnpack_input_padding.
• Teach Conv2d serialization to use folded asymmetric padding metadata when present, and add a late pass to repair Conv1d folded-pad tensor metadata after retracing passes.
• Add a regression test covering quantized Conv1d even-kernel padding="same".

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/xnnpack/test/ops/test_conv1d.py	Adds a regression test for quantized Conv1d even-kernel padding="same" and factors out calibration sample generation.
backends/xnnpack/partition/config/gemm_configs.py	Extends convolution activation-dependency discovery to include temporal-only zero constant_pad_nd for quantized Conv1d so it can be partitioned/delegated.
backends/xnnpack/operators/op_conv2d.py	Uses folded asymmetric xnnpack_input_padding metadata (top/right/bottom/left) when present instead of symmetric ATen padding args.
backends/xnnpack/_passes/conv1d_unsqueeze_pass.py	Folds eligible temporal-only constant_pad_nd into asymmetric input padding during Conv1d→Conv2d lowering and adds a pass to restore correct tensor metadata for folded-pad Conv1d.
backends/xnnpack/_passes/init.py	Registers the new folded-pad metadata repair pass in the default XNNPACK pass pipeline.

---

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

[pytorch-bot]

❌ 🤖 pytorchbot command failed:

Got EOF while in a quoted string```
Try `@pytorchbot --help` for more info.

[SakshamKapoor2911]

@pytorchbot label "release notes: xnnpack"

[Copilot]

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

[pytorch-bot]

Workflows were awaiting approval. CI has now been triggered for the ciflow labels on this PR.

[Copilot]

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

[Copilot]

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

[JakeStevens]

The PR for 2D has landed:

#20553

You can take the same approach here; have an explicit PAD op, which gets executed in XNNPACK. We can then later fold that XNNPACK pad in the delegated graph, this is a follow up to both of these PRs.

As is, this code will also fail on:

class M(torch.nn.Module):
    def __init__(self):
          super().__init__()
          self.conv = torch.nn.Conv1d(1, 8, kernel_size=4, padding="same")
      def forward(self, x):
          out = torch.reshape(x, (1, 1, 256))
          out = self.conv(out)
          return torch.flatten(out, 1)   # even kernel + this line => crash

[Copilot]

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

[SakshamKapoor2911]

Addressed the remaining review points:

Lint: lintrunner -a passes cleanly on all 5 changed files. No lint issues remain.

Gemm config _get_act_deps: The method extends the existing 2D-only pad absorption check from #20553 to also handle 1D convs and the QDQ-pair pattern (dequant -> pad -> q -> dq -> conv) that InsertPadQDQPass introduces. The fallthrough to super()._get_act_deps() is preserved at the end, so there is no duplication -- the method replaces the narrower 2D-only check with a version that covers both dimensions. Happy to refactor further if you have a specific structure in mind.

insert_pad_qdq.py: The changes are not purely additive because the pass needed to handle Conv1d's 1D constant_pad_nd and be idempotent (skip already-quantized pads). The core mechanism (insert QDQ after pad in a quantized context) is the same as the original.

[JakeStevens]

@claude please review

[claude]

Claude finished @JakeStevens's task in 7m 9s —— View job

---

Review — PR #20734: Fix quantized Conv1d same padding with even kernels

• Read changed files (insert_pad_qdq.py, gemm_configs.py, test_conv1d.py)
• Analyze correctness, idempotency, transposed-conv gating, edge cases
• Check the crash case Jake shared (even kernel + flatten)
• Post review feedback

Overall this is a clean adoption of the explicit-PAD approach from #20553 and reads much better than the earlier pad-folding version. The transposed-conv gating and idempotency concerns from earlier rounds are addressed. A few things worth a look before merge:

---

1. insert_pad_qdq.py — dropped the per-tensor guard (medium)

The version on main deliberately required the feeding dequant to be exactly dequantize_per_tensor.default:

if (dq.target != exir_ops.edge.quantized_decomposed.dequantize_per_tensor.default):
    continue

The new code relaxes this to is_dequant(pad_input), which also matches dequantize_per_tensor.tensor, dequantize_per_channel.default, and dequantize_per_channel_group.default (see _DQ_OPS in utils/quant_utils.py:26). It then does:

q_params = pad_input.args[1:]
q = graph.create_node(..., quantize_per_tensor.default, args=(node,) + q_params)

For a .tensor variant, scale/zp are tensor node args; for per-channel there's an extra axis arg and tensor scales/zps. Feeding those into a quantize_per_tensor.default (which expects scalar scale/zp) produces a malformed node. Activations feeding a conv are per-tensor in practice, so this is unlikely to fire — but the original guard was there on purpose. Suggest restoring the per-tensor check (is_per_tensor(pad_input) or the explicit .default target check) to keep it safe. Fix this →

2. gemm_configs.py — is the QDQ-chain branch reachable? (question / possible dead code)

The elif is_dequant(act_input) ... dequant -> pad -> q -> dq -> conv branch (lines 416–425) handles the pattern after InsertPadQDQPass has run. But partitioning (get_deps / _get_act_deps) runs during to_edge_transform_and_lower, whereas InsertPadQDQPass runs later inside XnnpackBackend.preprocess (xnnpack_preprocess.py:144, via XNNPACKPassManager). At partition time the graph is still dequant -> pad -> conv, so only the direct-pad branch (line 414) should ever match.

If that ordering is correct, the QDQ-chain branch is dead code (and untested — the new test only exercises the direct path). Can you confirm a flow where the pass runs before partitioning? If not, I'd drop the branch to keep the method tight; if there is one, worth a test that hits it.

3. Minor

• insert_pad_qdq.py:80 returns PassResult(graph_module, True) unconditionally even when no pad was modified. Matches the prior behavior, so not blocking, but modified is technically inaccurate on a no-op run.
• gemm_configs.py:443-447 — for constant_pad_nd, args[1] amounts always come in pairs, so len(pad_amounts) % 2 == 0 is always true. Harmless but redundant.
• gemm_configs.py:433-434 — the pad_value != 0.0 early return super()._get_act_deps(node, ...) duplicates the fallthrough at line 458; a plain fallthrough would read the same.

---

Things that look correct

• Transposed convs are gated out (not is_transpose) — the earlier Copilot concern and Jake's even-kernel+flatten crash case are handled by keeping the pad standalone with correct shape propagation.
• Idempotency: after one run the pad has an is_quant user, so the any(is_quant(user) ...) guard skips it on re-run. Good.
• Guards for negative pad amounts and non-zero pad value are sensible additions.
• Test coverage across odd/even kernels and dilation (2,1)/(3,1)/(4,1)/(4,2) with numerical comparison is solid.

I did not run the test suite in this review. The main item to resolve is #1 (safety) and a confirmation on #2 (reachability).
· branch sk/fix-xnnpack-conv1d-same-padding

[Copilot]

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

[SakshamKapoor2911]

Addressed all 3 items from Claude review:

1. Per-tensor guard restored (insert_pad_qdq.py)
Added is_per_tensor(pad_input) check alongside is_dequant(). This ensures only dequantize_per_tensor.default (per-tensor) activations get the implicit QDQ inserted — per-channel/per-token variants that produce different q_params shapes are excluded, matching the safety guard on main.

2. Removed unreachable QDQ-chain branch (gemm_configs.py)
Confirmed the InsertPadQDQPass runs during XnnpackBackend.preprocess (via XNNPACKPassManager at _passes/__init__.py:85), which is after partitioning. So at _get_act_deps call-graph time, the pattern is always dequant -> pad -> conv — the dequant -> pad -> q -> dq -> conv path is unreachable. Removed that branch.

3. Minor cleanups

• modified flag now accurately tracks whether any pad was actually modified
• Removed redundant len(pad_amounts) % 2 == 0 (always true for constant_pad_nd)
• Removed duplicate pad_value != 0.0 early return that duplicated the natural fallthrough

[Copilot]

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

[SakshamKapoor2911]

@JakeStevens All 3 items from the Claude review above are addressed and pushed. Could you take another look?

[JakeStevens]

These comments were removed but what they were explaining was not fully followed.

In the original, we are allowing only _per_tensor.default, and then using that in the create_node. So, everything matched. This is no longer the case in the PR, see below

[JakeStevens]

is_dequant now matches:

_DQ_OPS = {
    "dequantize_per_tensor.tensor",
    "dequantize_per_tensor.default",
    "dequantize_per_channel.default",
    "dequantize_per_channel_group.default",
    "dequantize_per_token.default",
    "dequantize_affine.default",
}

but then below, we only use the _per_tensor.default when rebuilding the graph.

[SakshamKapoor2911]

Addressed the review feedback: removed the redundant is_dequant check. is_per_tensor already verifies the node is a quant/dequant node with per_tensor in its target name, so the broader is_dequant (which now matches per_channel, per_token, etc.) was both redundant and confusing — it created a mismatch with the create_node targets below which always emit per_tensor.default variants.

- is_dequant(pad_input)
- and is_per_tensor(pad_input)
+ and is_per_tensor(pad_input)

@JakeStevens — ready for re-review when you get a chance.

[Copilot]

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

[JakeStevens]

I'm sorry if I was not clear; the issue is not the redundant quant check, the issue is that before we checked on the full signature, and then replaced with that exact signature. Now, we match on both;

    "dequantize_per_tensor.tensor",
    "dequantize_per_tensor.default",

but insert only the .default version.

So, you can have a _per_tensor.tensor quantized node and then we make the padding .default

Let us stick with just static/.default for now as that is what is covered by tests etc, we can land and if youd like follow up to see if .tensor can be supported too.

[Copilot]

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

[SakshamKapoor2911]

@JakeStevens Feedback addressed -- tightened the dequant check in InsertPadQDQPass back to exact signature matching.

• Replaced is_per_tensor(pad_input) with is_dequant(pad_input) and pad_input.target == dequantize_per_tensor.default
• This matches only dequantize_per_tensor.default (not .tensor), so the inserted quantize_per_tensor.default node always receives matching scalar-typed q_params
• The extra is_dequant gate is a fast-path guard, keeping the intent readable

The .tensor variant can be supported in a follow-up if needed -- for now, only the covered/tested .default path is matched. Ready for re-review when you get a chance.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review is ineligible. To be eligible to request a review, you need a paid Copilot license, or your organization must enable Copilot code review.