Cortex-M: lower subtraction, and stop miscompiling add with alpha - #22153
Cortex-M: lower subtraction, and stop miscompiling add with alpha#22153rascani wants to merge 1 commit into
Conversation
quantized_add carries a multiplier per operand, so subtraction is the same kernel with the second one negated. quantize_multiplier_aot always lands in [2^30, 2^31), so the negation cannot saturate; the first requantize stage rounds asymmetrically under negation, but the second absorbs it and the int8 output is identical across 400 scale and zero-point configurations. This exposed an existing bug. alpha scales the second operand, quantized_add has nowhere to put it, and nothing checked, so torch.add(x, y, alpha=2) computed x + y and returned wrong values silently. The quantizer now declines those nodes and they stay fp32. It has to be the quantizer: by the time the lowering runs FoldAndAnnotateQParamsPass has removed the dq nodes, so declining there would leave an fp32 add over raw int8. The lowering raises instead. Subtraction also makes an existing overflow reachable, where the kernel's int32 left shift wraps and saturates to the opposite rail. No multiplier split avoids it -- shrinking max_scale_2x buys a shift and costs the same headroom -- so the lowering refuses the configuration, which needs operands that nearly cancel. Two gaps left open: ActivationFusionPass does not fold relu into sub, and rsub with alpha is still wrong because Arm's pass drops alpha before annotation. Authored with Claude Code.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22153
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit e668893 with merge base baafd7e ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
This PR needs a
|
AdrianLundell
left a comment
AdrianLundell
left a comment
There was a problem hiding this comment.
LGTM, but why not address the two gaps in the commit message right away? And could alpha not be folded into the qparams as well at least as long as it isn't too big?
2 participants
Summary
quantized_add carries a multiplier per operand, so subtraction is the same kernel with the second one negated. quantize_multiplier_aot always lands in [2^30, 2^31), so the negation cannot saturate; the first requantize stage rounds asymmetrically under negation, but the second absorbs it and the int8 output is identical across 400 scale and zero-point configurations.
This exposed an existing bug. alpha scales the second operand, quantized_add has nowhere to put it, and nothing checked, so torch.add(x, y, alpha=2) computed x + y and returned wrong values silently. The quantizer now declines those nodes and they stay fp32. It has to be the quantizer: by the time the lowering runs FoldAndAnnotateQParamsPass has removed the dq nodes, so declining there would leave an fp32 add over raw int8. The lowering raises instead.
Subtraction also makes an existing overflow reachable, where the kernel's int32 left shift wraps and saturates to the opposite rail. No multiplier split avoids it - shrinking max_scale_2x buys a shift and costs the same headroom - so the lowering refuses the configuration, which needs operands that nearly cancel.
Two gaps left open: ActivationFusionPass does not fold relu into sub, and rsub with alpha is still wrong because pass drops alpha before annotation.
Authored with Claude Code.