Skip to content

Commit 55cdd9c

Browse files
committed
BUG: Fix f64 trig accuracy issues
- Match SVML rounding DAG in large-argument reconstruction - Give low-accuracy f64 cosine its own minimax fit - Restore sub-ULP accuracy on non-FMA targets - Mark dead x-sign term in High sign DAG for removal
1 parent 897e1ec commit 55cdd9c

5 files changed

Lines changed: 120 additions & 39 deletions

File tree

npsr/lut-inl.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#endif
77

88
#include <tuple>
9+
#include <limits>
910

1011
#include "npsr/hwy.h"
1112

npsr/trig/extended-inl.h

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -272,22 +272,19 @@ NPSR_INTRIN V Extended(V x) {
272272
// =============================================================================
273273
// PHASE 10: Final Assembly
274274
// =============================================================================
275-
V res_lo = NegMulAdd(func_hi, r, deriv);
276-
res_lo = MulAdd(res_lo, r_lo, func_lo);
277275
V res_hi_lo = MulAdd(sigma, r, func_hi);
278276
V res_hi = MulAdd(deriv_hi, r, res_hi_lo);
279277

280278
V sum_cor = MulAdd(sigma, r, Sub(func_hi, res_hi_lo));
281279
V deriv_hi_r_cor = MulAdd(deriv_hi, r, Sub(res_hi_lo, res_hi));
282-
deriv_hi_r_cor = Add(deriv_hi_r_cor, sum_cor);
283-
res_lo = Add(res_lo, deriv_hi_r_cor);
280+
V res_lo0 = Add(deriv_hi_r_cor, sum_cor);
284281

285282
// Polynomial corrections
286283
V s2 = Set(d, kIsSingle ? 0x1.1110b8p-7f : 0x1.1110fabb3551cp-7);
287284
V s1 = Set(d, kIsSingle ? -0x1.555556p-3f : -0x1.5555555554448p-3);
288285
V sin_poly = MulAdd(s2, r2, s1);
289-
sin_poly = Mul(sin_poly, r);
290286
sin_poly = Mul(sin_poly, r2);
287+
sin_poly = Mul(sin_poly, r);
291288

292289
V c1 = Set(d, kIsSingle ? 0x1.5554f8p-5f : 0x1.5555555554ccfp-5);
293290
const V neg_half = Set(d, static_cast<T>(-0.5));
@@ -301,8 +298,22 @@ NPSR_INTRIN V Extended(V x) {
301298
}
302299
cos_poly = Mul(cos_poly, r2);
303300

304-
res_lo = MulAdd(sin_poly, deriv, res_lo);
305-
res_lo = MulAdd(cos_poly, func_hi, res_lo);
301+
V corr = NegMulAdd(func_hi, r, deriv);
302+
V res_lo_rlo = MulAdd(corr, r_lo, func_lo);
303+
// SVML pairs the correction terms differently per precision: F32 scales the
304+
// sin term by corr (first-order cos at the reduced point) and accumulates
305+
// the cos term onto the r_lo chain; F64 scales the sin term by deriv and
306+
// accumulates the cos term onto it, adding the r_lo chain last.
307+
V res_lo;
308+
if constexpr (kIsSingle) {
309+
V res_lo_sin = MulAdd(corr, sin_poly, res_lo0);
310+
V res_lo_cos = MulAdd(func_hi, cos_poly, res_lo_rlo);
311+
res_lo = Add(res_lo_cos, res_lo_sin);
312+
} else {
313+
V res_lo_sin = MulAdd(deriv, sin_poly, res_lo0);
314+
V res_lo_cos = MulAdd(func_hi, cos_poly, res_lo_sin);
315+
res_lo = Add(res_lo_cos, res_lo_rlo);
316+
}
306317
return Add(res_hi, res_lo);
307318
}
308319
// NOLINTNEXTLINE(google-readability-namespace-comments)

npsr/trig/high-inl.h

