Commit ae227c2
authored
[Qualcomm] Scope ExpandBroadcastTensorShape rewrite to the broadcast node (#21583)
# [Qualcomm] Scope `ExpandBroadcastTensorShape` rewrite to the broadcast
node (fix rank-0 mutable-buffer `to_executorch()` failure)
> Draft for discussion with the Qualcomm backend team before opening.
Standalone: one file, +6/-3.
## Summary
`ExpandBroadcastTensorShape` reshapes a lower-rank broadcast input up to
the broadcast op's output
rank so `LayoutTransform` sees equal ranks. It redirected **every** user
of that input to the
rank-promoted view, instead of only the broadcast node it is processing.
This over-broad rewrite
breaks any *other* consumer of the same tensor — most visibly an
in-place mutation of a rank-0 user
input.
## Symptom
A model that mutates a rank-0 tensor **input** in place (a scalar
counter, `counter += N`) fails at
`to_executorch()` on the QNN path:
```
RuntimeError: expand: the requested shape has too few dimensions ...
at exir/passes/spec_prop_pass.py -> meta_copy_ -> expand_copy
```
XNNPACK / CoreML / generic `to_edge` are unaffected —
`ExpandBroadcastTensorShape` is QNN-only, so
only the QNN lowering produces the rank-mismatched graph.
## Root cause
The scalar counter feeds two things: (1) a broadcast `add` (`arange(N) +
counter`) and (2) the
in-place `counter.add_`. The pass promotes `counter` to `(1,)` for the
broadcast, then redirects
**all** users to the `(1,)` view — including the mutation. ExecuTorch
records the mutation as a
`USER_INPUT_MUTATION`, so at the end it writes the value back into the
input buffer via
`copy_(counter, new_value)`. That write-back is now `copy_(() <- (1,))`;
`SpecPropPass` re-traces it
and `expand_copy` cannot shrink `(1,)` into `()` → the error above.
Confirmed by graph dump: input placeholder `counter` = `()` vs the
`USER_INPUT_MUTATION` output =
`(1,)`; toggling this pass off makes both rank-0 and rank-1 models lower
cleanly.
## Fix
Redirect only the current broadcast node's input, not every user of
`arg`:
```diff
- users = list(arg.users.keys())
reshape_node = graph_module.graph.create_node( ... )
...
- for user in users:
- user.replace_input_with(arg, reshape_node)
+ # Redirect ONLY the current broadcast node's input to the reshaped
+ # view, not every user of `arg`.
+ node.replace_input_with(arg, reshape_node)
```
The mutation and the input placeholder both stay rank-0 → write-back
stays `copy_(() <- ())`. This
also fixes a latent correctness bug: blanket replacement is wrong when
one tensor feeds multiple
broadcasts that require different output ranks. HTP's no-rank-0
requirement is still satisfied by the
runtime `dims=[1]` promotion in `builders/node_visitor.py` (blob level),
independent of this
edge-graph rewrite.
## Reproduction
```python
import torch
from executorch.backends.qualcomm.serialization.qc_schema import QcomChipset
from executorch.backends.qualcomm.utils.utils import (
generate_qnn_executorch_compiler_spec, generate_htp_compiler_spec,
to_edge_transform_and_lower_to_qnn,
)
class CounterModel(torch.nn.Module):
def __init__(self):
super().__init__()
self.register_buffer("counter", torch.zeros((), dtype=torch.long)) # rank-0
def forward(self, x):
self.counter.add_(x.shape[-1]) # in-place mutation of rank-0 input
return x + self.counter # broadcast add -> triggers the pass
m = CounterModel().eval()
ex = (torch.randn(4),)
ep = torch.export.export(m, ex)
backend = generate_htp_compiler_spec(use_fp16=True)
specs = generate_qnn_executorch_compiler_spec(soc_model=QcomChipset.SM8650, backend_options=backend)
edge = to_edge_transform_and_lower_to_qnn(ep, ex, specs)
edge.to_executorch() # BEFORE: RuntimeError (expand: too few dimensions); AFTER: OK
```
## Testing
- Repro above: rank-0 `to_executorch()` **FAILED → OK**; rank-1 counter
still OK.
- Stress cases (all pass): one operand feeding two broadcasts of
different rank (each gets its own
correctly-shaped `view_copy`); non-mutated rank-0 broadcast;
tensor-valued increment. Negative control
(revert to the all-users redirect) reproduces the crash → change is
necessary and sufficient.
- **TODO before merge:** graph-level unit test, a real-model QNN
regression export, `lintrunner`.
## Questions for the Qualcomm team
1. Was the all-users rewrite intentional for any case (e.g. a shared
reshape feeding several
broadcasts), or is scoping to the single node always correct? I believe
single-node is correct and
strictly safer, but want your read.
2. The original all-users redirect also silently rebased
**non-broadcast** consumers (e.g. a `relu`) of
the same operand onto the rank-promoted view — a latent aliasing bug
this scoping also fixes. Confirm
that was never relied upon. (A rank>0 value written back into a rank-0
input is a separate,
model-level mismatch no operand fix addresses — out of scope here.)
3. Relation to the transformers ExecuTorch-exporter QNN work:
`StaticLayer.cumulative_length` is
exactly this rank-0-counter pattern, so this is a real blocker for that
flow.
cc @cbilgin1 parent 812c7f0 commit ae227c2
3 files changed
Lines changed: 208 additions & 25 deletions
Lines changed: 39 additions & 25 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
30 | | - | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
31 | 33 | | |
32 | 34 | | |
33 | 35 | | |
| |||
36 | 38 | | |
37 | 39 | | |
38 | 40 | | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | | - | |
43 | | - | |
44 | | - | |
45 | | - | |
46 | | - | |
47 | | - | |
48 | | - | |
49 | | - | |
50 | | - | |
51 | | - | |
52 | | - | |
53 | | - | |
54 | | - | |
55 | | - | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
62 | 76 | | |
63 | 77 | | |
64 | | - | |
| 78 | + | |
65 | 79 | | |
66 | 80 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3049 | 3049 | | |
3050 | 3050 | | |
3051 | 3051 | | |
| 3052 | + | |
| 3053 | + | |
| 3054 | + | |
| 3055 | + | |
| 3056 | + | |
| 3057 | + | |
| 3058 | + | |
| 3059 | + | |
| 3060 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
| 27 | + | |
26 | 28 | | |
27 | 29 | | |
28 | 30 | | |
| |||
35 | 37 | | |
36 | 38 | | |
37 | 39 | | |
| 40 | + | |
38 | 41 | | |
39 | 42 | | |
40 | 43 | | |
| |||
470 | 473 | | |
471 | 474 | | |
472 | 475 | | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
473 | 633 | | |
474 | 634 | | |
475 | 635 | | |
0 commit comments