🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

✅ No Failures

As of commit 58a639a with merge base 2af5638 ():
💚 Looks good so far! There are no failures yet. 💚

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

@pytorchbot label "release notes: qualcomm"

@claude review this PR

Claude finished @psiddh's task in 3m 42s —— View job

Code Review: QNN diagonal support

• Read the decomposition pass and integration points
• Review tests and models
• Post review feedback

Nice, clean decomposition. The index math is correct and the pass is wired into all three pipeline registration points (get_capture_program_passes, get_annotation_passes, and the dependency map) consistently with the other Decompose* passes. A few things worth looking at:

Verified correct

• Flat-index math in _compute_diag_params: for offset ≥ 0, (i, i+offset) → flat i*(N+1)+offset, so start=offset, stride=N+1, size=min(M, N-offset) ✅. For offset < 0, (i+|offset|, i) → flat i*(N+1)+|offset|*N, so start=(-offset)*N ✅.
• Output shape: torch.diagonal drops dim1/dim2 and appends the diagonal as the last dim → remaining_dims + [diag_size], which matches the index_select result on the flattened last dim. So copy_meta(meta) (reusing the original node's val) on index_select_node is correct.

Questions / potential issues

1. view on a non-contiguous permuted tensor in the aten (annotation) path. _get_ops returns torch.ops.aten.view.default for the non-edge path, and step 2 applies it directly after aten.permute.default. A permute produces a non-contiguous view, and aten.view on a non-contiguous tensor raises view size is not compatible with input tensor's size and stride. The manually-computed view_val sidesteps this inside the pass (you call .contiguous().view(...)), but any later fake-tensor re-propagation over the rewritten graph could trip on it. The edge path is safe because it uses view_copy. Consider using torch.ops.aten.reshape.default for the aten path (or confirm the annotation graph is never re-propagated). Worth a quick check on a case where need_permute is true in the annotation pipeline (e.g. the dim1=1, dim2=2 test) if it runs through that path.

2. Empty-diagonal full node dtype. In the diag_size <= 0 branch you emit ops["full"](empty_shape, 0.0) with no dtype/device kwargs. The fill value 0.0 forces float; if the input is integer/quantized the empty output dtype won't match the original. Since it's a zero-element tensor the values are irrelevant, but the dtype mismatch could confuse downstream shape/dtype checks. Passing dtype=input_val.dtype (and matching the edge full.default kwarg signature) would be safer. Minor, and none of the current tests exercise this branch — you may want to add an offset large enough to produce an empty diagonal (e.g. Diagonal(offset=5) on a 4x4) to cover it.

3. dim1 == dim2 / out-of-range dims aren't validated, but PyTorch would reject those upstream, so this is fine to leave.

Nits

• The full test body is duplicated verbatim between TestQNNFloatingPointOperator and TestQNNQuantizedOperator (only the get_qdq_module line differs). This matches the existing convention in the file, so no change needed — just noting it.
• Test coverage is good for offsets ±1 and the 3D dim1/dim2 case; consider adding a negative-dim case (e.g. dim1=-2, dim2=-1) to exercise the normalization at lines 169–172.

Overall this looks well-structured and consistent with the surrounding QNN passes. Item #1 is the only one I'd want confirmed before merge.
· branch diagonal