Skip to content

AK: Use correct built-in overload for several math functions#26696

Merged
spholz merged 2 commits intoSerenityOS:masterfrom
nico:ak-math-builtin
Apr 5, 2026
Merged

AK: Use correct built-in overload for several math functions#26696
spholz merged 2 commits intoSerenityOS:masterfrom
nico:ak-math-builtin

Conversation

@nico
Copy link
Copy Markdown
Contributor

@nico nico commented Apr 5, 2026

sqrt, sin, cos, tan, atan, atan2, log, log10 used to always call the
double built-in.

Now the float overload calls the float built-in, the double overload the
double built-in, and the long double overload the long double built-in.

Ideally, we'd stop calling built-ins for these (see #26662),
but as long as we do, we should call the right ones.

Similar to the last two commits in #18998.


I noticed this because I was trying to check how often our sin() is exact [1], and I forgot to apply something like #26662, meaning I was comparing to system libm (since I was doing this on macOS). And to my surprise, more numbers were exact than if I tested system libm directly [2]. Turns out that was because AK was calling system libm double sin() versions for floats. (Vaguely interesting that sin() is more often precise for float inputs/outputs than sinf().)

1:

AK sin precision count
#include <AK/Math.h>

#include <bit>
#include <stdint.h>
#include <stdio.h>

#include <math.h>

float ak_sinf(float f)
{
    return AK::sin(f);
}

#include "shared/math.h"

// Use `-I ~/src/llvm-project/libc`.
// Known to be exact for floats: https://members.loria.fr/PZimmermann/papers/accuracy.pdf
float libcxx_sinf(float f)
{
    return LIBC_NAMESPACE::shared::sinf(f);
}

int main()
{
    uint64_t num_wrong = 0;
    uint32_t i =0;

    float max_error = 0, max_arg = 0;;

    do {
        float f = std::bit_cast<float>(i);
        float ak_result, libcxx_result, abs_error;

        //if (isnan(f) || isinf(f)) goto next;
        //if (!isnormal(f) && f != 0) goto next;

        ak_result = ak_sinf(f);
        libcxx_result = libcxx_sinf(f);

        abs_error = fabsf(ak_result - libcxx_result);
        if (abs_error > max_error) {
            max_error = abs_error;
            max_arg = f;
        }

        if (ak_result != libcxx_result) {
            ++num_wrong;
        }

next:
        ++i;
    } while (i != 0);

    printf("max error: %f (%g)\n", max_error, max_error);
    printf("AK::sinf(%f) = %f\n", max_arg, ak_sinf(max_arg));
    printf("libcxx sinf(%f) = %f\n", max_arg, libcxx_sinf(max_arg));
    printf("%llu wrong\n", num_wrong);
}

2:

system libm sin precision count
#include <bit>
#include <stdint.h>
#include <stdio.h>

#include <math.h>

float apple_sinf(float f)
{
    return ::sinf(f);
}

//#define LIBC_MATH LIBC_MATH_FAST
//#define LIBC_MATH 0xf
#include "shared/math.h"

// Use `-I ~/src/llvm-project/libc`.
// Known to be exact for floats: https://members.loria.fr/PZimmermann/papers/accuracy.pdf
float libcxx_sinf(float f)
{
    return LIBC_NAMESPACE::shared::sinf(f);
}

int main()
{
    int num_wrong = 0;
    uint32_t i =0;

    float max_error = 0, max_arg = 0;;

    do {
        float f = std::bit_cast<float>(i);
        float apple_result, libcxx_result, abs_error;

        //if (isnan(f) || isinf(f)) goto next;
        //if (!isnormal(f) && f != 0) goto next;

        apple_result = apple_sinf(f);
        libcxx_result = libcxx_sinf(f);

        abs_error = fabsf(apple_result - libcxx_result);
        if (abs_error > max_error) {
            max_error = abs_error;
            max_arg = f;
        }

        if (apple_result != libcxx_result) {
            ++num_wrong;
        }

next:
        ++i;
    } while (i != 0);

    printf("max error: %f (%g)\n", max_error, max_error);
    printf("::sinf(%f) = %f\n", max_arg, apple_sinf(max_arg));
    printf("libcxx sinf(%f) = %f\n", max_arg, libcxx_sinf(max_arg));
    printf("%d wrong\n", num_wrong);
}

@github-actions github-actions bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Apr 5, 2026
Copy link
Copy Markdown
Member

@spholz spholz left a comment

Choose a reason for hiding this comment

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

LGTM, except for the build failures caused by incorrect function argument names.

nico added 2 commits April 5, 2026 10:37
sqrt, sin, cos, tan, atan, atan2, log, log10 used to always call the
double built-in.

Now the float overload calls the float built-in, the double overload the
double built-in, and the long double overload the long double built-in.

Ideally, we'd stop calling built-ins for these (see SerenityOS#26662),
but as long as we do, we should call the right ones.

Similar to the last two commits in SerenityOS#18998.
@nico nico force-pushed the ak-math-builtin branch from c519b55 to ef3d622 Compare April 5, 2026 14:37
@nico
Copy link
Copy Markdown
Contributor Author

nico commented Apr 5, 2026

LGTM, except for the build failures caused by incorrect function argument names.

Whoops, sorry about that! ceil / floor don't use that codepath on aarch64, which is where I am 😅

@spholz spholz added ✅ pr-maintainer-approved-but-awaiting-ci PR has been approved by a maintainer and can be merged after CI has passed and removed 👀 pr-needs-review PR needs review from a maintainer or community member labels Apr 5, 2026
@spholz spholz merged commit 65209b7 into SerenityOS:master Apr 5, 2026
15 checks passed
@github-actions github-actions bot removed the ✅ pr-maintainer-approved-but-awaiting-ci PR has been approved by a maintainer and can be merged after CI has passed label Apr 5, 2026
@nico nico deleted the ak-math-builtin branch April 5, 2026 22:47
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