Qualcomm AI Engine Direct - Keep node names unique across the QNN and Samsung passes, and decompose constant-only ops on HTP by shoumikhin · Pull Request #24316 · pytorch/executorch

shoumikhin · 2026-10-30T02:51:44Z

What is wrong today

A pass that adds nodes can give a new node a name that is already taken, and export then fails with Node redefined name t_1!. torch.export does not record some names it assigns: input names, and ops it renamed to make room for an input (with an input called mul, the model's own q * 2 becomes mul_1). A later pass can hand out one of those names again. In QNN, models with autocast regions or attention hit it. Samsung hits it too: with an input named cat, a model that calls torch.cat and then torch.roll fails in DecomposeRoll.

Separately, on HTP the partitioner keeps a partition that reads nothing at runtime on the CPU, because HTP refuses such a graph. That happens after decomposition, so an op QNN keeps whole, such as linear or hardswish, could stay on the CPU undecomposed, and to_edge_transform_and_lower failed with was not decomposed or delegated.

What this change does

A new helper, reserve_node_names in backends/transforms/utils.py, records the names of all existing nodes, so new nodes always get unused names. The QNN and Samsung pass managers call it before their passes run. torch.fx has no public way to reserve a name, so it writes the namespace's private name set, with a TODO to remove it once torch.export records the names (torch.export does not record the node names it assigns, so a later pass can create a node with a name that is already used pytorch#202927). The to-edge passes run on the program to_edge rebuilds, where every name is recorded.
On HTP, an op that QNN keeps whole is now decomposed when it reads only constants, even when its partition stays delegated (for example x + linear(table, w), where the linear becomes a permute and a matmul). Ops that read a runtime input, and quantize and dequantize ops such as a group-wise weight dequantize, are still kept whole. After lowering, exir still reports any kept-whole op left on the CPU, as before.
The constant-only partition test now checks, in the lowered graph, that the constant-only ops stay on the CPU and the rest is delegated. Its constant branch now includes a linear.

Not covered: with strict export, an autocast model still fails later in to_edge with Tensor dtype mismatch. That error is older than this change.

Tests

On Linux x86_64, with torch 2.14 and a torch 2.16 nightly:

The node name test has an autocast case and an attention case. Both fail without the fix; the attention case also fails if only input names are reserved. Each case checks that the passes removed the region or attention op and lints the result. It then runs again with reserve_node_names patched out and expects Node redefined name; if a torch update removes that collision, the test is skipped instead of failing.
A filter test needs no SDK, so it runs in CI. It checks that a constant-only linear is decomposed, that a linear on an input and a constant-only group-wise dequantize are kept whole, and that on the lowered program the filter lets no constant-only op through.
The constant-only partition test needs the QNN SDK libraries, so CI skips it. It was run with the QNN op check, manager and compile stubbed out: it fails without the HTP fix and passes with it.
The Samsung model above was run through the Samsung export passes with torch only: it fails without the helper and passes with it. Samsung tests need the Samsung SDK, so none was added.
test_passes.py passes, except test_mha_to_sha, which needs torchtune.

cc @cbilgin @psiddh

pytorch-bot · 2026-10-30T02:51:48Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 9b745c0 with merge base 7586c16 ():

FLAKY - The following job failed but was likely due to flakiness present on trunk:

Docathon Labels Sync / check-labels (gh) (detected as infra flaky with no log or failing log classifier)

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

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

shoumikhin · 2026-10-30T02:52:16Z

@claude /pr-review

claude · 2026-10-30T02:53:12Z

Claude finished @shoumikhin's task in 19m 11s —— View job

The reservation fix is correct for QNN's export pipeline, but it patches an upstream torch.export bug by writing private torch.fx state with no TODO, and both changed tests check less than they claim.

Detailed review

PR Review: #24316

Summary

QnnPassManager._transform now reserves every existing node name in the top-level graph's FX namespace, so nodes added by passes can't take a name torch.export gave a placeholder. A second commit tightens the constant-only partition test. The fix resolves the reported Node redefined name failures. The problems:

It works around a torch.export bug inside one backend by writing private torch.fx state, and it has no exit plan.
The two tests assert less than their names and commit messages say.

This review did not run the QNN tests (there is no QNN SDK on the runner). Claims about torch internals come from reading code, not from running it.

Code Quality

backends/qualcomm/_passes/qnn_pass_manager.py:103-108: the workaround writes private torch.fx state in one backend, with no TODO
- Root cause: in torch.export. placeholder_naming_pass assigns node.name directly and never registers the name.
- Private state: the fix writes graph._graph_namespace._used_names, two levels of private torch.fx state. The other two uses in the repo only read it (backends/transforms/fuse_rms_norm.py:375, backends/transforms/fuse_batch_norm_with_conv.py:119).
- Consequence: if a torch pin bump changes _Namespace, every QNN annotation, export and preprocess pipeline fails (lines 470, 481, 520), not just models that would collide.
- Same bug elsewhere: EnnPassManager._transform (backends/samsung/_passes/enn_pass_manager.py:46-47) is identical to QNN's pre-fix body. Its transform_for_export_pass (:60-64) runs DecomposeRoll and DecomposeRemainder in place on a raw torch.export.export(...) program (backends/samsung/utils/export_utils.py:76-77). A model with an input named after a node those passes insert (e.g. mul) would likely fail the same way. I did not reproduce this.
- Ask: file the upstream issue and add a TODO(<link>) at line 103.
  - The TODO should say this is a private-API workaround. The commit message says it ("torch.fx has no public way to reserve a name"); the code comment and PR description don't.
  - Optionally, move the reservation into a shared helper (e.g. backends/transforms/utils.py) that both pass managers call. Fix this →
Low confidence: qnn_pass_manager.py:107 reserves names only in the top-level graph
- Gap: when an outer autocast region is disabled, DecomposeWrapWithAutocast._replace(submod) (decompose_wrap_with_autocast.py:71) inlines nested regions into the submodule's own graph. This loop never touches that graph's namespace.
- Condition: this matters only if the pinned torch also assigns placeholder names directly inside autocast subgraphs. I couldn't check the torch source here.
- Fix: iterating every GraphModule in graph_module.modules() covers it at little cost. Fix this →

Testing

backends/qualcomm/tests/test_passes.py:215-217: the new test never checks that names are unique
- Why assert_close can't catch it: ep.module() deep-copies the graph, and the copy renames duplicate names.
- Why it fails without the fix today: DecomposeWrapWithAutocast (decompose_wrap_with_autocast.py:94) and the SDPA decomposition (backends/transforms/decompose_sdpa.py:47) happen to run DCE, which lints the graph. A collision created by any pass that doesn't run DCE afterwards would pass this test.
- Ask:
  - Call ep.graph_module.graph.lint() after transform_for_export_pipeline.
  - Before running the pipeline, assert that the placeholders really are named t_1 / mul. This follows the file's "Guard against a vacuous test" pattern (:926). Fix this →
test_passes.py:1003-1004: the constant-only partition check is weaker than its commit message
- Claim vs. check: the commit message says all of table * 2 - 1 must stay top-level and the add must be delegated. The test only asserts mul.Tensor and that some delegate exists.
- Missed regression: a partitioner whose constant-only check is non-transitive would keep mul (reads only table) on CPU. It would then delegate sub (reads mul's output) as a separate partition that reads nothing at runtime. The test would still pass.
- Ask: also assert that the sub is in top_level_ops and that exir_ops.edge.aten.add.Tensor is not. Fix this →

Recommendation

Approve

Nothing blocks. The reservation fixes the reported export-pipeline failures. Add the upstream TODO before merging. The shared-helper move and the test hardening can come later.

shoumikhin · 2026-10-30T03:04:27Z

@haowhsu-quic @shewu-quic @winskuo-quic could you take a look? This changes the Qualcomm backend.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

shoumikhin · 2026-10-30T04:04:54Z

@claude /pr-review

claude · 2026-10-30T04:05:45Z

Claude finished @shoumikhin's task in 20m 42s —— View job

The node name reservation fixes the reported Node redefined name export failures and nothing blocks merging, but the TODO still links no upstream issue, the private-state write has a public alternative, and the new test can pass without exercising the fix.

Detailed review

PR Review: #24316

Summary

QnnPassManager._transform now adds every existing top-level node name to the graph's FX namespace before the passes run. Nodes the passes add therefore can't reuse names that torch.export assigned directly. Two test-only commits also tighten the constant-only partition test.

All findings are minor:

The TODO gives no way to tell when the workaround can be removed.
A public graph rebuild would avoid the private write.
The workaround exists only in the QNN backend.
Nested autocast graphs may not be covered (low confidence).
The new test's guard checks only part of what the test depends on.

This review could not run Python or the QNN tests on this runner. Claims about torch internals come from reading code, not from running it.

Code Quality

backends/qualcomm/_passes/qnn_pass_manager.py:103-109: the TODO links no upstream issue
- "Remove once torch.export records the names it assigns" names no pytorch/pytorch issue, so nothing will signal when the workaround can go.
- The comment is seven lines over a three-line loop, and the first block and the TODO both explain the cause.
- Ask: replace it with one TODO(<issue link>) of two or three lines. Fix this →
qnn_pass_manager.py:110-112: a public API does what the private write does
- torch.fx has no public way to reserve a name. But graph_module.graph = copy.deepcopy(graph_module.graph) rebuilds the graph through Graph.graph_copy → create_node, which records every name.
- This is the same copy that makes ep.module() rename duplicates, as the test comment at test_passes.py:222 notes.
- Why it matters:
  - This is the repo's first write to _graph_namespace._used_names. The two other uses, backends/transforms/fuse_rms_norm.py:375 and fuse_batch_norm_with_conv.py:119, only read it.
  - _transform feeds the annotation, export and preprocess pipelines (lines 474, 485, 524). A torch pin bump that changes _Namespace would break all three at once.
- Caveat: the rebuild replaces every Node object, so no code may hold node references across _transform. This review did not run it. Fix this →
qnn_pass_manager.py:102-113: the workaround exists only in QNN, but the bug comes from torch.export's output
- EnnPassManager._transform (backends/samsung/_passes/enn_pass_manager.py:46-47) is the same as QNN's body before this fix.
- Its transform_for_export_pass (:60-65) runs passes that add nodes (ComputeConstAttrs, DecomposeRoll, DecomposeRemainder). They modify a raw torch.export.export(...) program in place (backends/samsung/utils/export_utils.py:76-77), so the Samsung backend is likely exposed the same way. This review did not reproduce it.
- Ask: add a shared helper in backends/transforms/utils.py and call it from both pass managers. That also leaves one place to delete when the TODO is resolved. Fix this →
Low confidence: qnn_pass_manager.py:110-112 reserves names only in the top-level graph
- How it would fail:
  1. For a disabled autocast region, DecomposeWrapWithAutocast._replace(submod) (decompose_wrap_with_autocast.py:71) inlines nested enabled regions into the submodule's own graph, whose names were never reserved.
  2. graph.erase_node(input_submod) (:90) leaves that submodule attached to the module.
  3. dead_code_elimination_pass (exir/passes/__init__.py:502-505) runs eliminate_dead_code(), which lints, on every GraphModule in modules(). A duplicate name in the submodule raises the same Node redefined name error.
- Condition: this only happens if the pinned torch also assigns names directly inside wrap_with_autocast subgraphs, for example if _name_hoo_subgraph_placeholders handles that HOP. The torch source was not available here to check.
- Fix: reserve names in every GraphModule in graph_module.modules(). Fix this →

Testing

backends/qualcomm/tests/test_passes.py:216-220: the guard against a vacuous test only checks that an input has the colliding name
- The test also relies on two conditions it never asserts:
  - torch.export does not record that name in the namespace;
  - the pipeline still decomposes the autocast HOP or SDPA node, which is what creates the node that would take the name.
- If either changes, both subtests pass without exercising the reservation, and nothing flags the workaround as dead.
- Ask:
  - Before the pipeline runs, assert name not in ep.graph._graph_namespace._used_names. This assertion also starts failing once torch.export records the names, which tells you the TODO can be resolved.
  - After the pipeline, assert that no WrapWithAutocast or aten.scaled_dot_product_attention.default node is left, as lines 181-188 do. Fix this →

Recommendation

Approve

Nothing blocks: the reservation fixes the reported export failures. Link the upstream issue in the TODO before merging. The other changes can come later.

…st keeps the constants on CPU test_partitioner_keeps_constant_only_subgraph_on_cpu only lowered the model and serialized it. It caught the original crash, but it would also pass if the partitioner dropped every partition, so nothing was delegated at all. Check the lowered graph instead. The constant-only part (table * 2 - 1) must stay in the top-level graph, and the rest (the add that reads x) must still go to a delegate. With the partitioner fix removed, the test still fails as before, when building the context binary. With the fix changed to drop every partition, the test now fails on the delegate check, where it used to pass. Ran test_passes.py with the QNN SDK on Linux x86_64.

torch.export names a program's inputs after the user's arguments, but it does not record those names in the graph's namespace. So a QNN pass that adds nodes can give a new node the name of an input, and the pass then fails. With an input named t_1 and an autocast region that calls t() twice, DecomposeWrapWithAutocast copies a second transpose also named t_1. With an input named mul, the attention decomposition creates a second mul: RuntimeError: Node redefined name t_1! RuntimeError: Node redefined name mul! LeRobot's vla_jepa and molmoact2 policies hit this when lowering to QNN. The QNN pass manager now records every existing node name in the graph's namespace before it runs a pipeline's passes, so every pass that adds nodes gets fresh names. torch.fx has no public way to reserve a name, so this adds to the namespace's private name set. test_export_pipeline_keeps_node_names_unique runs the export pipeline on both models and compares with eager. Both cases fail without this change.

…round The QNN pass manager writes the graph namespace's private name set to work around torch.export, which assigns input names, and renames other nodes to make room, without recording them. Say so in a TODO at that loop, so it can be removed once torch.export records the names. test_export_pipeline_keeps_node_names_unique only compared outputs. ep.module() copies the graph and renames duplicate names, so that comparison cannot see a collision. It failed without the fix only because the passes that collide here happen to lint the graph. The test now lints the graph the pipeline returns, and first checks that t_1 and mul really are input names, so the test cannot pass by accident. Without the reservation, the Autocast and Attention cases fail with "Node redefined name". Ran test_passes.py with the QNN SDK on Linux x86_64.

…h stays on CPU test_partitioner_keeps_constant_only_subgraph_on_cpu checked that the mul of table * 2 - 1 stays in the top-level graph and that something is delegated. It would still pass if the sub of that branch were delegated, so the constant-only branch would be split between the CPU and the delegate. Also check that the sub stays in the top-level graph and that the add that reads x does not. Ran test_passes.py with the QNN SDK on Linux x86_64.

…and tighten its test The comment on the node name reservation is now a short TODO that says when it can go and why it writes private torch.fx state. The test now also checks that torch.export leaves the colliding input name unrecorded. Once torch.export records it, the test fails with a message saying the reservation can go, instead of passing without exercising it. It also checks that the autocast and attention decompositions, which add the nodes that would collide, have run.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

shoumikhin · 2026-10-30T14:39:03Z

@claude /pr-review

claude · 2026-10-30T14:39:44Z

Claude finished @shoumikhin's task in 24m 36s —— View job

The node name reservation fixes the reported Node redefined name export failures and nothing blocks merging, but the TODO still links no upstream issue, the new canary assertion would fail CI once torch.export is fixed, and Samsung's pass manager has the same exposure.

Detailed review

PR Review: #24316

Summary

QnnPassManager._transform now adds every top-level node name to the graph's FX namespace before the annotation, export and preprocess pipelines run their passes. New nodes therefore can't reuse a name that torch.export assigned without recording it. Two test-only commits also tighten the constant-only partition test. There are five findings below, all minor; none blocks.

This review ran nothing. Python can't run on this runner and the torch source isn't available, so claims about torch behavior come from reading ExecuTorch code.

Code Quality

backends/qualcomm/_passes/qnn_pass_manager.py:103: the TODO still links no upstream issue
- The workaround can only go once torch.export is fixed, but the TODO, the PR description and the commits link no pytorch/pytorch issue. If nobody reports the bug upstream, the fix may never land and the private write stays.
- Ask: file the issue against torch.export's placeholder naming and turn the comment into TODO(<issue link>). Fix this →
backends/qualcomm/tests/test_passes.py:221-226: the canary assertion will fail CI once torch.export is fixed
- Once torch.export records the names, the reservation does nothing and nothing is broken, yet this assertNotIn fails.
- test-qnn-passes-linux runs on every PR (.github/workflows/pull.yml:1290) as pytest -xvs backends/qualcomm/tests/test_passes.py (:1323).
  - On the torch pin bump that brings the fix, pytest stops at this failure. The file's remaining tests then don't run on the PR most likely to break them.
  - Whoever bumps the pin has to change QNN code to land the bump.
- An earlier automated review asked for this assertion without considering -x.
- Ask: drop it and track removal through the issue linked in the TODO. The lint at :237 is still a valid check after torch is fixed. Fix this →
qnn_pass_manager.py:102-109: the workaround covers only QNN, but Samsung's pass manager has the same exposure
- EnnPassManager._transform (backends/samsung/_passes/enn_pass_manager.py:46-47) is the same code QNN had before this PR.
- backends/samsung/utils/export_utils.py:76-77 passes the raw torch.export.export(...) program to transform_for_export_pass, which runs DecomposeRemainder. That pass creates floor_divide, mul and sub nodes with default names (decompose_remainder.py:92-105), then lints the graph through graph.eliminate_dead_code() (:124).
- Trigger: a model with an integer torch.remainder and an input named mul would likely fail with Node redefined name mul!. This review did not reproduce it.
- Ask: move the reservation into a helper in backends/transforms/utils.py and call it from both pass managers' _transform. Then there is one place to delete when the TODO is resolved. Fix this →
test_passes.py:1019-1027: unrelated partitioner-test changes are bundled in
- Commits 63013ec and 55f83d5 tighten test_partitioner_keeps_constant_only_subgraph_on_cpu, which has nothing to do with node names.
- With "Squash and merge" (CONTRIBUTING.md:444-446), they land under the title "Keep node names unique across QNN passes".
  - That title won't lead anyone to them in git log (:395-396).
  - Reverting the node name fix would revert them too.
- Ask: move them to their own PR.

Testing

test_passes.py:228-234: the check that the decompositions ran doesn't show they had anything to decompose
- Commit e5f40e2 says the test checks that the autocast and attention decompositions ran. It only checks that no WrapWithAutocast or aten.scaled_dot_product_attention.default node is left afterwards, not that one was there before.
- Trigger: suppose torch.export starts inlining autocast regions or decomposing SDPA itself. The passes then add no nodes and nothing can collide, so both subtests pass even with the reservation deleted. The name checks at :218-226 pass as well.
- Ask: before transform_for_export_pipeline, assert that ep.graph has a WrapWithAutocast node (Autocast case) or an SDPA node (Attention case). Fix this →

Recommendation

Approve

Nothing blocks: the reservation fixes the reported failures, and CI is green. Link the upstream issue in the TODO before merging. The other changes can follow.

psiddh · 2026-10-30T23:18:10Z

@@ -100,6 +100,12 @@
 class QnnPassManager(PassManager):

    def _transform(self, graph_module: GraphModule):


transform_for_to_edge_pipeline doesn't call this, so the name reservation doesn't cover it. Should we call the same reservation there as well ? (in a follow up PR)

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

The skip fires when export records the reused name or stops producing it. Say both, in the message and the comment.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

…red helper The reservation repairs what torch.export returns, so it is not specific to QNN. It now lives in backends/transforms/utils.py as reserve_node_names, which QnnPassManager._transform calls. Other backends can call it too, and there is one place to delete once torch.export records the names. No behavior change.

…he test The node name test read the private torch.fx name set to see whether export still leaves the reused names unrecorded. It now runs each case again with reserve_node_names patched out and expects "Node redefined name". If that no longer happens, a torch update removed the collision, and the test skips instead of failing. This checks what the reservation protects against, and the test no longer reads torch.fx internals.

…amsung passes torch.export leaves some node names unrecorded, so a pass that adds nodes can reuse one. With an input named cat, a model that calls torch.cat and then torch.roll failed in DecomposeRoll with a "Node redefined name" error. EnnPassManager._transform now calls reserve_node_names first, the same helper the Qualcomm pass manager uses.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

…ly op The comment said ops that read only constants are decomposed like other CPU ops. Partitions are not known when the filter is built, so the rule also decomposes such ops whose partition stays delegated. The comment and the ops_to_not_decompose docstring now say so. Comment only.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

… call it means to If the patch missed the reserve_node_names call, the run without it would not collide and the test would skip silently. Assert the patched helper was called.

Say plainly that HTP also decomposes ops that read only constants, and that reserve_node_names covers the top-level graph only. Docstrings only.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

shoumikhin · 2026-10-31T19:46:01Z

@claude /pr-review

claude · 2026-10-31T19:46:48Z

Claude finished @shoumikhin's task in 30m 34s —— View job

The node name changes look ready. But on HTP, the new constant-only exemption also covers the group-wise quantize/dequantize ops that QNN has to keep whole, so pre-quantized group-wise weights would likely be decomposed. That should be fixed before merging.

Detailed review

PR Review: #24316

Summary

The PR reserves every existing node name before the QNN and Samsung passes run, using a shared reserve_node_names helper. On HTP, it also decomposes skip-table ops that read only constants. The reviewers found no problems in the node name changes. The HTP change has three:

The exemption reaches the group-wise quantize/dequantize ops.
It also stops the post-lowering check from reporting constant-only ops left on CPU.
No test of it runs in CI.

This review ran no code. Python and the QNN SDK aren't available on this runner, and torch isn't installed. What it says about torch's decomposition table comes from ExecuTorch code, not torch source.

Backward Compatibility

backends/qualcomm/partition/qnn_partitioner.py:413-425: on HTP, group-wise weight quantize/dequantize ops are now decomposed
- Scope: the exemption applies to every target in get_skip_decomp_table(). That list includes quantized_decomposed.quantize_per_channel_group and dequantize_per_channel_group (partition/utils.py:271-272).
- Why they count as constant-only: a weight dequantize reads the frozen weight, the scales and the zero points. is_parameter (builders/utils.py:39-47) treats all three as constants, so the node goes into constant_only and the lambda returns False for it.
- What changed: before this PR, filter_fn returned True for these nodes, because all their dtypes are in QNN_TENSOR_TYPE_MAP.
- How it gets decomposed:
  1. With False, _replace_aten_ops_with_transformed_ops doesn't swap the node for its preserved form (exir/program/_program.py:921-929).
  2. The second run_decompositions(_default_decomposition_table()) (:1250) then decomposes it.
  3. That table pops only _QUANT_PRIMITIVES (exir/tracer.py:671-674), and the two group-wise ops aren't in that list (exir/operator/util.py:47-66).
- Result: QNN gets the decomposed ops instead of the dequantize it builds the per-block encoding from (builders/node_visitor.py:86-91). The PER_CHANNEL_GROUP_ENCODING arch gate (qnn_partitioner.py:143-157) then has no node to check.
- Trigger: a model with pre-quantized group-wise weights, lowered for HTP with to_edge_transform_and_lower_to_qnn. The partitioner comment at :143-145 names this as a supported input. LPBQ from the QNN quantizer is not affected: it emits torchao affine ops, which are in _QUANT_PRIMITIVES.
- Confidence: the ExecuTorch path above is confirmed in the code. That torch's default table decomposes these two ops was inferred, not checked: they are custom ops with a CompositeImplicitAutograd implementation, which is why QNN lists them in its skip table.
- Fix: in the lambda, keep the skip table's quantize/dequantize targets whole even when they are constant-only. Still propagate constant_only through them, so the ops after them are decomposed. Fix this →

Code Quality

qnn_partitioner.py:422-425: the post-lowering check no longer reports constant-only ops left on CPU
- Mechanism (confirmed): after lowering, to_edge_transform_and_lower calls ops_to_not_decompose again on the lowered program. It passes the filter to _sanity_check_graph_for_non_decomp_ops (_program.py:1418-1436, :1019-1033). The lambda is recomputed on that graph. So a kept-whole op that is still on CPU and reads only constants no longer raises was not decomposed or delegated.
- How such an op can remain (low confidence): RecomposeRmsNorm and RecomposePixelUnshuffle are active by default (qnn_pass_manager.py:164-165). They run as transform_passes (utils/utils.py:525, _program.py:1397-1398) after decomposition and before partitioning. Both rebuild skip-table ops (partition/utils.py:255, 259).
- Trigger (not reproduced): the test model, with F.rms_norm(self.table, [16], self.w) in place of linear, lowered for HTP. If the patterns at recompose_rms_norm.py:100-105 match torch's decomposition of rms_norm:
  1. The PR decomposes rms_norm, and RecomposeRmsNorm rebuilds it.
  2. Its partition is dropped.
  3. Lowering succeeds with an undecomposed rms_norm on CPU. Before this PR, lowering failed here with a clear error.
- Ask: if the patterns match, one option is to make the two recompose passes skip matches whose inputs are all constants on HTP. Fix this →

Testing

backends/qualcomm/tests/test_passes.py:1011-1049: no test of the HTP filter runs in CI, and the existing test checks only part of it
- Not run in CI: this is the only test that reaches the new branch. It skips without the QNN SDK, and the PR description says CI doesn't have the SDK.
- The SDK isn't needed to test the filter: QnnPartitioner.__init__ only builds options from the compile specs (qnn_partitioner.py:256-259), and the test already builds those outside the try. So QnnPartitioner(compiler_specs).ops_to_not_decompose(torch.export.export(...)) can be tested directly.
- Gaps in the existing test:
  - It never checks that linear is gone. Because of the previous finding, an undecomposed constant-only linear left on CPU no longer fails lowering, and to_executorch() doesn't check for CPU kernels.
  - It never checks that ops reading a runtime input are still kept whole. A filter that decomposed everything on HTP would pass.
- Ask:
  - Add an SDK-free test of the filter. It should be False for linear(self.table, self.weight) and True for linear(x, self.weight). After the fix above, it should also be True for a constant-only dequantize_per_channel_group.
  - In the existing test, add assertNotIn(exir_ops.edge.aten.linear.default, top_level_ops).
  - Fix this →

Recommendation

Request Changes

On HTP, the constant-only exemption likely breaks models with pre-quantized group-wise weights. Fixing it takes a small change to the lambda and a test that runs in CI.

… HTP The HTP rule that decomposes ops reading only constants also covered the quantize and dequantize ops in the skip table, such as dequantize_per_channel_group on a frozen weight. QNN builds the weight encoding from those ops. With today's torch they are not decomposed anyway, so nothing changed in practice, but the filter should not ask for it. They are now kept whole, and ops after them still count as reading only constants. A new test checks the filter itself and needs no QNN SDK, so it runs in CI: a constant-only linear is decomposed, a linear on an input and a constant-only group-wise dequantize are kept whole. The constant-only partition test also checks that no linear is left in the top-level graph.

After lowering, exir asks the partitioner for its filter again and reports any kept-whole op still on the CPU. On HTP the filter now let constant-only ops through, so that report went quiet. For example RecomposeRmsNorm rebuilt a constant-only rms_norm that then stayed on the CPU, and the program failed later in to_executorch with "Missing out variants" instead of the clear error from before. On the lowered (edge) program the filter is plain filter_fn again, so these cases report as they did before the HTP change. The filter test checks this too.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

shoumikhin · 2026-10-31T21:28:33Z

Thanks, all three are done.

The test no longer reads private torch.fx state. Each case now runs again with reserve_node_names patched out and expects Node redefined name. If a torch update removes the collision, the test skips instead of failing and does not stop the later tests.
The reservation is now reserve_node_names in backends/transforms/utils.py, and the Samsung pass manager calls it too. A model with an input named cat that calls torch.cat and then torch.roll failed in Samsung's DecomposeRoll before, and passes now.
I kept the partition test changes here, and the description covers them, so the squash message does too.

I could not find an upstream PyTorch issue for the export naming bug yet.

shoumikhin · 2026-10-31T21:28:35Z

Thanks, all three are addressed.

Group-wise quantize and dequantize ops: I ran it. With today's torch, a constant-only dequantize_per_channel_group (and torchao dequantize_affine) is not decomposed and still reaches QNN whole. The filter now keeps those ops whole anyway, so it no longer asks for it.
The post-lowering check: confirmed. A constant-only rms_norm was rebuilt by RecomposeRmsNorm and stayed on the CPU without a report. On the lowered program the filter is plain filter_fn again, so this reports as before.
A new filter test needs no SDK and runs in CI. The partition test also checks that no linear is left.

…rt issue reserve_node_names works around torch.export not recording some of the node names it assigns. Its TODO now links the upstream issue that tracks the fix, pytorch/pytorch#202927. The node name test's skip message, which shows once a torch update removes the collisions, links the issue too. Comment and message only.

Copilot

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

shoumikhin added the release notes: qualcomm Changes to the Qualcomm backend delegate label Oct 30, 2026

Copilot AI balanced review requested due to automatic review settings October 30, 2026 02:51

shoumikhin requested a review from psiddh as a code owner October 30, 2026 02:51

shoumikhin added the release notes: qualcomm Changes to the Qualcomm backend delegate label Oct 30, 2026

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 30, 2026

Copilot AI reviewed Oct 30, 2026

shoumikhin mentioned this pull request Oct 30, 2026

[v1.6.0] Release Schedule and Tracker #23608

Open

Copilot AI balanced review requested due to automatic review settings October 30, 2026 03:52

Copilot AI reviewed Oct 30, 2026

shoumikhin force-pushed the qnn-node-names branch from 3e91127 to b84aa23 Compare October 30, 2026 04:04

Copilot AI balanced review requested due to automatic review settings October 30, 2026 04:04

Copilot AI reviewed Oct 30, 2026

shoumikhin added 5 commits October 29, 2026 23:30

shoumikhin force-pushed the qnn-node-names branch from b84aa23 to e5f40e2 Compare October 30, 2026 07:17

Copilot AI balanced review requested due to automatic review settings October 30, 2026 07:17

Copilot AI reviewed Oct 30, 2026

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

psiddh reviewed Oct 30, 2026

View reviewed changes

Copilot AI reviewed Oct 31, 2026

Qualcomm AI Engine Direct - Name both triggers in the node name skip

40cab1c

The skip fires when export records the reused name or stops producing it. Say both, in the message and the comment.

Copilot AI balanced review requested due to automatic review settings October 31, 2026 17:12

Copilot AI reviewed Oct 31, 2026

shoumikhin added 3 commits October 31, 2026 10:39

Copilot AI balanced review requested due to automatic review settings October 31, 2026 17:51

Copilot AI reviewed Oct 31, 2026

shoumikhin changed the title ~~Qualcomm AI Engine Direct - Keep node names unique across QNN passes, and decompose constant-only ops on HTP~~ Qualcomm AI Engine Direct - Keep node names unique across the QNN and Samsung passes, and decompose constant-only ops on HTP Oct 31, 2026

Copilot AI balanced review requested due to automatic review settings October 31, 2026 17:57

Copilot AI reviewed Oct 31, 2026

shoumikhin mentioned this pull request Oct 31, 2026

Fix crashes, wrong outputs and lowering failures in several backends and in exir #24298

Open

shoumikhin added 2 commits October 31, 2026 11:54

Qualcomm AI Engine Direct - Check that the node name test patches the…

0e6bba3

… call it means to If the patch missed the reserve_node_names call, the run without it would not collide and the test would skip silently. Assert the patched helper was called.

Qualcomm AI Engine Direct - Clarify two docstrings

c53d2fe

Say plainly that HTP also decomposes ops that read only constants, and that reserve_node_names covers the top-level graph only. Docstrings only.

Copilot AI balanced review requested due to automatic review settings October 31, 2026 18:56

Copilot AI reviewed Oct 31, 2026

shoumikhin added 2 commits October 31, 2026 13:37

Copilot AI balanced review requested due to automatic review settings October 31, 2026 20:38

Copilot AI reviewed Oct 31, 2026

shoumikhin mentioned this pull request Nov 1, 2026

torch.export does not record the node names it assigns, so a later pass can create a node with a name that is already used pytorch/pytorch#202927

Open

Copilot AI balanced review requested due to automatic review settings November 1, 2026 06:28

Copilot AI reviewed Nov 1, 2026

		@@ -100,6 +100,12 @@
		class QnnPassManager(PassManager):

		def _transform(self, graph_module: GraphModule):

Conversation

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

Uh oh!

What is wrong today

What this change does

Tests

Uh oh!

pytorch-bot Bot commented Oct 30, 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/24316

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

shoumikhin commented Oct 30, 2026

Uh oh!

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

Uh oh!

PR Review: #24316

Summary

Code Quality

Testing

Recommendation

Uh oh!

shoumikhin commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

shoumikhin commented Oct 30, 2026

Uh oh!

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

Uh oh!

PR Review: #24316

Summary

Code Quality

Testing

Recommendation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

shoumikhin commented Oct 30, 2026

Uh oh!

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

Uh oh!

PR Review: #24316

Summary

Code Quality

Testing

Recommendation

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

shoumikhin commented Oct 31, 2026

Uh oh!

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

Uh oh!

shoumikhin commented Oct 30, 2026 •

edited

Loading

pytorch-bot Bot commented Oct 30, 2026 •

edited

Loading

claude Bot commented Oct 30, 2026 •

edited

Loading

claude Bot commented Oct 30, 2026 •

edited

Loading

claude Bot commented Oct 30, 2026 •

edited

Loading

psiddh Oct 30, 2026 •

edited

Loading

claude Bot commented Oct 31, 2026 •

edited

Loading