Skip to content

Qualcomm AI Engine Direct - Add HTP context graph splitting option - #22595

Open
chenweng-quic wants to merge 3 commits into
pytorch:mainfrom
CodeLinaro:dev1/chenweng/graph_split
Open

chenweng-quic wants to merge 3 commits into
pytorch:mainfrom
CodeLinaro:dev1/chenweng/graph_split

Conversation

@chenweng-quic

@chenweng-quic chenweng-quic commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Exposes QNN's HTP context graph-splitting config (QNN_HTP_CONTEXT_CONFIG_OPTION_GRAPH_SPLITTING_CONFIGS / QnnHtpContext_GraphSplit_t) via a new use_graph_splitting option on generate_htp_compiler_spec(), see https://docs.qualcomm.com/doc/80-63442-10/topic/htp_backend.html. This option significantly reduces finalization time at the cost of a slight increase in execution time.

Misc

  • Wired up for the offline-prepare (host/x86 export) path only, since this feature is used in finalize.
  • Gated behind QNN_HTP_API_VERSION_MAJOR >= 5 && QNN_HTP_API_VERSION_MINOR >= 49 since this option doesn't exist in older QNN_HTP_API_VERSION

Test plan

python backends/qualcomm/tests/test_qnn_delegate.py -k "test_qnn_backend_graph_splitting" --device ef5e4029 --host localhost --soc_model SM8850 --build_folder build-android --executorch_root . 

LLAMA3.2 3B performance:

Graph Graph Preparation Initializing (us) Graph Optimizations (us) Post Graph Optimization (us) Graph Sequencing for Target (us) VTCM Allocation (us) Parallelization Optimization (us) Finalizing Graph Sequence (us) Completion (us) QNN (execute) time (us)
kv_forward1_without_gpe 1146 26843138 433549 6828982 449030 5929118 266695 15171 9552
kv_forward1_with_gpe 496 2014382 26630 427743 27504 246911 22066 1268 9880
kv_forward2_without_gpe 373 27618966 1049136 10700233 313808 7947260 291464 14841 9452
kv_forward2_with_gpe 599 2511473 34099 577162 36742 353654 31248 1754 9787
kv_forward3_without_gpe 405 30262309 541066 8523872 407082 8058194 314898 16692 11136
kv_forward3_with_gpe 512 5575498 75421 1322133 111640 1009982 51157 3371 11437
kv_forward4_without_gpe 1966 10304161 36241 1151113 87287 749097 34706 1944 6703
kv_forward4_with_gpe 342 8387236 41997 1104533 96270 870457 31896 2245 6686
prefill_forward1_without_gpe 403 23425582 521963 20288202 28141820 13415019 534725 19491 23283
prefill_forward1_with_gpe 635 2404414 42852 928676 607167 368336 25194 1790 20824
prefill_forward2_without_gpe 354 22249865 496426 16837420 24738364 9634081 464298 16200 20473
prefill_forward2_with_gpe 557 2521494 41107 929431 765368 426039 28336 1955 20764
prefill_forward3_without_gpe 744 31356868 553758 17889195 17232990 9485483 500217 15025 21125
prefill_forward3_with_gpe 1933 5689220 99188 2404130 2256161 1236475 79918 6054 21835
prefill_forward4_without_gpe 889 10117697 43571 1450046 1323893 1062400 35717 2122 9431
prefill_forward4_with_gpe 331 8484639 50456 1647489 1798310 1306534 45152 2760 9446

This table is created by extracting data from finalization log:
image

This PR was authored with assistance from Claude Code (Anthropic).

cc @cbilgin @psiddh

@pytorch-bot

pytorch-bot Bot commented Sep 8, 2026 •

Copy link
Copy Markdown

🔗 Helpful Links

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

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

❌ 1 New Failure, 4 Pending

