Repository navigation
Lower bf16 mean.dim to XNNPACK as an fp32 static mean - #24746
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/24746
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit fedc378 with merge base b3e2457 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@claude review effort=max |
|
Claude finished @JakeStevens's task in 24m 26s —— View job Nothing blocks this PR, but two things should be settled before merge: whether bf16 means carry their converts in the serialized program or get them from the runtime (as bf16 matmul does), and a bf16 PR Review: #24746SummaryWith Coverage: three independent reviewers went through all 13 files. Tests could not be run here. The XNNPACK submodule is not checked out, so XNNPACK-side behaviour (bf16 convert and transpose support) is unverified. API Design
Testing
Performance
Code Quality
Specific Comments
RecommendationNeeds Discussion
|
28c20a9 to
b3d74b7
Compare
|
Thanks for the review. Replies to the review above, as of b3d74b7:
Authored with AI using Claude Code. |
b3d74b7 to
e5c4836
Compare
## Ulterior Motive Keep bf16 RMSNorm in XNNPACK for bf16 models, such as the bf16 LeRobot policies in pytorch#24667, where bf16 ops that XNNPACK does not support fall back to the portable kernels. ## Rationale **What**: With `enable_bf16=True`, lower a bf16 `mean.dim` to the `XNNStaticMean` from pytorch#24741. The serialized graph stays bf16; when the program loads, the runtime reduces in fp32 between a convert from bf16 and a convert back, as it already does for bf16 matmul. **Why**: XNNPACK has no bf16 reduction, so `MeanDimConfig` rejected every bf16 mean, and a bf16 RMSNorm split the delegate around its mean. XNNPACK does convert between bf16 and fp32, and when the mean has lowered neighbours the two converts cost less than leaving the mean outside the delegate. ## Details - `defineGenericReduceNode` adds the converts when a serialized value is bf16, the same way `defineSoftmaxNode` does since pytorch#24805. It reuses that change's helpers: `convertIfDistinct`, and `defineBf16SoftmaxFp32Value`, renamed `defineFp32ValueForBf16` now that reductions use it too. Because the program does not encode the converts, a runtime can switch to a native bf16 reduction if XNNPACK adds one, without re-exporting. The XNNPACK subgraph is the same as with the converts serialized: outputs are bitwise identical and latency is within noise. Each bf16 mean adds an fp32 copy of its input to the XNNPACK workspace (1 MiB for 1x128x2048). - A bf16 mean never takes the Global Average Pooling path, which has no bf16 kernel. A 4D mean over dims [2, 3] with `keepdim=True` is a static mean like any other bf16 mean, so a channels-last input is copied to NCHW for it. - bf16 means are lowered at any size, as fp16 and fp32 means are in pytorch#24741. fp32 and fp16 programs are byte-identical to main. - Latency, branch time over main time (below 1 is faster), each a median over at least 1000 timed runs per program, alternating main and branch programs on one runtime, on an Apple M-series CPU (12 performance + 4 efficiency cores) with the default 16 threads: bf16 mean over the last dim of 1x128x2048 0.42x, bf16 RMSNorm 1x128x2048 0.56x, decode RMSNorm 1x1x2048 0.80x, 4 residual blocks of `x + Linear(RMSNorm(x))` at width 2048 0.87x for prefill (64 tokens) and 0.98x for decode (1 token). bf16 RMSNorm on 13 static shapes from 1x1x128 to 1x32x2048 is 0.47x to 0.96x at 1 thread. At 4 threads the shapes under 8K elements with several outputs are 1.19x to 1.57x (1.6 to 3.0 us more) and the rest 0.57x to 0.96x; at 16 threads those small shapes are 3.8x to 5.4x (22 to 24 us more) and the rest 0.70x to 0.99x, the same threadpool cost as for fp32 in pytorch#24741. - A mean alone in its delegate, with nothing else lowered around it, pays the threadpool wake-up on its own and can be slower than the portable kernel at high thread counts. At 1x128x128 and 16 threads, a lone fp32 mean is 3.2x (as in pytorch#24741) and a lone bf16 mean 5.7x, because of its two converts; at 4 threads the bf16 mean is 1.35x, and at 1 thread it is faster. Any op lowered alone pays this, such as `add` (2.95x at 1x128x128 and 16 threads on main). - Accuracy against fp32 eager is unchanged for single ops. It improves when the means were the only places a model's delegate split: for the 4 blocks above at prefill, max abs error drops from 4.6e-2 to 1.8e-2, because fewer intermediates are rounded to bf16 at delegate boundaries. - An RMSNorm that casts to fp32 and back, as Hugging Face Llama does, already has an fp32 mean. Its remaining cost is the bf16 <-> fp32 casts, which pytorch#24745 lowers for programs exported with dim order, and pytorch#24768 for programs exported with `skip_dim_order`. - `test_enable_bf16_leaves_ops_xnnpack_cannot_define_in_bf16` no longer lists `mean`, which is now delegated in bf16. Test plan: ``` python -m pytest backends/xnnpack/test/ops/test_mean_dim.py python -m pytest backends/xnnpack/test/ops backends/xnnpack/test/passes backends/xnnpack/test/test_xnnpack_partitioner.py backends/xnnpack/test/serialization backends/xnnpack/test/quantizer ``` Authored with AI using Claude Code.
e5c4836 to
fedc378
Compare
Ulterior Motive
Keep bf16 RMSNorm in XNNPACK for bf16 models, such as the bf16 LeRobot policies in #24667, where bf16 ops that XNNPACK does not support fall back to the portable kernels.
Rationale
What: With
enable_bf16=True, lower a bf16mean.dimto theXNNStaticMeanfrom #24741. The serialized graph stays bf16; when the program loads, the runtime reduces in fp32 between a convert from bf16 and a convert back, as it already does for bf16 matmul.Why: XNNPACK has no bf16 reduction, so
MeanDimConfigrejected every bf16 mean, and a bf16 RMSNorm split the delegate around its mean. XNNPACK does convert between bf16 and fp32, and when the mean has lowered neighbours the two converts cost less than leaving the mean outside the delegate.Details
defineGenericReduceNodeadds the converts when a serialized value is bf16, the same waydefineSoftmaxNodedoes since XNNPACK: delegate bf16 softmax, computing in fp32 #24805. It reuses that change's helpers:convertIfDistinct, anddefineBf16SoftmaxFp32Value, renameddefineFp32ValueForBf16now that reductions use it too. Because the program does not encode the converts, a runtime can switch to a native bf16 reduction if XNNPACK adds one, without re-exporting. The XNNPACK subgraph is the same as with the converts serialized: outputs are bitwise identical and latency is within noise. Each bf16 mean adds an fp32 copy of its input to the XNNPACK workspace (1 MiB for 1x128x2048).keepdim=Trueis a static mean like any other bf16 mean, so a channels-last input is copied to NCHW for it.x + Linear(RMSNorm(x))at width 2048 0.87x for prefill (64 tokens) and 0.98x for decode (1 token). bf16 RMSNorm on 13 static shapes from 1x1x128 to 1x32x2048 is 0.47x to 0.96x at 1 thread. At 4 threads the shapes under 8K elements with several outputs are 1.19x to 1.57x (1.6 to 3.0 us more) and the rest 0.57x to 0.96x; at 16 threads those small shapes are 3.8x to 5.4x (22 to 24 us more) and the rest 0.70x to 0.99x, the same threadpool cost as for fp32 in Lower mean.dim beyond 4D pooling as an XNNPACK static mean #24741.add(2.95x at 1x128x128 and 16 threads on main).skip_dim_order.test_enable_bf16_leaves_ops_xnnpack_cannot_define_in_bf16no longer listsmean, which is now delegated in bf16.Test plan:
Authored with AI using Claude Code.