Repository navigation
Commit a3bf556
authored
Treat aten.slice.Tensor as a shared-observer op in XNNPACKQuantizer (pytorch#21568)
### Summary
`_is_share_obs_or_fq_op` lists `aten.slice_copy.Tensor` but not
`aten.slice.Tensor`.
The quantizer runs before `to_edge()`, so the op it actually sees is
`aten.slice.Tensor`, the `_copy` entry never matches, and
`propagate_annotation`
does not give the slice its producer's `SharedQuantizationSpec`.
The other data-movement ops in that list are registered under both names
(`permute.default`/`permute_copy.default`,
`squeeze.dim`/`squeeze_copy.dim`,
`view.default`/`view_copy.default`); slice only had the `_copy` one.
`slice` has
no `.default` overload, which is why the pre-functionalization name is
`slice.Tensor` and not `slice_copy.default`.
Without the shared spec, the slice input and output end up with
different quant
params. XNNPACK's `static_slice` requires them to be equal, so the model
partitions and lowers fine and then fails at runtime init:
```
[XNNCompiler.cpp:1421] Failed to create static slice node 377 with code: xnn_status_invalid_parameter
[XNNPACKBackend.cpp:119] XNNCompiler::compileModel failed: 0x1
[method.cpp:110] Init failed for backend XnnpackBackend: 0x1
```
I hit this quantizing a Gemma-family embedding model with
`XNNPACKQuantizer`
(per-channel W8A8 static), where all 96 slices from the RoPE
`rotate_half`
pattern were left without quant params.
`_test_qs8_slice_copy` was already disabled for this ("T156004676 -
slice isn't
propagated"), and its comment names the same cause, so I re-enabled it
here.
### Test plan
```
pytest backends/xnnpack/test/ops/test_slice_copy.py -k qs8
```
The test runs end to end now: quantize, lower, serialize, run and
compare
outputs. Reverting the one-line change makes it fail with 2
`quantize_per_tensor` nodes instead of 5.
Two assertions in it needed updating first. `check_node_count` takes op
overloads rather than the `"aten::slice.Tensor"` strings the test was
written
with, so it reported `Expected 3 aten::slice.Tensor nodes but found 0`
on a
graph that did contain them. And the expected `quantize_per_tensor`
count is 5,
not 3, since the three slices are now quantized along with the input and
the add
output.
`lintrunner -m upstream/main` is clean.
cc @GregoryComer @digantdesai @cbilgin @JakeStevens1 parent 52b176f commit a3bf556
2 files changed
Lines changed: 7 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1102 | 1102 | | |
1103 | 1103 | | |
1104 | 1104 | | |
| 1105 | + | |
1105 | 1106 | | |
1106 | 1107 | | |
1107 | 1108 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
140 | 140 | | |
141 | 141 | | |
142 | 142 | | |
143 | | - | |
144 | | - | |
145 | | - | |
146 | | - | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
147 | 146 | | |
148 | 147 | | |
149 | 148 | | |
| |||
157 | 156 | | |
158 | 157 | | |
159 | 158 | | |
160 | | - | |
161 | | - | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
162 | 162 | | |
163 | 163 | | |
164 | 164 | | |
| |||
0 commit comments