Make FCB lowering and cleanup safe (#24159) by alirezamonfared · Pull Request #24159 · pytorch/executorch

alirezamonfared · 2026-10-26T19:07:15Z

Summary:

Flexible Context Binary (FCB) compilation already works through preprocess_multimethod(), but a single ExportedProgram lowered through to_backend() (i.e. QnnBackend.preprocess()) still took the single-SoC path and failed.

This change routes FCB compiler specs from preprocess() through preprocess_multimethod(); checks InitContext results and releases QNN contexts, DLC handles and managers on both success and failure; restores the outer manager registry even when teardown fails; rejects empty or unequal-length target lists; and rejects DLBC combined with reference-weight sharing, which QAIRT does not support. A one-target list still produces a scalar context binary.

Tests are added to TestQNNFcb in test_qnn_delegate.py, which the test-qnn-delegate-linux workflow now runs.

Authored with Codex; review fixes with Claude Code.

Test Plan:

QNN delegate FCB test suite: 18 passed, 2 skipped (device-only end-to-end cases skip on the x86 simulator).
Targeted floating-point and quantized QNN delegate regression suite covering multi-context, convolution, linear, element-wise addition, matrix multiplication, and softmax paths: 18 passed, 0 failed.
ExecuTorch lintrunner: clean.

FCB size data from the original export (unchanged by this revision): SM8450 plus three v7x targets 1,033,499 bytes; the same three v7x targets without SM8450 925,337 bytes.

Differential Revision: D119582979

Pulled By: alirezamonfared

pytorch-bot · 2026-10-26T19:07:20Z

🔗 Helpful Links

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

📄 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:

PRs based before #202327 fail all Linux builds: can't find build-external-packages action

⏳ No Failures, 7 Pending

As of commit 63c6878 with merge base 5c1112c ():
💚 Looks good so far! There are no failures yet. 💚

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

meta-codesync · 2026-10-26T19:07:26Z

@alirezamonfared has exported this pull request. If you are a Meta employee, you can view the originating Diff in D119582979.

linux-foundation-easycla · 2026-10-26T19:07:26Z

The committers listed above are authorized under a signed CLA.

✅ login: alirezamonfared / name: Ali Monfared (63c6878)

psiddh · 2026-10-26T19:11:18Z

Thanks for the PR. Can you fix EasyCLA and LintRunner CI issues ?

github-actions · 2026-10-26T19:13:15Z

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.

Summary: ## Summary Flexible Context Binary (FCB) compilation already existed behind the multimethod path, but a single `ExportedProgram` passed through `BackendDetails.preprocess()` still took the scalar path. This made the standard `to_backend()` API unable to produce a multi-target QNN artifact safely. This change: - routes FCB compiler specs from `QnnBackend.preprocess()` through `preprocess_multimethod()`; - scopes `QnnManager` reuse to `QnnManagerContext`, releases every manager, QNN context, and DLC handle on success or failure, and preserves every teardown failure; - validates empty, unequal, duplicate, unsupported, and incompatible multi-target specs while preserving every one-target scalar/list input form; - keeps DLBC plus reference-weight sharing advisory rather than rejecting it; - warns when v6x and v7x-or-newer HTP targets are combined because weight sharing may be less effective; - builds debug mappings after wrapper construction assigns tensor names; - adds public QNN CI coverage for compiler specs, lifecycle, preprocessing, protocol stripping, and cleanup. The scalar path remains unchanged. A one-target list still produces a scalar context binary. ## Authorship Authored with Codex. Differential Revision: D119582979

alirezamonfared · 2026-10-26T20:23:56Z

@psiddh -- I think the CLA shall be fixed, I opted into Meta and the bot has added it to the PR

Summary: ## Summary Flexible Context Binary (FCB) compilation already existed behind the multimethod path, but a single `ExportedProgram` passed through `BackendDetails.preprocess()` still took the scalar path. This made the standard `to_backend()` API unable to produce a multi-target QNN artifact safely. This change: - routes FCB compiler specs from `QnnBackend.preprocess()` through `preprocess_multimethod()`; - scopes `QnnManager` reuse to `QnnManagerContext`, releases every manager, QNN context, and DLC handle on success or failure, and preserves every teardown failure; - validates empty, unequal, duplicate, unsupported, and incompatible multi-target specs while preserving every one-target scalar/list input form; - keeps DLBC plus reference-weight sharing advisory rather than rejecting it; - warns when v6x and v7x-or-newer HTP targets are combined because weight sharing may be less effective; - builds debug mappings after wrapper construction assigns tensor names; - adds public QNN CI coverage for compiler specs, lifecycle, preprocessing, protocol stripping, and cleanup. The scalar path remains unchanged. A one-target list still produces a scalar context binary. ## Authorship Authored with Codex. Test Plan: ## Automated - Focused FCB compiler-spec, lifecycle, preprocessing, and cleanup suite: 18 passed. - The public test-qnn-delegate-linux workflow runs the compiler-spec, manager-lifecycle, and FCB-preprocessing test files. - Existing floating-point and quantized QNN operator coverage remains in the same workflow. ## FCB size comparison - Two production FCB exports used the identical source, exporter, SDK, precision, shape, and reference-weight-sharing setting; only the target list changed. - SM8450 plus three v7x targets: 1,033,499 bytes. - The same three v7x targets without SM8450: 925,337 bytes. - Including SM8450 added 108,162 bytes, or 11.69 percent relative to the three-target artifact. This is artifact-size evidence only; it does not independently prove the compiler physical weight-sharing behavior. ## Coverage - single-program FCB dispatch through the standard backend API; - all one-target scalar/list compatibility forms; - invalid and incompatible multi-target specifications; - nested manager registry restoration, best-effort teardown, and multi-error aggregation; - QNN context and DLC cleanup on initialization and compilation failures; - supported custom-protocol stripping; - debug mapping after tensor-name assignment. Differential Revision: D119582979 Pulled By: alirezamonfared

psiddh · 2026-10-28T07:28:42Z

@quic-boyuc can you review this PR ?

cc @billmguo

quic-boyuc · 2026-10-28T08:43:59Z

    """
    setup_qnn_sdk()
-    # Normalize inputs
-    soc_models = soc_model if isinstance(soc_model, list) else [soc_model]


Hi @alirezamonfared
Thanks for the PR. I think the safety fixes helps!

I notice some parts of your PR description doesn't seem to reflect the current mainline behavior.
I suspect the changes may have been prepared on top of the draft PR rather than the officially merged PR in mainline. Could you please update the description accordingly?

keeps DLBC plus reference-weight sharing advisory rather than rejecting it;

This behavior is already a warning rather than an error in mainline. From what I can see, this PR updates the comment wording, but does not change the behavior itself. Could you update the description to reflect that?

warns when v6x and v7x-or-newer HTP targets are combined because weight sharing may be less effective;

I couldn't find this behavior in the code changes. I think this would be a useful addition. Could you confirm whether you intend to include it in this PR? If so, please sync the implementation and the description. Otherwise, we can remove it from the description.

builds debug mappings after wrapper construction assigns tensor names;

I also couldn't find this change in the PR. As far as I can tell, this issue does not exist in the current mainline version, so I think this item can be removed from the description.

Thanks!

Good catch, agree. The description was written against the draft PR. The next patch updates it to only list what this diff changes, and drops the v6x/v7x warning and debug-map items since they're already in mainline.

Thanks
I think the new version fix all the issues.

quic-boyuc · 2026-10-28T08:57:02Z

        pytest -vs backends/qualcomm/tests/test_qnn_delegate.py \
            -k "TestQNNFloatingPointOperator or TestQNNQuantizedOperator"

+        # Run focused FCB compiler-contract, lifecycle, and preprocessing tests.


Hi @shewu-quic @haowhsu-quic

I have a few questions about the testing strategy here:

Do we have a policy or preferred approach for sharing tests between Meta internal CI and Qualcomm CI?

Some of the functional tests proposed here seem to overlap with tests previously added to test_qnn_delegate.py and rework framework tests. It would be nice if we could reuse/share these test cases rather than maintain duplicate coverage.

Codex is proposing standalone test files under the test directory. What's your take on this approach?

I'm wondering whether we should follow an existing structure or convention instead, especially if we want to keep the test framework consistent.

Do we plan to provide contributors with instructions or guidelines for running and adding tests?

Currently, the lack of clear guidance may make it difficult for both coding agents and contributors to determine the preferred testing approach, which could lead to further fragmentation of the test framework over time.

Thanks!

Do we have a policy or preferred approach for sharing tests between Meta internal CI and Qualcomm CI?

Yes, that's ongoing work. Previously, Meta has provided a guide to support cross-repo CI.

Codex is proposing standalone test files under the test directory. What's your take on this approach?

Use the existing test_qnn_delegate.py and tests/rework as references when adding the test case. Once this PR is merged, we will begin migrating to the new test framework (tests/rework) to ensure quality.

Do we plan to provide contributors with instructions or guidelines for running and adding tests?

Sure. In fact, we do have a contributor guideline; please feel free to update it if it’s outdated.

https://github.com/pytorch/executorch/tree/main/backends/qualcomm#pull-requests

Agree. The next patch moves these into TestQNNFcb in test_qnn_delegate.py, removes the standalone files, and adds TestQNNFcb to the existing job's filter. Happy to move them into tests/rework as part of your migration.

quic-boyuc · 2026-10-28T08:59:22Z

        option = generate_qnn_executorch_option(compile_specs)
        obj_options = flatbuffer_to_option(option)
+        if obj_options.target_options is not None:
+            return QnnBackend.preprocess_multimethod(


Hi @shewu-quic @winskuo-quic

Do we need to support to_backend and single method lowering?
Previously this was not included in planned scope.
Should we just delegate every call to preprocess_multimethod to reduce maintainance effort?

Based on the interface contract, it looks like preprocess still needs to be supported by every backend, even though single-method lowering wasn't originally included in our planned scope.
https://github.com/pytorch/executorch/blob/main/exir/backend/backend_details.py#L132

That said, I think we can share as much implementation as possible between preprocess and preprocess_multimethod to minimize the maintenance effort.

I think we do need preprocess(), per the backend interface contract, and we have users lowering through to_backend() directly. Agree on delegating everything to preprocess_multimethod to remove the duplicate path. To keep this PR focused, I'd like to do that as a follow-up right after this lands, if that works for you. Wdyt ?

shewu-quic

Thanks to make it robust.
About test file, could you use an existing test case as a basis for adding a new one?

shewu-quic · 2026-10-28T09:40:04Z

    setup_qnn_sdk()
-    # Normalize inputs
-    soc_models = soc_model if isinstance(soc_model, list) else [soc_model]
+    soc_model_is_list = isinstance(soc_model, list)


I think the inline check is a bit simpler here since soc_model_is_list and backend_options_is_list are only used once. Is there a particular reason you'd prefer to keep it as a separate variable?

shewu-quic · 2026-10-28T09:43:34Z

            (Refer to qc_schema.py for a complete list of supported SoCs).
            Pass an ordered list with an equal-length ``backend_options`` list to
-            create a Flexible Context Binary (FCB). Each position
+            create a Flexible Context Binary (FCB). A one-element list retains


Is this clarification necessary? FCB is only triggered when multiple SoCs are provided, so a single-element list naturally follows the single-SoC behavior

will drop that sentence in the next patch

shewu-quic · 2026-10-28T09:46:27Z

        fcb_reference_weight_sharing: Enable reference-weight sharing while the
            host AOT flow appends FCB contexts to its DLC. Enabled by default;
-            has no effect for a scalar target. It cannot be combined with DLBC.
+            has no effect for a scalar target. Combining it with DLBC emits a


Based on QAIRT documentation, reference-weight sharing is not supported with DLBC. Do you have any usecase for it?

ok. Thanks for pointing to the doc.. no use case on our side. The next patch makes it a ValueError that names the fix (disable use_dlbc or set fcb_reference_weight_sharing=False), so it fails early instead of hitting an opaque QNN error later.

shewu-quic · 2026-10-28T10:05:58Z

        pytest -vs backends/qualcomm/tests/test_qnn_delegate.py \
            -k "TestQNNFloatingPointOperator or TestQNNQuantizedOperator"

+        # Run focused FCB compiler-contract, lifecycle, and preprocessing tests.


Do we have a policy or preferred approach for sharing tests between Meta internal CI and Qualcomm CI?

Yes, that's ongoing work. Previously, Meta has provided a guide to support cross-repo CI.

Codex is proposing standalone test files under the test directory. What's your take on this approach?

Use the existing test_qnn_delegate.py and tests/rework as references when adding the test case. Once this PR is merged, we will begin migrating to the new test framework (tests/rework) to ensure quality.

Do we plan to provide contributors with instructions or guidelines for running and adding tests?

Sure. In fact, we do have a contributor guideline; please feel free to update it if it’s outdated.

https://github.com/pytorch/executorch/tree/main/backends/qualcomm#pull-requests

psiddh · 2026-10-28T23:26:31Z

Thanks @quic-boyuc and @shewu-quic for the review! Agree with the feedback. The next patch addresses it: the description will only cover what this PR changes, the tests move into TestQNNFcb in test_qnn_delegate.py, DLBC with reference-weight sharing becomes an error, and the two nits get fixed. It'll also replace the ExceptionGroup in destroy_all() so it works on Python 3.10.

psiddh · 2026-10-29T06:08:31Z

@shewu-quic @quic-boyuc Can you check if all your comments are addressed ? Any other feedback ?

Summary: Flexible Context Binary (FCB) compilation already works through `preprocess_multimethod()`, but a single `ExportedProgram` lowered through `to_backend()` (i.e. `QnnBackend.preprocess()`) still took the single-SoC path and failed. This change routes FCB compiler specs from `preprocess()` through `preprocess_multimethod()`; checks `InitContext` results and releases QNN contexts, DLC handles and managers on both success and failure; restores the outer manager registry even when teardown fails; rejects empty or unequal-length target lists; and rejects DLBC combined with reference-weight sharing, which QAIRT does not support. A one-target list still produces a scalar context binary. Tests are added to `TestQNNFcb` in `test_qnn_delegate.py`, which the `test-qnn-delegate-linux` workflow now runs. Authored with Codex; review fixes with Claude Code. Differential Revision: D119582979

quic-boyuc · 2026-10-29T08:57:08Z

@shewu-quic @quic-boyuc Can you check if all your comments are addressed ? Any other feedback ?

My comments are all addressed
Thanks a lot 👍

Summary: Flexible Context Binary (FCB) compilation already works through `preprocess_multimethod()`, but a single `ExportedProgram` lowered through `to_backend()` (i.e. `QnnBackend.preprocess()`) still took the single-SoC path and failed. This change routes FCB compiler specs from `preprocess()` through `preprocess_multimethod()`; checks `InitContext` results and releases QNN contexts, DLC handles and managers on both success and failure; restores the outer manager registry even when teardown fails; rejects empty or unequal-length target lists; and rejects DLBC combined with reference-weight sharing, which QAIRT does not support. A one-target list still produces a scalar context binary. Tests are added to `TestQNNFcb` in `test_qnn_delegate.py`, which the `test-qnn-delegate-linux` workflow now runs. Authored with Codex; review fixes with Claude Code. Test Plan: - QNN delegate FCB test suite: 18 passed, 2 skipped (device-only end-to-end cases skip on the x86 simulator). - Targeted floating-point and quantized QNN delegate regression suite covering multi-context, convolution, linear, element-wise addition, matrix multiplication, and softmax paths: 18 passed, 0 failed. - ExecuTorch lintrunner: clean. FCB size data from the original export (unchanged by this revision): SM8450 plus three v7x targets 1,033,499 bytes; the same three v7x targets without SM8450 925,337 bytes. Differential Revision: D119582979 Pulled By: alirezamonfared Signed-off-by: Ali Monfared <2058201+alirezamonfared@users.noreply.github.com>

alirezamonfared requested a review from psiddh as a code owner October 26, 2026 19:07

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 Oct 26, 2026

meta-codesync Bot added the meta-exported label Oct 26, 2026

psiddh added the module: qnn Issues related to Qualcomm's QNN delegate and code under backends/qualcomm/ label Oct 26, 2026

meta-codesync Bot changed the title ~~Make FCB lowering and cleanup safe~~ Make FCB lowering and cleanup safe (#24159) Oct 26, 2026

alirezamonfared force-pushed the export-D119582979 branch 2 times, most recently from c228a35 to 565a818 Compare October 26, 2026 19:30

alirezamonfared force-pushed the export-D119582979 branch from 565a818 to 3c4b95f Compare October 26, 2026 19:44

alirezamonfared force-pushed the export-D119582979 branch from 3c4b95f to f1cf142 Compare October 26, 2026 21:55

quic-boyuc reviewed Oct 28, 2026

View reviewed changes

shewu-quic reviewed Oct 28, 2026

View reviewed changes

meta-codesync Bot changed the title ~~Make FCB lowering and cleanup safe (#24159)~~ Make FCB lowering and cleanup safe Oct 29, 2026

alirezamonfared force-pushed the export-D119582979 branch from f1cf142 to 8decea0 Compare October 29, 2026 05:45

meta-codesync Bot changed the title ~~Make FCB lowering and cleanup safe~~ Make FCB lowering and cleanup safe (#24159) Oct 29, 2026

alirezamonfared force-pushed the export-D119582979 branch from 8decea0 to 66ac9d3 Compare October 29, 2026 07:11

psiddh approved these changes Oct 29, 2026

View reviewed changes

alirezamonfared force-pushed the export-D119582979 branch from 66ac9d3 to 63c6878 Compare October 29, 2026 16:19

psiddh merged commit 012fd14 into pytorch:main Oct 29, 2026
219 checks passed

Conversation

alirezamonfared commented Oct 26, 2026 • edited by meta-codesync Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

❗ 1 Active SEVs

⏳ No Failures, 7 Pending

Uh oh!

meta-codesync Bot commented Oct 26, 2026

Uh oh!

linux-foundation-easycla Bot commented Oct 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psiddh commented Oct 26, 2026

Uh oh!

github-actions Bot commented Oct 26, 2026

This PR needs a release notes: label

Uh oh!

alirezamonfared commented Oct 26, 2026

Uh oh!

psiddh commented Oct 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shewu-quic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psiddh commented Oct 28, 2026

Uh oh!

psiddh commented Oct 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quic-boyuc commented Oct 29, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alirezamonfared commented Oct 26, 2026 •

edited by meta-codesync Bot

Loading

pytorch-bot Bot commented Oct 26, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Oct 26, 2026 •

edited

Loading

This PR needs a `release notes:` label

psiddh commented Oct 29, 2026 •

edited

Loading