Thanks for the review. @Jeremy You're right, since `long double` is the same as `double` on AArch64, adding a separate inline-asm implementation here does duplicate. Aliasing the long double entry points to the double ones in `cygwin.din` would be good as you suggested.
I can look at extending `gendef` to allow either preprocessor support or a minimal arch-conditional syntax so that we can express something like: ``` #if defined(__aarch64__) sqrtl = sqrt NOSIGFE #else sqrtl NOSIGFE #endif ``` Will submit a revised patch shortly.. Thanks & regards Thirumalai Nagalingam -----Original Message----- From: Jeremy Drake <[email protected]> Sent: 13 August 2025 23:04 To: Thirumalai Nagalingam <[email protected]> Cc: [email protected] Subject: Re: [PATCH] Cygwin: math: Add AArch64 support for sqrt() On Tue, 5 Aug 2025, Thirumalai Nagalingam wrote: > Hi all, > > This patch adds support for the `fsqrt` instruction on AArch64 > platforms in the `__FLT_ABI(sqrt)` implementation. This looks OK as far as it goes, but I have a few thoughts. >From the comments, it appears this code originally came from mingw-w64. Their current version of this code has aarch64 implementations. The difference with this one is they have a version for float as well as double. The versions here seem to only be used for long double (which on aarch64 is the same as double). Given that long double is the same as double on aarch64, might it make sense to redirect/alias the long double names to the double implementations in the def file (cygwin.din) on aarch64, rather than providing two different implementations (one in newlib for double and one in this cygwin/math directory for long double)? It seems like that's asking for subtle discrepancies between the implementations. I'm not seeing any obvious preprocesor-like operations in gendef (mingw-w64 uses cpp to preprocess .def.in => .def files for arch-specific #ifdefs) so maybe this would be more complicated. > > In-lined Patch: > > From aee895b4b7c4045dea64d1206731dc01d29c155c Mon Sep 17 00:00:00 2001 > From: Thirumalai Nagalingam > <[email protected]> > Date: Wed, 23 Jul 2025 00:37:09 -0700 > Subject: [PATCH] Cygwin: math: Add AArch64 support for sqrt() > > Extend `__FLT_ABI(sqrt)` implementation to support AArch64 using the > `fsqrt` instruction for double-precision floating point values. > This addition enables square root computation on 64-bit ARM platforms > improving compatibility and performance. > > Signed-off-by: Thirumalai Nagalingam > <[email protected]> > --- > winsup/cygwin/math/sqrt.def.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/winsup/cygwin/math/sqrt.def.h > b/winsup/cygwin/math/sqrt.def.h index 3d1a00908..598614d3a 100644 > --- a/winsup/cygwin/math/sqrt.def.h > +++ b/winsup/cygwin/math/sqrt.def.h > @@ -89,6 +89,8 @@ __FLT_ABI (sqrt) (__FLT_TYPE x) > __fsqrt_internal(x); > #elif defined(_X86_) || defined(__i386__) || defined(_AMD64_) || > defined(__x86_64__) > asm volatile ("fsqrt" : "=t" (res) : "0" (x)); > +#elif defined(__aarch64__) > + asm volatile ("fsqrt %d0, %d1" : "=w"(res) : "w"(x)); > #else > #error Not supported on your platform yet #endif > -- > 2.49.0.windows.1 > > Thanks, > Thirumalai Nagalingam > > >