As of commit ccb9f02 with merge base 5e21c13 (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 Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

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.

@chenweng-quic chenweng-quic added the module: qnn Issues related to Qualcomm's QNN delegate and code under backends/qualcomm/ label Sep 8, 2026
@chenweng-quic
chenweng-quic force-pushed the dev1/chenweng/graph_split branch from 0fa0ae7 to aed61f4 Compare September 10, 2026 06:57
@chenweng-quic

Copy link
Copy Markdown
Contributor Author

Hi @psiddh,
When you have a moment, could you take a look at the PR?

@psiddh

psiddh commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

@claude thoroughly review this PR , also review it from any potential regression stand point

@claude

claude Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

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


PR Review: HTP context graph-splitting option

Reviewed the full diff against the QNN backend. Overall this is a clean, well-scoped, opt-in feature with the right default (False) and preprocessor gating, so the regression risk to existing paths is low. A few correctness concerns worth addressing before merge.

🔴 Potential bug — uninitialized struct fields

HtpContextCustomConfig.cpp:34-36

QnnHtpContext_GraphSplit_t graph_split_info;   // not initialized
graph_split_info.graphSplittingEnabled = true;
p_custom_config->graphSplittingConfigs = graph_split_info;

Only graphSplittingEnabled is set. If QnnHtpContext_GraphSplit_t has any other members (thresholds, reserved/padding, etc.), they are copied into the config as indeterminate garbage — the QNN runtime may then read uninitialized values. The weight-sharing path above gets away with a single-field assignment because that config is a bare bool, but a struct should be value-initialized. QNN headers typically ship an init macro for exactly this. Prefer one of:

QnnHtpContext_GraphSplit_t graph_split_info = QNN_HTP_CONTEXT_GRAPH_SPLIT_INIT; // if the SDK defines it
// or, at minimum:
QnnHtpContext_GraphSplit_t graph_split_info{};                                  // zero-init all fields

Fix this →

🟠 Version-gate logic compares major/minor independently

HtpContextCustomConfig.cpp:29

#if (QNN_HTP_API_VERSION_MAJOR >= 5 && QNN_HTP_API_VERSION_MINOR >= 49)

This does not correctly express "HTP API ≥ 5.49". A future 6.0 evaluates as 6>=5 && 0>=49 → false, silently disabling the feature on a newer SDK. The correct predicate is:

#if (QNN_HTP_API_VERSION_MAJOR > 5 || (QNN_HTP_API_VERSION_MAJOR == 5 && QNN_HTP_API_VERSION_MINOR >= 49))

(The existing >= 2 && >= 21 style checks elsewhere in the backend have the same latent issue, but since this PR introduces a fresh gate it's worth getting right here.)

🟠 Warning message prints the wrong version macros

HtpContextCustomConfig.cpp:41-46 — the #else branch talks about the HTP API requirement ("requires HTP API 5.49 or newer") but interpolates QNN_API_VERSION_MAJOR/MINOR (the core QNN API, ~2.x). So a user on an old SDK sees something like "API 2.35 does not support ... requires HTP API 5.49", mixing two version schemes. Use QNN_HTP_API_VERSION_MAJOR/QNN_HTP_API_VERSION_MINOR to match the gate and the message.

🟡 Tests leak an artifact on failure

test_qnn_delegate.py:7228-7231 and :8331-8334 (and the rework feature.py variant):

self.assertTrue(os.path.isfile("forward_schematic.bin_sg_0.py"))
os.remove("forward_schematic.bin_sg_0.py")

If the assertTrue fails, os.remove never runs and the file is left in the CWD, which can poison later runs. It's also written to the current working directory with a fixed name (collision risk under pytest -n auto). Suggest self.addCleanup(...) / try/finally, e.g.:

self.addCleanup(lambda: os.path.exists("forward_schematic.bin_sg_0.py") and os.remove("forward_schematic.bin_sg_0.py"))
self.assertTrue(os.path.isfile("forward_schematic.bin_sg_0.py"))

🟡 Version-scheme consistency between Python and C++

The Python tests gate on is_qnn_sdk_version_less_than("2.49") (QNN SDK version) while the C++ gates on HTP API 5.49. These are different numbering schemes; please double-check they truly correspond (SDK 2.49 ↔ HTP API 5.49) — otherwise a test could run and assert on a build where the C++ path is compiled out, or vice-versa. Note is_qnn_sdk_version_less_than only compares when the major matches (current_major == target_major and current_minor < target_minor), so on a hypothetical SDK 3.x it returns False and the test would run regardless; that's the existing helper behavior, just worth being aware of.

✅ Regression assessment

  • Default use_graph_splitting=False throughout (qc_schema.py, utils.py, fbs) → no change to existing exports unless explicitly opted in.
  • New flatbuffer field is appended at the end of QnnExecuTorchHtpBackendOptions → backward/forward-compatible; older .pte blobs deserialize fine.
  • C++ config only appended when the flag is set and SDK supports it; empty otherwise.
  • Feature is offline/host-only per the PR description — confirm the flag is genuinely inert on the on-device (target) build so it can't perturb runtime finalize there.

Minor

  • The new log line in QnnBackendFactory.cpp:63-65 is fine and mirrors the neighboring use_weight_sharing log.
  • The docstring in utils.py for use_slc_allocator has a pre-existing typo ("help the by reducing overall bandwith") — not yours, just noting since you're editing adjacent lines.

Nice, focused PR — addressing the struct init (#1) is the one I'd consider blocking; the rest are polish.
· branch dev1/chenweng/graph_split

@chenweng-quic
chenweng-quic force-pushed the dev1/chenweng/graph_split branch from 2c6b578 to 1606852 Compare September 17, 2026 02:11
@chenweng-quic
chenweng-quic force-pushed the dev1/chenweng/graph_split branch from 86835b8 to 99abe96 Compare October 1, 2026 02:53
Exposes QNN_HTP_CONTEXT_CONFIG_OPTION_GRAPH_SPLITTING_CONFIGS through
generate_htp_compiler_spec(use_graph_splitting=...) for the offline-prepare
(host) export path. Gated behind QNN HTP API >= 5.49 since this option does not
exist in older version.

Co-authored-by with assistance from Claude Code (Anthropic).
@chenweng-quic
chenweng-quic force-pushed the dev1/chenweng/graph_split branch from 99abe96 to ccb9f02 Compare October 5, 2026 03:16
@psiddh

psiddh commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

do you know what this failure is ? MLTEC_elastic - GitHubPOC - executorch dev1-chenweng-graph_split

# file for subgraph 0
# delete artifact before assertion to avoid leak
file_name = "forward_schematic.bin_sg_0.py"
file_exist = os.path.isfile(file_name)

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.

os.path.isfile accepts a file descriptor, so this stats fd 1 or 0 - stdout/stdin not the artifact, did I miss anything ?

This branch has not been deployed

No deployments
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: qnn Issues related to Qualcomm's QNN delegate and code under backends/qualcomm/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants