Qualcomm AI Engine Direct - Refactor QnnDlcManager

[chenweng-quic]

Summary

Refactor Dlc manager from experimental API to formal one.

Reference

https://docs.qualcomm.com/doc/80-63442-10/topic/function_QnnSystemDlc_8h_1ad09233e5a66c421e0e80f4cdbf4c1b7e.html
https://docs.qualcomm.com/doc/80-63442-10/topic/function_QnnSystemDlc_8h_1aa3fcdf5c15256a69d445fc2a8c7a0e60.html

Test plan

There is a unit test for online prepare already.

cc @cccclai @cbilgin @abhinaykukkadapu

[pytorch-bot]

🔗 Helpful Links

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

• 📄 Preview Python docs built from this PR

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

• Rolling out OSDC (ARC) runners on pull & trunk workflows in PyTorch main

❌ 1 New Failure, 4 Unrelated Failures

As of commit 33d9fa0 with merge base 8e5ec80 ():

NEW FAILURE - The following job has failed:

• pull / android / run-emulator (gh)
  The process '/usr/bin/sh' failed with exit code 1

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

• pull / unittest / macos / macos-job (gh) (trunk failure)
  export/tests/test_target_recipes.py::TestTargetRecipes::test_mv2_model
• pull / unittest / windows / windows-job (gh) (trunk failure)
  ##[error]The operation was canceled.
• pull / unittest-editable / macos / macos-job (gh) (trunk failure)
  export/tests/test_target_recipes.py::TestTargetRecipes::test_mv2_model
• pull / unittest-editable / windows / windows-job (gh) (trunk failure)
  ##[error]The operation was canceled.

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

[github-actions]

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.

[shewu-quic]

@claude Review this PR.

[shewu-quic]

Hi @abhinaykukkadapu ,

I noticed that I’m unable to request a Claude review. Is there any plan to allow collaborators access to this feature?

[abhinaykukkadapu]

@claude review this PR.

[claude]

Claude finished @abhinaykukkadapu's task in 2m 49s —— View job

---

Review of PR #19105: Qualcomm AI Engine Direct - Refactor QnnDlcManager

• Gather context and understand the PR
• Review changed files in detail
• Post review feedback

---

Overall: Good refactoring. Moving from the experimental libQnnModelDlc.so / QnnModel_composeGraphsFromDlc API to the formal QnnSystemDlc API via libQnnSystem.so is a solid improvement. The new API is cleaner, eliminates the memfd_create/mmap workaround, and removes a runtime dlopen/dlsym dependency. A few issues to address:

---

Issues

1. dlc_handle is never freed (resource leak)
QnnDlcManager.h:59 — RegisterGraphsFromDLC creates a QnnSystemDlc_Handle_t dlc_handle via qnn_system_dlc_create_from_binary, but never frees it after qnn_system_dlc_compose_graphs completes. Per the Qualcomm docs, there should be a corresponding QnnSystemDlc_free or equivalent cleanup call. Consider either:

• Storing dlc_handle as a member and freeing it in Destroy(), or
• Freeing it at the end of RegisterGraphsFromDLC if it's not needed after graph composition.

Fix this →

2. Commented-out code left in
QnnDlcManager.h:81-82 — Two commented-out variable declarations remain:

// QnnSystemContext_GraphInfo_t* graphs = nullptr;
// uint32_t num_graphs = 0;

These appear to be leftover from development and should be removed.

Fix this →

3. Dead type alias and stale include
QnnDlcManager.h:17,23 — The #include "QnnWrapperUtils.hpp" and the using QnnModel_composeGraphsFromDlc = qnn_wrapper_api::ModelError_t (*)(...) type alias are no longer used anywhere (the old composeGraphsFromDlc call site is deleted). These should be removed.

Fix this →

4. Missing error check on qnn_graph_retrieve
QnnManager.cpp:553-556 — The return value of qnn_graph_retrieve is discarded. If the graph name doesn't exist in the context, this would silently return an invalid handle that's then used in GraphFinalize. Consider checking the error:

error = backend_bundle_ptr_->implementation->GetQnnInterface().qnn_graph_retrieve(
    backend_params_ptr_->qnn_context_ptr_->GetHandle(),
    graphInfo.graphName,
    &graphHandle);
if (error != QNN_SUCCESS) {
    QNN_EXECUTORCH_LOG_ERROR("Failed to retrieve graph %s. Error %d.",
        graphInfo.graphName, QNN_GET_ERROR_CODE(error));
    return Error::Internal;
}

Fix this →

---

Minor / Nits

5. RegisterGraphsFromDLC is now a large inline method in the header
QnnDlcManager.h:49-104 — This ~55-line method with non-trivial logic (API calls, error handling, loops) is defined inline in the header. This works, but putting non-trivial implementations in headers increases compilation coupling and rebuild times. The host-side override was removed (ir/host/QnnDlcManager.cpp no longer has RegisterGraphsFromDLC), so the header definition is shared across host and target. Consider whether this should be a virtual method with a target-side implementation, or at minimum, a method defined in a common .cpp file.

6. Implicit nullptr safety for system_implementation_ on host path
QnnContextCommon.cpp:90-91 — On the host (IR) path, system_implementation_ is nullptr (the host QnnDlcManager doesn't load libQnnSystem.so). The ONLINE_PREPARE cache state guard protects against reaching this code, but if that invariant ever breaks, this will segfault with no diagnostic. A null check or assertion would be cheap insurance.

7. QnnBackendBundle destructor ordering
QnnBackendUnifiedRegistry.h:43-48 — system_implementation is reset after implementation in the destructor. Since system_implementation may hold symbols used by contexts that reference implementation, it might be safer to reset system_implementation before or alongside implementation. Worth verifying the dependency order.

---

Positive observations

• Removing the memfd_create / mmap / /proc/self/fd/ workaround significantly simplifies the target-side code and removes platform-specific dependencies (__ANDROID_API__ checks).
• Removing the libQnnModelDlc.so runtime dependency (from export_utils.py and the dlopen in target code) reduces the deployment surface.
• The _adb error-check improvement in export_utils.py is a nice catch — previously, non-callback ADB failures were silently ignored.
• The Destroy() reorder in QnnManager (DLC manager destroyed before backend params) is correct and prevents use-after-free.

---

[abhinaykukkadapu]

Hi @abhinaykukkadapu ,

I noticed that I’m unable to request a Claude review. Is there any plan to allow collaborators access to this feature?

Sorry, i think it is currently only enabled for maintainers.