AK/Math: Generic rint, and better rounding inlining + fallout care by Hendiadyoin1 · Pull Request #26835 · SerenityOS/serenity

Hendiadyoin1 · 2026-06-23T14:43:46Z

No description provided.

nico · 2026-06-24T20:11:25Z

Do we have any test coverage for rint? (I don't see it in TestMath.cpp and TestAKMath.cpp at least.) Could you add some?

Could a similar impl work in LibC/math.cpp for {,l}lrint{f,,l}? If so, this would also help with #25934.

Hendiadyoin1 · 2026-06-30T00:11:10Z

Could a similar impl work in LibC/math.cpp for {,l}lrint{f,,l}? If so, this would also help with #25934.

lrint* now delegates to AK/Math, so now the issue is the lack of a long double there I guess?

as for tests, I added a few crude ones for rint and round_to was covered by FixedPoint, which actually exposed an off by one in my rint impl

BuggieBot · 2026-07-20T01:12:09Z

Hello!

One or more of the commit messages in this PR do not match the SerenityOS code submission policy, please check the lint_commits CI job for more details on which commits were flagged and why.
Please do not close this PR and open another, instead modify your commit message(s) with git commit --amend and force push those changes to update this PR.

ADKaster · 2026-07-20T18:59:11Z

 {
    CONSTEXPR_STATE(round, x);
+#if ARCH(AARCH64) || ARCH(RISCV64) || defined(__SSE4_1__)
+    ELEMENTWISE_BUILTIN(round, x);


let me confirm my understanding here.

The way these get stamped out is something like this:

constexpr FP foo(FP x) { if (is_constant_evaluated()) // if consteval? 🤔 { some slow path or builtin that the compiler can use at compile time } if (has_builtin(builtin_elementwise_foo) { call builtin_elementwise_foo(); // guaranteed to not libcall? } #if arch(aarch64) call assembly for foo (FP == float, double) #elif arch(riscv64) call assembly for foo (FP == float, double, long double?) #elif ( SSE 4.1 ) // or w/e revision call x86_64 assembly for foo (FP == float, double, long double) #endif return some slow path; }

Is that what all of these are doing?

Is there a way to keep track of which methods are doing which steps here?

Is anyone doing it out of order that you've touched in this PR?

Should we have all this gunk in AK, or move it to LibC and just have AK consumers call libc methods? (personally, I would prefer AK::foo to be: constexpr version and/or call builtin, and let the compiler call a libcall into libc if it wants)

I don't think we need to answer all these questions right now, but at the very least, question 3 should be answered before we merge this.

nico · 2026-07-20T19:01:22Z

+{
+    if constexpr (sizeof(I) <= sizeof(u32)) {
+        i32 res;
+        asm("fcvt.w.d %0, %1"


It'd be nice if we could do builtins instead of inline asm as much as possible. The inline asm is a barrier for at least clang's autovectorizers, and I locally have sped up a bunch of loops dramatically by replacing Math/Rounding.h inline asm with intrinsics and telling clang to vectorize a loop.

Don't we currently forward most LibC functions to AK? I'm a bit worried that some of these builtins get compiled to LibC calls. (I did already have that problem with __builtin_ffs before on RISC-V)

But I agree that AK should ideally be completely constexpr. Making it constexpr-only should probably(?) also prevent this exact problem.

I think we should strive to strip the inline asm from AK and move it all into LibC. Then reversing the call-into arrow from LibC -> AK to AK -> LibC would be manageable. It's kind of pointless to include a bunch of if constexpr speculation in the implementation of libcalls.

The only concern imo would be integer operations that the Kernel calls. But for floating point operations, the only thing the kernel knows (and needs to know) how to do is push/pop the FP register state.

Hendiadyoin1 · 2026-07-21T16:23:39Z

So with the latest commit, I think I can answer Andrew's questions a bit and alleviate some of thakis' concerns:
I have checked and documented the behavior of all rounding builtins and reordered the checks in a way,
that those are only used when we can guarantee they are libcall free,
this also removes a lot of the handwritten assembly so vectorization and compiler reasoning should work better now

I have not really checked all other inline asm spaghetti caves, but I know that the Trig header has some unnecessary nesting

I think I should squash that commit, but I am not sure what the best final commit order and composition would be

nico · 2026-07-22T13:05:55Z

+{
+    if constexpr (sizeof(I) <= sizeof(u32)) {
+        i32 res;
+        asm("fcvt.w.d %0, %1"


let's not add new inline asm impls

Sure...

Should I also remove the other ones?
afaict compilers really dont like rint that much, only clang 20+ risc-v and x86 clang 21+ seems to concat the rint with the ltrunc

Hendiadyoin1 · 2026-07-23T22:12:54Z

OK,
I removed the more feng-shui elementwise builtin,
and now only rely on the fallback in round_to, which with the compiler parseable rint should be similarly fast if not faster after optimizations

Hendiadyoin1 · 2026-07-23T22:25:11Z

Ah seems like i forgot to remove one, will fix
(Tomorrow)

spholz · 2026-07-25T15:26:18Z

 };
 static_assert(AssertSize<FloatExtractor<f80>, sizeof(f80)>());
+#else
+// On some platforms (Arm Mac and Arm Windows) long double is a distinct version of f64


I think Windows in general (at least x86 windows) uses double precision floats for long double.

Otherwise we try to convert a Nan to u8, which is undefined

This is a) computationally smarter, and b) prevents a division by 0 when "mixing" with a 0 alpha color and full weight

Seems like we never did, not sure how it ever worked without

That test previously succeeded, as it by coincidence produced a negative NaNs, which is not the correct result none the less. Explcitely checking the value does not help, as EXPECT_APPROXIMATE does not handle NaNs, and enabling them has quite the fallout

While both versions are constexpr-able, directly relying on the compiler feels nicer

While slightly less efficient with current compilers, this allows more reasoning and optimizations by those compilers. There were also some copy-os in the aarch version of the old code, which now disappears together with the other inline asm implementations

This makes it pick up generic implementations in these cases

To allow compiler reasoning and vectorization we should try to use the builtins when possible, and avoid assembly in other cases This commit does a few things: * Exclude long double from some early `if consteval` paths As noted, we cant round-trip cast those in all cases * Add a table of tested inlining behavior * Always rely on inlining when guaranteed * Adjust floor/ceil/trunc to match each other and the compiler (gcc) internal expansion more closely * Reimplement the round fallback based on a trick borrowed from clang This now also has an alive2 proof :^)

We now have AK tests relying on this and the fallback paths go through here

github-actions · 2026-08-22T01:58:14Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions!

github-actions Bot added the 👀 pr-needs-review PR needs review from a maintainer or community member label Jun 23, 2026

Hendiadyoin1 force-pushed the clang-can-math branch from 2873014 to a5a35ba Compare June 23, 2026 14:55

spholz reviewed Jun 24, 2026

View reviewed changes

Comment thread AK/Math/Rounding.h Outdated

Hendiadyoin1 force-pushed the clang-can-math branch 2 times, most recently from bf02a77 to 6b71064 Compare June 30, 2026 00:08

Hendiadyoin1 force-pushed the clang-can-math branch from 6b71064 to 558801a Compare July 4, 2026 13:51

ADKaster reviewed Jul 20, 2026

View reviewed changes

Comment thread AK/Math/Rounding.h Outdated

ADKaster reviewed Jul 20, 2026

View reviewed changes

Comment thread AK/Math/Rounding.h Outdated

Hendiadyoin1 force-pushed the clang-can-math branch from 558801a to 69584fc Compare July 20, 2026 01:11

Hendiadyoin1 force-pushed the clang-can-math branch from 69584fc to dd59493 Compare July 20, 2026 01:18

ADKaster reviewed Jul 20, 2026

View reviewed changes

nico reviewed Jul 20, 2026

View reviewed changes

Hendiadyoin1 force-pushed the clang-can-math branch from dd59493 to 879e25f Compare July 21, 2026 16:16

Hendiadyoin1 force-pushed the clang-can-math branch 5 times, most recently from 65bda6a to bb01b9f Compare July 22, 2026 12:58

nico reviewed Jul 22, 2026

View reviewed changes

Hendiadyoin1 force-pushed the clang-can-math branch from bb01b9f to 0af939f Compare July 23, 2026 22:09

Hendiadyoin1 requested a review from nico July 23, 2026 22:13

Hendiadyoin1 force-pushed the clang-can-math branch 2 times, most recently from 75f3077 to 87caaa5 Compare July 24, 2026 10:00

spholz reviewed Jul 25, 2026

View reviewed changes

Hendiadyoin1 force-pushed the clang-can-math branch 2 times, most recently from 9eb6951 to 65c6a3a Compare July 25, 2026 21:36

Hendiadyoin1 changed the title ~~AK/Math: Generic rint, and smarter clang builtins~~ AK/Math: Generic rint, and better rounding inlining + fallout care Jul 25, 2026

Hendiadyoin1 force-pushed the clang-can-math branch from 65c6a3a to 8124303 Compare July 25, 2026 23:13

Hendiadyoin1 added 14 commits August 1, 2026 00:03

LibGfx: Short-circuit zero area pixel draws in bitmap painting

dddda8a

Otherwise we try to convert a Nan to u8, which is undefined

LibGfx: Short-circuit Color::mixed_with for full weights

5c878af

This is a) computationally smarter, and b) prevents a division by 0 when "mixing" with a 0 alpha color and full weight

LibM: Set result sign on fast path in lgamma_r

f4e35c3

Seems like we never did, not sure how it ever worked without

LibM: Perform internal_gamma in the specified type

d76a01d

AK: Define FloatExtractor<long double> on non f80/f128 systems

95636c5

AK/Math: Rely on compiler builtins for copysign and mark it constexpr

4915d3b

While both versions are constexpr-able, directly relying on the compiler feels nicer

AK/Math: Add a generic rint implementation

fe45655

LibM: Delegate [l,ll]rint to AK/Math round_to

b389d91

AK/Math: Let AARCH64_INSTRUCTION and RiscV fall through for long double

a080a3c

This makes it pick up generic implementations in these cases

Tests/AK: Add more rounding tests

6a96712

LibM: Preserve sign on trunc[lf]?

010542b

We now have AK tests relying on this and the fallback paths go through here

Hendiadyoin1 force-pushed the clang-can-math branch from 8124303 to 010542b Compare July 31, 2026 22:14

github-actions Bot added the stale label Aug 22, 2026

Hendiadyoin1 removed the stale label Aug 24, 2026

Hendiadyoin1 mentioned this pull request Aug 28, 2026

LibC: Improve lgamma coverage, performance and precision #26984

Merged

Uh oh!

Conversation

Hendiadyoin1 commented Jun 23, 2026

Uh oh!

Uh oh!

nico commented Jun 24, 2026

Uh oh!

Hendiadyoin1 commented Jun 30, 2026

Uh oh!

Uh oh!

Uh oh!

BuggieBot commented Jul 20, 2026

Uh oh!

ADKaster Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

nico Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

spholz Jul 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ADKaster Jul 20, 2026

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 commented Jul 21, 2026

Uh oh!

nico Jul 22, 2026

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

Hendiadyoin1 commented Jul 23, 2026

Uh oh!

Hendiadyoin1 commented Jul 23, 2026

Uh oh!

spholz Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

spholz Jul 20, 2026 •

edited

Loading