Repository navigation
Fix NaN and Inf division by zero on EMU128 and SCALAR - #3511
Open
reginaldalfret wants to merge 1 commit into
Open
reginaldalfret wants to merge 1 commit into
reginaldalfret wants to merge 1 commit into
Conversation
In commit 2ed1121, division by zero was handled using ScalarCopySign(inf, a * den). This had two issues: 1. When a is NaN, a == T{0} evaluates to false, causing ScalarCopySign(inf, ...) to convert NaN into +/-Inf instead of preserving NaN. 2. When a is +/-Inf, a * den is (+/-Inf) * 0 which yields NaN. ScalarCopySign then extracted an unexpected sign from the NaN, returning -Inf for Inf / +0. Fix by checking ScalarIsNaN(a) || a == 0 to return NaN without division by zero, and computing the quotient sign via ScalarSignBit(a) ^ ScalarSignBit(den). Add regression tests in TestFloatDiv for +/-Inf / +-0 and +/-NaN / +-0. Signed-off-by: reginaldalfret <reginaldalfret@gmail.com>
jan-wassenberg
approved these changes
Oct 26, 2026
jan-wassenberg
left a comment
jan-wassenberg
left a comment
Member
There was a problem hiding this comment.
Lots of corner cases here. Thanks for fixing.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In commit 2ed1121 ("emu128, scalar: return +/-Inf on division by zero"), floating-point division by zero was rewritten to avoid division by zero sanitizers:
a.raw = (a.raw == T{0}) ? (inf * T{0}) : ScalarCopySign(inf, a.raw * den);This introduced two edge-case defects on EMU128 and SCALAR:
aisNaN,a == T{0}evaluates tofalse, causingScalarCopySign(inf, NaN * 0)to convertNaNinto+Infor-Infinstead of preservingNaN.ais+Inf,a * denevaluates toInf * 0 == NaN.ScalarCopySignthen extracted an invalid sign bit from the generatedNaN, returning-Inffor+Inf / +0(and-Inffor-Inf / -0).Solution
ScalarIsNaN(a) || a == 0to produceNaNviaa + (inf * 0)without dividing by zero.ScalarSignBit(a) ^ ScalarSignBit(den)and returnsign ? -inf : inf.TestFloatDivinhwy/tests/div_test.ccto test+Inf / +-0,-Inf / +-0,+NaN / +-0, and-NaN / +-0.Verification
Abort at div_test.cc:241: Assert quot(pos_inf, zero) == pos_inf.HwyDivTestGroup/HwyDivTestpass across all targets (AVX3_DL, AVX3, AVX2, SSE4, SSSE3, SSE2, EMU128).HWY_COMPILE_ONLY_SCALARandHWY_COMPILE_ONLY_EMU128test binaries both pass all tests.git diff --checkpassed cleanly.