Re: Please revert b2e843161d

On 6/27/26 11:49, Adrian Chadd wrote:
> [cc'ing pkubaj about this, as it started with him and landing some
> ppc64le float size changes]
> 
> On Fri, 26 Jun 2026 at 10:20, Steve Kargl <[email protected]> wrote:
> 
>> My point is that a compiler will use its builtin if one is available
>> in lieu of a function in libm.  One does not need to pollute the msun
>> source code with "#ifdef __builtin_XXX() #else ... #endif".  It simply
>> clutters comparisons to the libm code in NetBSD, OpenBSD, and Openlibm.
>> I do recognize that one may need to specify a compiler option such
>> as -mfma for gcc on amd64 to get a builtin.  That is due to the lack
>> of an fma instruction in the original amd64 instruction set.
>>
>> Can you at least add a cautionary comment in the source code that
>> USE_BUILTIN_FMA[F] have only been tested on arm64 and may lead to
>> segfaults on other architectures?
> 
> So pkubaj@ changed the ppc64le long double from 64 -> 128 bits, and
> there was a bunch of fallout from that.
> 
> See https://reviews.freebsd.org/D57388 for more information.
>

I'm not surprised.  I fixed a few bugs and implemented a couple
ld128 functions but have no access to a ld128 system.  markm
gave me limited access to one of his systems to test changes
a few years ago, but that's it.

> There's also some missing functions he's found we also need, and that
> is in https://reviews.freebsd.org/D57850 .

Yes, there are "some missing functions", but I think there's a small
difference in what is meant here.  None of the *f128() are required
by C23; whereas e.g., asinpi[fl](x) are.  See

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=295884

There are ld128 asinpil, acospil, and atanpil implementations in the
patch of that bug report.  I've compiled the code on amd64, but
have no current means for actually testing it.

The patch in D57850 seems to be wrong.  Instead of potential
function call overhead in msun/powerpc/ld128_compat.c, e.g.,

   long double acosf128(long double x) { return acosl(x); }

one should likely use a weak reference in msun/src/e_acosl.c:

#if LDBL_MANT_DIG == 128
__weak_reference(acosl, acosf128);
#endif

This would make acosf128 available on all ld128 architectures.
Admittedly, I'm not sure if this conflicts with any particular
compiler's _float128 or _float128_t.

> So I/we would really like some guidance on what/where to do here.
> I wonder if we would benefit from a "math" library group in reviews
> so we can include that for some very targeted feedback.
> 
> Steve, would you mind helping us navigate this? I'd super duper
> appreciate it!

There is the old freebsd-numerics@ mailing list.  I suspect I'm the
only person that is still subscribed.  Typically, I sent a patch
to the list.  bde and I would exchange a few emails on the list, and
then, ultimately he and I would have a long private email exchange.

We're getting a bit off-topic for my original post.  I think that
the msun sources should not directly use compiler __builtin_XXX()
functions.  A compiler will use a builtin function if one is
available (and yes, one might need to add a compiler option to
get access to it).  Direct use infers with the -fno-builtin compiler
option.  Finally, the USE_BUILTIN_* in msun code should have a
comment about what arch it can be with and should likely be
guarded with '#if defined(__arm64__)' (or whatever __yada applies).

-- 
steve