Skip to content

Commit 7f95e81

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 (Match SVML) - Restore sub-ULP accuracy on non-FMA targets - Mark dead x-sign term in High sign DAG for removal
1 parent f9df5ab commit 7f95e81

7 files changed

Lines changed: 1162 additions & 1059 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/data/approx.h

Lines changed: 1016 additions & 1016 deletions
Large diffs are not rendered by default.

npsr/trig/data/constants.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ template <> inline constexpr double kPiMul2<double>[] = {
3030
template <bool FMA> inline constexpr double kPiDiv16Prec29[] = {
3131
0x1.921fb54442d18p-3, 0x1.1a62633p-57, 0x1.45c06e0e68948p-89, };
3232
template <> inline constexpr double kPiDiv16Prec29<false>[] = {
33-
0x1.921fb54p-3, 0x1.a626331p-61, 0x1.1701b839a252p-91, 0x1.10b461p-33,
33+
0x1.921fb54p-3, 0x1.10b461p-33, 0x1.a626331p-61, 0x1.1701b839a252p-91,
3434
};
3535

3636
template <typename T> inline constexpr char kInvPi = '_';

npsr/trig/extended-inl.h

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,16 @@ NPSR_INTRIN V Extended(V x) {
233233
r_lo = Combine(d, DemoteTo(dh, r_lo_w0), DemoteTo(dh, r_lo_w1));
234234
r_w0 = BitCast(d, r0);
235235
r_w1 = BitCast(d, r1);
236+
} else if constexpr (!kNativeFMA) {
237+
// F64 without FMA: `MulSub(pi2_hi, n, r)` sees only the already-rounded
238+
// product and yields exactly zero, dropping the 0.5-ulp(r) rounding
239+
// error of r. Near k*pi/2 the result is ~deriv*r, so that loss is worth
240+
// 0.5 ulp of the final result. Recover it with the exact head product:
241+
// head + rest == pi2_hi*n, and head - r is exact by Sterbenz.
242+
V head, rest;
243+
SplitMul(pi2_hi, n, head, rest);
244+
r_lo = Add(Sub(head, r), rest);
245+
r_lo = Add(Mul(pi2_med, n), r_lo);
236246
} else {
237247
r_lo = MulSub(pi2_hi, n, r);
238248
r_lo = MulAdd(pi2_med, n, r_lo);
@@ -272,22 +282,31 @@ NPSR_INTRIN V Extended(V x) {
272282
// =============================================================================
273283
// PHASE 10: Final Assembly
274284
// =============================================================================
275-
V res_lo = NegMulAdd(func_hi, r, deriv);
276-
res_lo = MulAdd(res_lo, r_lo, func_lo);
277285
V res_hi_lo = MulAdd(sigma, r, func_hi);
278-
V res_hi = MulAdd(deriv_hi, r, res_hi_lo);
279-
286+
V res_hi, deriv_hi_r_cor;
287+
if constexpr (!kNativeFMA && !kIsSingle) {
288+
// Same FMA-idiom substitution as in Phase 7: near k*pi/2, res_hi is
289+
// ~deriv_hi*r, so the product rounding lost by a decayed MulAdd costs
290+
// up to 0.5 ulp of the result. (sigma*r in the chains below is fine
291+
// decayed: sigma ~ 2^-53*deriv makes its product error second-order.)
292+
V dr, dr_rest;
293+
SplitMul(deriv_hi, r, dr, dr_rest);
294+
res_hi = Add(res_hi_lo, dr);
295+
V dr_hi = Sub(res_hi, res_hi_lo);
296+
deriv_hi_r_cor = Add(Sub(dr, dr_hi), dr_rest);
297+
} else {
298+
res_hi = MulAdd(deriv_hi, r, res_hi_lo);
299+
deriv_hi_r_cor = MulAdd(deriv_hi, r, Sub(res_hi_lo, res_hi));
300+
}
280301
V sum_cor = MulAdd(sigma, r, Sub(func_hi, res_hi_lo));
281-
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);
302+
V res_lo0 = Add(deriv_hi_r_cor, sum_cor);
284303

285304
// Polynomial corrections
286305
V s2 = Set(d, kIsSingle ? 0x1.1110b8p-7f : 0x1.1110fabb3551cp-7);
287306
V s1 = Set(d, kIsSingle ? -0x1.555556p-3f : -0x1.5555555554448p-3);
288307
V sin_poly = MulAdd(s2, r2, s1);
289-
sin_poly = Mul(sin_poly, r);
290308
sin_poly = Mul(sin_poly, r2);
309+
sin_poly = Mul(sin_poly, r);
291310

292311
V c1 = Set(d, kIsSingle ? 0x1.5554f8p-5f : 0x1.5555555554ccfp-5);
293312
const V neg_half = Set(d, static_cast<T>(-0.5));
@@ -301,8 +320,22 @@ NPSR_INTRIN V Extended(V x) {
301320
}
302321
cos_poly = Mul(cos_poly, r2);
303322

304-
res_lo = MulAdd(sin_poly, deriv, res_lo);
305-
res_lo = MulAdd(cos_poly, func_hi, res_lo);
323+
V corr = NegMulAdd(func_hi, r, deriv);
324+
V res_lo_rlo = MulAdd(corr, r_lo, func_lo);
325+
// SVML pairs the correction terms differently per precision: F32 scales the
326+
// sin term by corr (first-order cos at the reduced point) and accumulates
327+
// the cos term onto the r_lo chain; F64 scales the sin term by deriv and
328+
// accumulates the cos term onto it, adding the r_lo chain last.
329+
V res_lo;
330+
if constexpr (kIsSingle) {
331+
V res_lo_sin = MulAdd(corr, sin_poly, res_lo0);
332+
V res_lo_cos = MulAdd(func_hi, cos_poly, res_lo_rlo);
333+
res_lo = Add(res_lo_cos, res_lo_sin);
334+
} else {
335+
V res_lo_sin = MulAdd(deriv, sin_poly, res_lo0);
336+
V res_lo_cos = MulAdd(func_hi, cos_poly, res_lo_sin);
337+
res_lo = Add(res_lo_cos, res_lo_rlo);
338+
}
306339
return Add(res_hi, res_lo);
307340
}
308341
// NOLINTNEXTLINE(google-readability-namespace-comments)

npsr/trig/high-inl.h

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -147,12 +147,13 @@ NPSR_INTRIN V High(V x) {
147147

148148
// Step 3: Multi-precision computation of remainder r
149149
// r = x - n*(π/16)_high
150+
// Both splits apply their parts in descending order through the same DAG.
151+
// Without native FMA the non-tail parts carry 27/27/29 bits so every
152+
// n*part product is exact for |n| <= 85445660 = round(2^24 * 16/π); each
153+
// subtraction's rounding error is then captured exactly below, and the
154+
// decayed mul+sub sequence loses nothing against the fused path.
150155
constexpr auto kPiDiv16Prec29 = data::kPiDiv16Prec29<kNativeFMA>;
151156
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-
}
156157
const V pi16_med = Set(d, kPiDiv16Prec29[1]);
157158
const V pi16_lo = Set(d, kPiDiv16Prec29[2]);
158159
V r_med = NegMulAdd(n, pi16_med, r_hi);
@@ -162,6 +163,15 @@ NPSR_INTRIN V High(V x) {
162163
V term = NegMulAdd(pi16_med, n, Sub(r_hi, r_med));
163164
V r_lo = MulAdd(pi16_lo, n, Sub(r, r_med));
164165
r_lo = Sub(term, r_lo);
166+
if constexpr (!kNativeFMA) {
167+
// Fourth piece (53 bits at 2^-91). Reuse the one rounded product in both
168+
// the subtraction and its error capture so (r, r_lo) stays an exact
169+
// double-double of the reduction even when r is tiny near k*π/2.
170+
const V tail_prod = Mul(n, Set(d, kPiDiv16Prec29[3]));
171+
const V r_prev = r;
172+
r = Sub(r_prev, tail_prod);
173+
r_lo = Add(r_lo, Sub(Sub(r_prev, r), tail_prod));
174+
}
165175

166176
// Step 4: Polynomial approximation
167177
V r2 = Mul(r, r);
@@ -210,13 +220,23 @@ NPSR_INTRIN V High(V x) {
210220
if constexpr (OP == Operation::kCos) {
211221
// Cosine reconstruction: cos_table - sin_table*remainder
212222
// 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
223+
V res_hi, r_sin_low;
224+
if constexpr (kNativeFMA) {
225+
res_hi = NegMulAdd(r, sin_hi, cos_hi); // cos_hi - r*sin_hi
226+
227+
// This captures the precision lost in the main computation
228+
V r_sin_hi = Sub(cos_hi, res_hi); // Extract high part of multiplication
229+
230+
// Handles rounding errors via the FMA product-error idiom
231+
r_sin_low = MulSub(r, sin_hi, r_sin_hi); // Compute multiplication error
232+
} else {
233+
// Exact-by-construction head product replaces the FMA idiom
234+
V r_sin, r_sin_rest;
235+
SplitMul(r, sin_hi, r_sin, r_sin_rest);
236+
res_hi = Sub(cos_hi, r_sin);
237+
V r_sin_hi = Sub(cos_hi, res_hi);
238+
r_sin_low = Add(Sub(r_sin, r_sin_hi), r_sin_rest);
239+
}
220240
V sin_low_corr = MulAdd(r, sin_lo, r_sin_low); // Add sin_low term
221241

222242
// This is used to apply the low-precision remainder correction
@@ -245,13 +265,23 @@ NPSR_INTRIN V High(V x) {
245265
} else {
246266
// Sine reconstruction: sin_table + cos_table*remainder
247267
// 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
268+
V res_hi, r_cos_low;
269+
if constexpr (kNativeFMA) {
270+
res_hi = MulAdd(r, cos_hi, sin_hi); // sin_hi + r*cos_hi
271+
272+
// This captures the precision lost in the main computation
273+
V r_cos_hi = Sub(res_hi, sin_hi); // Extract high part of multiplication
274+
275+
// Handles rounding errors via the FMA product-error idiom
276+
r_cos_low = MulSub(r, cos_hi, r_cos_hi); // Compute multiplication error
277+
} else {
278+
// Exact-by-construction head product replaces the FMA idiom
279+
V r_cos, r_cos_rest;
280+
SplitMul(r, cos_hi, r_cos, r_cos_rest);
281+
res_hi = Add(sin_hi, r_cos);
282+
V r_cos_hi = Sub(res_hi, sin_hi);
283+
r_cos_low = Add(Sub(r_cos, r_cos_hi), r_cos_rest);
284+
}
255285
V cos_low_corr = MulAdd(r, cos_lo, r_cos_low); // Add cos_low term
256286

257287
// Intermediate term for r_low correction: cos_table - sin_table*r
@@ -282,6 +312,9 @@ NPSR_INTRIN V High(V x) {
282312
// This unified approach works because:
283313
// - sin(x + π) = -sin(x)
284314
// - cos(x + π) = -cos(x)
315+
// TODO(seiko2plus): x_sign_int and the Xor are dead code (always zero) and
316+
// should be removed. The sign is bit 4 of n_biased alone; sin(-0.0) is
317+
// handled in trig/inl.h under kSpecialCases.
285318
VU x_sign_int = ShiftLeft<63>(BitCast(du, x));
286319
// XOR with quadrant info in n_biased
287320
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: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,30 @@ namespace npsr::HWY_NAMESPACE::trig {
1414

1515
enum class Operation { kSin = 0, kCos = 1 };
1616

17+
/**
18+
* Substitute for the FMA product-error idiom on targets without native FMA.
19+
*
20+
* With FMA, `e = MulSub(a, b, Round(a*b))` recovers the rounding error of the
21+
* product exactly. Without it, that expression only sees the already-rounded
22+
* product and returns nothing useful. Instead, split both operands with a bit
23+
* mask (26 significant bits in the head) so `head = a_hi*b_hi` is exact by
24+
* construction, and return the dropped cross terms in `rest`:
25+
* a*b == head + rest, up to a negligible ~2^-105-scale rounding of `rest`.
26+
*/
27+
template <typename V, HWY_IF_F64(TFromV<V>)>
28+
NPSR_INTRIN void SplitMul(V a, V b, V& head, V& rest) {
29+
using namespace hn;
30+
const DFromV<V> d;
31+
const RebindToUnsigned<decltype(d)> du;
32+
const V mask = BitCast(d, Set(du, 0xFFFFFFFFF8000000u));
33+
const V a_hi = And(a, mask);
34+
const V a_lo = Sub(a, a_hi);
35+
const V b_hi = And(b, mask);
36+
const V b_lo = Sub(b, b_hi);
37+
head = Mul(a_hi, b_hi);
38+
rest = MulAdd(a_hi, b_lo, Mul(a_lo, b));
39+
}
40+
1741
template <Operation OP, typename V, HWY_IF_F32(TFromV<V>)>
1842
NPSR_INTRIN V PolyLow(V r, V r2) {
1943
using namespace hn;
@@ -54,15 +78,17 @@ NPSR_INTRIN V PolyLow(V r, V r2) {
5478
template <Operation OP, typename V, HWY_IF_F64(TFromV<V>)>
5579
NPSR_INTRIN V PolyLow(V r, V r2) {
5680
using namespace hn;
57-
5881
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);
82+
// Sine and cosine use distinct minimax fits of sin(r): the cosine path
83+
// reduces around (N + 0.5)*pi, so its coefficient set differs
84+
constexpr bool kCos = OP == Operation::kCos;
85+
const V c15 = Set(d, kCos ? -0x1.9f0d60811aac8p-41 : -0x1.9f1517e9f65fp-41);
86+
const V c13 = Set(d, kCos ? 0x1.60e6857a2f220p-33 : 0x1.60e6bee01d83ep-33);
87+
const V c11 = Set(d, kCos ? -0x1.ae63546002231p-26 : -0x1.ae6355aaa4a53p-26);
88+
const V c9 = Set(d, kCos ? 0x1.71de38030fea0p-19 : 0x1.71de3806add1ap-19);
89+
const V c7 = Set(d, kCos ? -0x1.a01a019a5b87bp-13 : -0x1.a01a019a659ddp-13);
90+
const V c5 = Set(d, kCos ? 0x1.111111110a4a8p-7 : 0x1.111111110a573p-7);
91+
const V c3 = Set(d, kCos ? -0x1.55555555554a7p-3 : -0x1.55555555554a8p-3);
6692
V poly = MulAdd(c15, r2, c13);
6793
poly = MulAdd(r2, poly, c11);
6894
poly = MulAdd(r2, poly, c9);
@@ -117,10 +143,14 @@ NPSR_INTRIN V Low(V x) {
117143
r = NegMulAdd(n, Set(d, kPi[1]), r);
118144
V r_lo = NegMulAdd(n, Set(d, kPi[2]), r);
119145
if constexpr (!kNativeFMA) {
120-
if (!kIsSingle) {
146+
r_lo = NegMulAdd(n, Set(d, kPi[3]), r_lo);
147+
if constexpr (!kIsSingle) {
148+
// The polynomial must consume the fully reduced value: the partial
149+
// chain still carries ~n*2^-49 of unapplied π tail, far too coarse
150+
// for r² and its correction terms. (The f32 path already reads the
151+
// chain's end below.)
121152
r = r_lo;
122153
}
123-
r_lo = NegMulAdd(n, Set(d, kPi[3]), r_lo);
124154
}
125155

126156
if constexpr (kIsSingle) {

0 commit comments

Comments
 (0)