Lines changed: 75 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,30 @@ NPSR_INTRIN V High(V x) {
9292
}
9393
return poly;
9494
}
95+
/**
96+
* Substitute for the FMA product-error idiom on targets without native FMA.
97+
*
98+
* With FMA, `e = MulSub(a, b, Round(a*b))` recovers the rounding error of the
99+
* product exactly. Without it, that expression only sees the already-rounded
100+
* product and returns nothing useful. Instead, split both operands with a bit
101+
* mask (26 significant bits in the head) so `head = a_hi*b_hi` is exact by
102+
* construction, and return the dropped cross terms in `rest`:
103+
* a*b == head + rest, up to a negligible ~2^-105-scale rounding of `rest`.
104+
*/
105+
template <typename V, HWY_IF_F64(TFromV<V>)>
106+
NPSR_INTRIN void SplitMul(V a, V b, V& head, V& rest) {
107+
using namespace hn;
108+
const DFromV<V> d;
109+
const RebindToUnsigned<decltype(d)> du;
110+
const V mask = BitCast(d, Set(du, 0xFFFFFFFFF8000000u));
111+
const V a_hi = And(a, mask);
112+
const V a_lo = Sub(a, a_hi);
113+
const V b_hi = And(b, mask);
114+
const V b_lo = Sub(b, b_hi);
115+
head = Mul(a_hi, b_hi);
116+
rest = MulAdd(a_hi, b_lo, Mul(a_lo, b));
117+
}
118+
95119
/**
96120
* This function computes sin(x) or cos(x) for |x| < 2^24 using the Cody-Waite
97121
* reduction algorithm combined with table lookup and polynomial approximation,
@@ -147,12 +171,13 @@ NPSR_INTRIN V High(V x) {
147171

148172
// Step 3: Multi-precision computation of remainder r
149173
// r = x - n*(π/16)_high
174+
// Both splits apply their parts in descending order through the same DAG.
175+
// Without native FMA the non-tail parts carry 27/27/29 bits so every
176+
// n*part product is exact for |n| <= 85445660 = round(2^24 * 16/π); each
177+
// subtraction's rounding error is then captured exactly below, and the
178+
// decayed mul+sub sequence loses nothing against the fused path.
150179
constexpr auto kPiDiv16Prec29 = data::kPiDiv16Prec29<kNativeFMA>;
151180
V r_hi = NegMulAdd(n, Set(d, kPiDiv16Prec29[0]), x);
152-
if constexpr (!kNativeFMA) {
153-
// For F64, we need to handle the low precision part separately
154-
r_hi = NegMulAdd(n, Set(d, kPiDiv16Prec29[3]), r_hi);
155-
}
156181
const V pi16_med = Set(d, kPiDiv16Prec29[1]);
157182
const V pi16_lo = Set(d, kPiDiv16Prec29[2]);
158183
V r_med = NegMulAdd(n, pi16_med, r_hi);
@@ -162,6 +187,15 @@ NPSR_INTRIN V High(V x) {
162187
V term = NegMulAdd(pi16_med, n, Sub(r_hi, r_med));
163188
V r_lo = MulAdd(pi16_lo, n, Sub(r, r_med));
164189
r_lo = Sub(term, r_lo);
190+
if constexpr (!kNativeFMA) {
191+
// Fourth piece (53 bits at 2^-91). Reuse the one rounded product in both
192+
// the subtraction and its error capture so (r, r_lo) stays an exact
193+
// double-double of the reduction even when r is tiny near k*π/2.
194+
const V tail_prod = Mul(n, Set(d, kPiDiv16Prec29[3]));
195+
const V r_prev = r;
196+
r = Sub(r_prev, tail_prod);
197+
r_lo = Add(r_lo, Sub(Sub(r_prev, r), tail_prod));
198+
}
165199

166200
// Step 4: Polynomial approximation
167201
V r2 = Mul(r, r);
@@ -210,13 +244,23 @@ NPSR_INTRIN V High(V x) {
210244
if constexpr (OP == Operation::kCos) {
211245
// Cosine reconstruction: cos_table - sin_table*remainder
212246
// Equivalent to: cos(a)*cos(r) - sin(a)*sin(r) but more efficient
213-
V res_hi = NegMulAdd(r, sin_hi, cos_hi); // cos_hi - r*sin_hi
214-
215-
// This captures the precision lost in the main computation
216-
V r_sin_hi = Sub(cos_hi, res_hi); // Extract high part of multiplication
217-
218-
// Handles rounding errors and adds sin_low contribution
219-
V r_sin_low = MulSub(r, sin_hi, r_sin_hi); // Compute multiplication error
247+
V res_hi, r_sin_low;
248+
if constexpr (kNativeFMA) {
249+
res_hi = NegMulAdd(r, sin_hi, cos_hi); // cos_hi - r*sin_hi
250+
251+
// This captures the precision lost in the main computation
252+
V r_sin_hi = Sub(cos_hi, res_hi); // Extract high part of multiplication
253+
254+
// Handles rounding errors via the FMA product-error idiom
255+
r_sin_low = MulSub(r, sin_hi, r_sin_hi); // Compute multiplication error
256+
} else {
257+
// Exact-by-construction head product replaces the FMA idiom
258+
V r_sin, r_sin_rest;
259+
SplitMul(r, sin_hi, r_sin, r_sin_rest);
260+
res_hi = Sub(cos_hi, r_sin);
261+
V r_sin_hi = Sub(cos_hi, res_hi);
262+
r_sin_low = Add(Sub(r_sin, r_sin_hi), r_sin_rest);
263+
}
220264
V sin_low_corr = MulAdd(r, sin_lo, r_sin_low); // Add sin_low term
221265

222266
// This is used to apply the low-precision remainder correction
@@ -245,13 +289,23 @@ NPSR_INTRIN V High(V x) {
245289
} else {
246290
// Sine reconstruction: sin_table + cos_table*remainder
247291
// Equivalent to: sin(a)*cos(r) + cos(a)*sin(r) but more efficient
248-
V res_hi = MulAdd(r, cos_hi, sin_hi); // sin_hi + r*cos_hi
249-
250-
// This captures the precision lost in the main computation
251-
V r_cos_hi = Sub(res_hi, sin_hi); // Extract high part of multiplication
252-
253-
// Handles rounding errors and adds cos_low contribution
254-
V r_cos_low = MulSub(r, cos_hi, r_cos_hi); // Compute multiplication error
292+
V res_hi, r_cos_low;
293+
if constexpr (kNativeFMA) {
294+
res_hi = MulAdd(r, cos_hi, sin_hi); // sin_hi + r*cos_hi
295+
296+
// This captures the precision lost in the main computation
297+
V r_cos_hi = Sub(res_hi, sin_hi); // Extract high part of multiplication
298+
299+
// Handles rounding errors via the FMA product-error idiom
300+
r_cos_low = MulSub(r, cos_hi, r_cos_hi); // Compute multiplication error
301+
} else {
302+
// Exact-by-construction head product replaces the FMA idiom
303+
V r_cos, r_cos_rest;
304+
SplitMul(r, cos_hi, r_cos, r_cos_rest);
305+
res_hi = Add(sin_hi, r_cos);
306+
V r_cos_hi = Sub(res_hi, sin_hi);
307+
r_cos_low = Add(Sub(r_cos, r_cos_hi), r_cos_rest);
308+
}
255309
V cos_low_corr = MulAdd(r, cos_lo, r_cos_low); // Add cos_low term
256310

257311
// Intermediate term for r_low correction: cos_table - sin_table*r
@@ -282,6 +336,9 @@ NPSR_INTRIN V High(V x) {
282336
// This unified approach works because:
283337
// - sin(x + π) = -sin(x)
284338
// - cos(x + π) = -cos(x)
339+
// TODO(seiko2plus): x_sign_int and the Xor are dead code (always zero) and
340+
// should be removed. The sign is bit 4 of n_biased alone; sin(-0.0) is
341+
// handled in trig/inl.h under kSpecialCases.
285342
VU x_sign_int = ShiftLeft<63>(BitCast(du, x));
286343
// XOR with quadrant info in n_biased
287344
VU combined = Xor(BitCast(du, n_biased), ShiftLeft<4>(x_sign_int));

npsr/trig/inl.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ namespace npsr::HWY_NAMESPACE::trig {
4949
* - Double: |x| > 2^24 (16,777,216 - where 53-bit mantissa loses precision)
5050
*/
5151
template <Operation OP, typename Prec, typename V>
52-
NPSR_INTRIN V Trig(Prec &prec, V x) {
52+
NPSR_INTRIN V Trig(Prec& prec, V x) {
5353
using namespace hwy::HWY_NAMESPACE;
5454
constexpr bool kIsSingle = std::is_same_v<TFromV<V>, float>;
5555
const DFromV<V> d;
@@ -81,8 +81,14 @@ NPSR_INTRIN V Trig(Prec &prec, V x) {
8181
// Thresholds chosen based on when standard reduction loses accuracy:
8282
// - Float: 10,000 is conservative but ensures < 1 ULP error
8383
// - Double: 2^24 is where mantissa can't represent x and x-2π distinctly
84+
// - Double low-accuracy cosine: stays on its fast path
85+
// until trunc(|x|/π) > 8388606; this is the largest double below that
86+
// boundary (~8388607π)
87+
constexpr bool kIsLowCos =
88+
Prec::kLowAccuracy && OP == trig::Operation::kCos;
89+
constexpr double kLargeF64 = kIsLowCos ? 0x1.921fb2200366fp+24 : 16777216.0;
8490
auto has_large_arg =
85-
And(Gt(Abs(x), Set(d, kIsSingle ? 10000.0f : 16777216.0)), is_finite);
91+
And(Gt(Abs(x), Set(d, kIsSingle ? 10000.0f : kLargeF64)), is_finite);
8692

8793
// Extended precision is expensive, only use when necessary
8894
if (HWY_UNLIKELY(!AllFalse(d, has_large_arg))) {
@@ -121,7 +127,7 @@ namespace npsr::HWY_NAMESPACE {
121127
* ```
122128
*/
123129
template <typename Prec, typename V>
124-
NPSR_INTRIN V Sin(Prec &prec, V x) {
130+
NPSR_INTRIN V Sin(Prec& prec, V x) {
125131
return trig::Trig<trig::Operation::kSin>(prec, x);
126132
}
127133

@@ -143,7 +149,7 @@ NPSR_INTRIN V Sin(Prec &prec, V x) {
143149
* ```
144150
*/
145151
template <typename Prec, typename V>
146-
NPSR_INTRIN V Cos(Prec &prec, V x) {
152+
NPSR_INTRIN V Cos(Prec& prec, V x) {
147153
return trig::Trig<trig::Operation::kCos>(prec, x);
148154
}
149155

npsr/trig/low-inl.h

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,17 @@ NPSR_INTRIN V PolyLow(V r, V r2) {
5454
template <Operation OP, typename V, HWY_IF_F64(TFromV<V>)>
5555
NPSR_INTRIN V PolyLow(V r, V r2) {
5656
using namespace hn;
57-
5857
const DFromV<V> d;
59-
const V c15 = Set(d, -0x1.9f1517e9f65fp-41);
60-
const V c13 = Set(d, 0x1.60e6bee01d83ep-33);
61-
const V c11 = Set(d, -0x1.ae6355aaa4a53p-26);
62-
const V c9 = Set(d, 0x1.71de3806add1ap-19);
63-
const V c7 = Set(d, -0x1.a01a019a659ddp-13);
64-
const V c5 = Set(d, 0x1.111111110a573p-7);
65-
const V c3 = Set(d, -0x1.55555555554a8p-3);
58+
// Sine and cosine use distinct minimax fits of sin(r): the cosine path
59+
// reduces around (N + 0.5)*pi, so its coefficient set differs
60+
constexpr bool kCos = OP == Operation::kCos;
61+
const V c15 = Set(d, kCos ? -0x1.9f0d60811aac8p-41 : -0x1.9f1517e9f65fp-41);
62+
const V c13 = Set(d, kCos ? 0x1.60e6857a2f220p-33 : 0x1.60e6bee01d83ep-33);
63+
const V c11 = Set(d, kCos ? -0x1.ae63546002231p-26 : -0x1.ae6355aaa4a53p-26);
64+
const V c9 = Set(d, kCos ? 0x1.71de38030fea0p-19 : 0x1.71de3806add1ap-19);
65+
const V c7 = Set(d, kCos ? -0x1.a01a019a5b87bp-13 : -0x1.a01a019a659ddp-13);
66+
const V c5 = Set(d, kCos ? 0x1.111111110a4a8p-7 : 0x1.111111110a573p-7);
67+
const V c3 = Set(d, kCos ? -0x1.55555555554a7p-3 : -0x1.55555555554a8p-3);
6668
V poly = MulAdd(c15, r2, c13);
6769
poly = MulAdd(r2, poly, c11);
6870
poly = MulAdd(r2, poly, c9);
@@ -117,10 +119,14 @@ NPSR_INTRIN V Low(V x) {
117119
r = NegMulAdd(n, Set(d, kPi[1]), r);
118120
V r_lo = NegMulAdd(n, Set(d, kPi[2]), r);
119121
if constexpr (!kNativeFMA) {
120-
if (!kIsSingle) {
122+
r_lo = NegMulAdd(n, Set(d, kPi[3]), r_lo);
123+
if constexpr (!kIsSingle) {
124+
// The polynomial must consume the fully reduced value: the partial
125+
// chain still carries ~n*2^-49 of unapplied π tail, far too coarse
126+
// for r² and its correction terms. (The f32 path already reads the
127+
// chain's end below.)
121128
r = r_lo;
122129
}
123-
r_lo = NegMulAdd(n, Set(d, kPi[3]), r_lo);
124130
}
125131

126132
if constexpr (kIsSingle) {

0 commit comments

Comments
 (0)