Skip to content

contrib/math: preserve negative zero in Expm1 - #3273

Open
EylonKrause wants to merge 1 commit into
google:masterfrom
EylonKrause:fix/expm1-negative-zero-x
Open

contrib/math: preserve negative zero in Expm1#3273
EylonKrause wants to merge 1 commit into
google:masterfrom
EylonKrause:fix/expm1-negative-zero-x

Conversation

@EylonKrause

Copy link
Copy Markdown
Contributor

Summary

Expm1(-0.0) returns +0.0 instead of -0.0, losing the sign of zero. C11 Annex F (F.10.3.3) requires expm1(±0) = ±0, and scalar std::expm1 complies — so this is a signed-zero correctness bug (observable via std::signbit, or 1.0 / expm1(-0.0) giving +inf instead of -inf).

Cause

On the |x| < ln2/2 polynomial path, ExpPoly computes MulAdd(poly, x*x, x). For x = -0.0 this is 0.5 * (+0.0) + (-0.0), and IEEE addition of oppositely-signed zeros yields +0.0, so the sign bit is dropped. Unlike the odd functions in this file (Sin, Tan, Sinh, …), Expm1 never reapplies the sign of x, so the negative zero is not restored.

Fix

Reapply x's sign on the polynomial path using the sign-bit mask already defined in the function:

const V z = IfThenElse(Lt(Abs(x), kLn2Over2), Or(y, And(x, kNegZero)),
                       Sub(impl.LoadExpShortRange(d, Add(y, kOne), q), kOne));

expm1(x) shares the sign of x for all x, so OR-ing in x's sign bit is a no-op for every input except x == -0.0, which it corrects to -0.0. +0.0, small nonzero x, the large-|x| branch, and the -1 clamp are unchanged, on every SIMD target.

Testing

  • Added TestExpm1SignedZero, a bit-exact check that Expm1(±0) returns ±0 — the existing ULP comparator treats +0 == -0 and cannot detect this (which is why it went unnoticed).
  • Before the fix the new test fails (Expm1(-0) yields +0); after, it passes.
  • Full math_test passes (104 tests) across every enabled SIMD target.

Disclosure: this contribution was authored with an AI coding assistant (Claude) and reviewed before submission.

Expm1(-0.0) returned +0.0 instead of -0.0. On the |x| < ln2/2 polynomial
path, ExpPoly forms 0.5 * (+0) + (-0) = +0, dropping the sign bit, and
Expm1 -- unlike the odd functions in this file -- never reapplies the
sign of x. C11 Annex F (F.10.3.3) requires expm1(+/-0) = +/-0.

Reapply x's sign on the polynomial path via Or(y, And(x, kNegZero)),
using the sign-bit mask already defined in this function. Since expm1(x)
shares the sign of x, this only affects the x == -0.0 case; every other
input (including +0, small nonzero x, and the large-|x| branch) is
unchanged.

Add a bit-exact signed-zero regression test for Expm1; the existing ULP
comparator treats +0 == -0 and cannot catch this.
// Reapply the sign of x on the polynomial path so a negative-zero input is
// preserved: expm1(-0) = -0 (C11 F.10.3.3), which the polynomial drops when
// it forms 0.5 * (+0) + (-0) = +0.
const V z = IfThenElse(Lt(Abs(x), kLn2Over2), Or(y, And(x, kNegZero)),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for fixing! Rather than Or+And, please use CopySign(y, x) - this is more efficient on some platforms.

// Reapply the sign of x on the polynomial path so a negative-zero input is
// preserved: expm1(-0) = -0 (C11 F.10.3.3), which the polynomial drops when
// it forms 0.5 * (+0) + (-0) = +0.
const V z = IfThenElse(Lt(Abs(x), kLn2Over2), Or(y, And(x, kNegZero)),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once again thanks for fixing! Rather than Or+And, please use CopySign(y, x) - this is more efficient on some platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants