Skip to content

Commit 8cd47ad

Browse files
authored
libm: Fix tests for lgamma
The tests were using `rug::ln_gamma` as a reference for `libm::lgamma`, which actually computes the natural logarithm *of the absolute value* of the Gamma function. This changes the range of inputs used for the icount-benchmarks of these functions, which causes false regressions in [1]. [1]: https://github.com/rust-lang/compiler-builtins/actions/runs/21788698368/job/62864230903?pr=1075#step:7:2710. Fixes: a1a066611dc2 ("Create interfaces for testing against MPFR")
1 parent 3d09892 commit 8cd47ad

3 files changed

Lines changed: 45 additions & 20 deletions

File tree

library/compiler-builtins/libm-test/src/domain.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ impl<F: Float, I: Int> EitherPrim<Domain<F>, Domain<I>> {
207207
.into_prim_float()];
208208

209209
/// Domain for `loggamma`
210-
const LGAMMA: [Self; 1] = Self::STRICTLY_POSITIVE;
210+
const LGAMMA: [Self; 1] = Self::UNBOUNDED1;
211211

212212
/// Domain for `jn` and `yn`.
213213
// FIXME: the domain should provide some sort of "reasonable range" so we don't actually test

library/compiler-builtins/libm-test/src/mpfloat.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,9 @@ libm_macros::for_each_function! {
170170
ldexpf,
171171
ldexpf128,
172172
ldexpf16,
173+
lgamma,
173174
lgamma_r,
175+
lgammaf,
174176
lgammaf_r,
175177
modf,
176178
modff,
@@ -213,7 +215,6 @@ libm_macros::for_each_function! {
213215
fmaximum_num | fmaximum_numf | fmaximum_numf16 | fmaximum_numf128 => max,
214216
fmin | fminf | fminf16 | fminf128 |
215217
fminimum_num | fminimum_numf | fminimum_numf16 | fminimum_numf128 => min,
216-
lgamma | lgammaf => ln_gamma,
217218
log | logf => ln,
218219
log1p | log1pf => ln_1p,
219220
tgamma | tgammaf => gamma,
@@ -576,6 +577,30 @@ impl MpOp for crate::op::lgammaf_r::Routine {
576577
}
577578
}
578579

580+
impl MpOp for crate::op::lgamma::Routine {
581+
type MpTy = MpFloat;
582+
583+
fn new_mp() -> Self::MpTy {
584+
new_mpfloat::<Self::FTy>()
585+
}
586+
587+
fn run(this: &mut Self::MpTy, input: Self::RustArgs) -> Self::RustRet {
588+
<crate::op::lgamma_r::Routine as MpOp>::run(this, input).0
589+
}
590+
}
591+
592+
impl MpOp for crate::op::lgammaf::Routine {
593+
type MpTy = MpFloat;
594+
595+
fn new_mp() -> Self::MpTy {
596+
new_mpfloat::<Self::FTy>()
597+
}
598+
599+
fn run(this: &mut Self::MpTy, input: Self::RustArgs) -> Self::RustRet {
600+
<crate::op::lgammaf_r::Routine as MpOp>::run(this, input).0
601+
}
602+
}
603+
579604
/* stub implementations so we don't need to special case them */
580605

581606
impl MpOp for crate::op::nextafter::Routine {

library/compiler-builtins/libm-test/src/precision.rs

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ pub fn default_ulp(ctx: &CheckCtx) -> u32 {
6767
Bn::Exp2 => 1,
6868
Bn::Expm1 => 1,
6969
Bn::Hypot => 1,
70-
Bn::Lgamma | Bn::LgammaR => 16,
70+
Bn::Lgamma | Bn::LgammaR => 4,
7171
Bn::Log => 1,
7272
Bn::Log10 => 1,
7373
Bn::Log1p => 1,
@@ -102,7 +102,6 @@ pub fn default_ulp(ctx: &CheckCtx) -> u32 {
102102
match ctx.base_name {
103103
Bn::Cosh => ulp = 2,
104104
Bn::Exp10 if usize::BITS < 64 => ulp = 4,
105-
Bn::Lgamma | Bn::LgammaR => ulp = 400,
106105
Bn::Tanh => ulp = 4,
107106
_ => (),
108107
}
@@ -218,17 +217,17 @@ impl MaybeOverride<(f16,)> for SpecialCase {}
218217

219218
impl MaybeOverride<(f32,)> for SpecialCase {
220219
fn check_float<F: Float>(input: (f32,), actual: F, expected: F, ctx: &CheckCtx) -> CheckAction {
221-
if (ctx.base_name == BaseName::Lgamma || ctx.base_name == BaseName::LgammaR)
222-
&& input.0 > 4e36
223-
&& expected.is_infinite()
224-
&& !actual.is_infinite()
225-
{
226-
// This result should saturate but we return a finite value.
220+
if ctx.base_name == BaseName::J0 && input.0 < -1e34 {
221+
// Errors get huge close to -inf
227222
return XFAIL_NOCHECK;
228223
}
229224

230-
if ctx.base_name == BaseName::J0 && input.0 < -1e34 {
231-
// Errors get huge close to -inf
225+
// FIXME(correctness): lgammaf has high relative inaccuracy near its zeroes
226+
if matches!(ctx.base_name, BaseName::Lgamma | BaseName::LgammaR)
227+
&& input.0 > -13.0625
228+
&& input.0 < -2.0
229+
&& (expected.abs() < F::ONE || (input.0 - input.0.round()).abs() < 0.02)
230+
{
232231
return XFAIL_NOCHECK;
233232
}
234233

@@ -275,6 +274,15 @@ impl MaybeOverride<(f64,)> for SpecialCase {
275274
return XFAIL_NOCHECK;
276275
}
277276

277+
// FIXME(correctness): lgamma has high relative inaccuracy near its zeroes
278+
if matches!(ctx.base_name, BaseName::Lgamma | BaseName::LgammaR)
279+
&& input.0 > -32.0
280+
&& input.0 < -2.0
281+
&& (expected.abs() < F::ONE || (input.0 - input.0.round()).abs() < 0.02)
282+
{
283+
return XFAIL_NOCHECK;
284+
}
285+
278286
// maybe_check_nan_bits(actual, expected, ctx)
279287
unop_common(input, actual, expected, ctx)
280288
}
@@ -304,14 +312,6 @@ fn unop_common<F1: Float, F2: Float>(
304312
expected: F2,
305313
ctx: &CheckCtx,
306314
) -> CheckAction {
307-
if (ctx.base_name == BaseName::Lgamma || ctx.base_name == BaseName::LgammaR)
308-
&& input.0 < F1::ZERO
309-
&& !input.0.is_infinite()
310-
{
311-
// loggamma should not be defined for x < 0, yet we both return results
312-
return XFAIL_NOCHECK;
313-
}
314-
315315
// fabs and copysign must leave NaNs untouched.
316316
if ctx.base_name == BaseName::Fabs && input.0.is_nan() {
317317
// LLVM currently uses x87 instructions which quieten signalling NaNs to handle the i686

0 commit comments

Comments
 (0)