Skip to content

Qualcomm AI Engine Direct - Adding QNN backend support for as_strided core ATen op - #21987

Open
qti-horodnic wants to merge 1 commit into
pytorch:mainfrom
CodeLinaro:as_strided
Open

Qualcomm AI Engine Direct - Adding QNN backend support for as_strided core ATen op#21987
qti-horodnic wants to merge 1 commit into
pytorch:mainfrom
CodeLinaro:as_strided

Conversation

@qti-horodnic

Copy link
Copy Markdown
Contributor

Summary

Added support for the aten.as_strided core ATen op on the QNN backend.

as_strided creates a view of a tensor with specified size, stride, and storage_offset. Since QNN has no native as_strided op, we decompose it by:

Fast path:
If strides are contiguous, storage_offset == 0, and the output has the same number of elements as the input, replace the op with a reshape/view.

General path (via module export + merge_decomposed_graph):

  1. Flatten input to 1D
  2. index_select with precomputed linear indices (int32 constant)
  3. Reshape to target output size

Test plan

pytest backends/qualcomm/tests/rework/htp/op/v68/test.py -k "test_as_strided" -v


python backends/qualcomm/tests/test_qnn_delegate.py TestQNNQuantizedOperator.test_qnn_backend_as_strided --soc_model SM8750 --host aisw-vm15-labsd --device 545ee4aa --build_folder build-android

python backends/qualcomm/tests/test_qnn_delegate.py TestQNNFloatingPointOperator.test_qnn_backend_as_strided --soc_model SM8750 --host aisw-vm15-labsd --device 545ee4aa --build_folder build-android

Note: The fp cases in the rework fail because of a bug introduced in this pr. There is a fix pending review internally; I've tried that fix and the rework test does pass with it applied. I'll let that change go separately, adding this note here for visibility.

@pytorch-bot

pytorch-bot Bot commented Aug 20, 2026

Copy link
Copy Markdown

🔗 Helpful Links

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

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

❌ You can merge normally! (1 Unrelated Failure), 2 Unclassified Failures

As of commit e90b748 with merge base ed7a23e (image):

UNCLASSIFIED FAILURES - DrCI could not classify the following jobs because the workflow did not run on the merge base. The failures may be pre-existing on trunk or introduced by this PR:

FLAKY - The following job failed but was 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 Aug 20, 2026
@qti-horodnic

Copy link
Copy Markdown
Contributor Author

@pytorchbot label "release notes: qualcomm"

@pytorch-bot pytorch-bot Bot added the release notes: qualcomm Changes to the Qualcomm backend delegate label Aug 20, 2026
@psiddh

psiddh commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

@claude pls review

@claude

claude Bot commented Aug 20, 2026

Copy link
Copy Markdown

Claude finished @psiddh's task in 2m 55s —— View job


PR Review: QNN backend support for as_strided

  • Read changed files
  • Review decompose pass logic
  • Review pass manager & registration
  • Review tests
  • Post feedback

Overall this is a clean, well-structured addition. The decomposition strategy (view fast-path + flatten/index_select/reshape general path) is sound, it follows the established merge_decomposed_graph pattern (same as DecomposeRoll), int32 indices are correctly chosen for QNN, and the pass is registered in both the annotation and export pipelines so the gather is visible to the quantizer. Test coverage across FP/quantized/rework is good. A few points below.

Correctness / robustness

1. storage_offset (and size/stride) read positionally onlydecompose_as_strided.py:164-169

input_node = node.args[0]
size_arg = node.args[1]
stride_arg = node.args[2]
storage_offset = node.args[3] if len(node.args) > 3 else 0

If any of these ever arrive as node.kwargs (e.g. storage_offset= passed by keyword), a non-zero offset would be silently read as 0, producing a wrong result. torch.export normally normalizes to positional args so this is low-risk, but a defensive node.kwargs.get("storage_offset", ...) fallback (or an assert that kwargs is empty) would remove the sharp edge. Fix this →

2. Bounds check uses assertdecompose_as_strided.py:187
The out-of-bounds guard is a bare assert, which is stripped under python -O. Since it guards index correctness (an OOB index would produce garbage / read out of the flattened buffer at runtime), consider raising an explicit exception instead so it survives optimized runs.

3. Assumption that the input is contiguous in the view fast-pathdecompose_as_strided.py:193-198
The fast-path replaces as_strided with a view when strides are contiguous, offset is 0, and numel matches. This is correct only if the input tensor's storage is itself contiguous. After functionalization intermediates are materialized contiguous, so this holds in practice, but it's an implicit assumption worth a one-line comment for future readers.

Design / performance

4. Index buffer size — The general path materializes an int32 index buffer of size prod(output_size) as a constant baked into the .pte. For large strided views this can noticeably inflate model size and add an index_select over the full flattened input. Fine for the tested shapes; worth a brief note (or a log/comment) that this path is intended for modest view sizes.

Minor / style

  • Duplicated test bodiestest_qnn_delegate.py:334 and :3454: the FP and quantized test_qnn_backend_as_strided share ~45 lines of identical test_comb. Consider hoisting the case list to a module-level constant or helper to avoid drift between the two. (Matches the repo's "no needless duplication" guidance, though I see many existing tests follow this same copy pattern, so it's optional.)
  • # noqa: C901 on _decompose_with_gather (:121) — the function is fairly linear; the complexity suppression may be unnecessary. Not blocking.
  • setattr(decomposed_module, buffer_name, model.indices) (:144) — the comment says "so node_copy works", but node_copy only copies node metadata (the get_attr has no args to transform) and the attribute is resolved against graph_module where you already register_buffer. If this setattr isn't actually required, dropping it would remove a confusing line; if it is required for a downstream pass, a more precise comment would help.

Notes

  • I did not run the test suite (device-farm / QNN SDK required), so I'm relying on the code and your reported results.
  • You mention the FP rework cases fail due to a pre-existing bug from Qualcomm AI Engine Direct - Add QNN HadamardTransform op support #21729 with a fix pending internally — worth linking that fix here once it lands so this PR's rework expectations can be confirmed green.

Nice work overall — the structure mirrors the existing decompose passes well and the fast-path/general-path split is a good call.
• branch as_strided

@qti-horodnic

qti-horodnic commented Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Regarding the Claude comments, 1. and 2. are the general conventions for our op decomposition implementations, will keep as-is for uniformity.
3. is a valid concern, added a guard for if the input is not contiguous. Added a docstring clarification for 4.

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. release notes: qualcomm Changes to the Qualcomm backend delegate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants