RE: Patch for cacosh

[Michael Morrell <mmorrell@tachyum.com>]

[-- Attachment #1: Type: text/plain, Size: 1773 bytes --]

Here's a revised version that used "int" instead of "_Bool" and signbit().  You were correct that -0 failed.

I'm not sure how to do this by adjusting the input to cacos, so I left that alone.

Thanks for considering taking this patch.   Most of the rest of the failures in the Fortran portion of the gcc validation suite are due to lack of quad math support which I know is something you want to tackle, but is still a ways off.

   Michael

-----Original Message-----
From: Rich Felker <dalias@aerifal.cx> On Behalf Of Rich Felker
Sent: Tuesday, October 1, 2019 4:28 PM
To: musl@lists.openwall.com
Subject: Re: [musl] Patch for cacosh

On Tue, Oct 01, 2019 at 09:57:17PM +0000, Michael Morrell wrote:
> Running the gcc validation suite, I noticed that
> gfortran.dg/complex_intrinsic_5.f90 was failing when using MUSL.
> 
> I tracked it down to the cacosh routines not getting the correct 
> result when the imaginary part of the argument was negative.
> 
> Attached is a patch to fix this.

Thanks!

> diff --git a/src/complex/cacosh.c b/src/complex/cacosh.c index 
> 8c68cb01..0b598052 100644
> --- a/src/complex/cacosh.c
> +++ b/src/complex/cacosh.c
> @@ -4,6 +4,9 @@
>  
>  double complex cacosh(double complex z)  {
> +        _Bool zineg = cimag(z) < 0;
> +

I think this fails to give the desired result for negative zero.
Should probably be signbit(cimag(z)) or similar.

If there's a way to adjust the input to cacos to avoid having to patch up after it returns based on a flag saved before the call, that would make it more efficient I think (no need to spill/reload), but it's not a big deal if not.

Also, as a style matter, musl codebase generally doesn't use _Bool; rather just int for flags/boolean values.

Rich

[-- Attachment #2: cacosh.patch --]
[-- Type: application/octet-stream, Size: 1281 bytes --]

diff --git a/src/complex/cacosh.c b/src/complex/cacosh.c
index 8e42f1ae..76127f75 100644
--- a/src/complex/cacosh.c
+++ b/src/complex/cacosh.c
@@ -4,6 +4,9 @@
 
 double complex cacosh(double complex z)
 {
+	int zineg = signbit(cimag(z));
+
 	z = cacos(z);
-	return CMPLX(-cimag(z), creal(z));
+	if (zineg) return CMPLX(cimag(z), -creal(z));
+	else       return CMPLX(-cimag(z), creal(z));
 }
diff --git a/src/complex/cacoshf.c b/src/complex/cacoshf.c
index d7e6b545..8bd80581 100644
--- a/src/complex/cacoshf.c
+++ b/src/complex/cacoshf.c
@@ -2,6 +2,9 @@
 
 float complex cacoshf(float complex z)
 {
+	int zineg = signbit(cimagf(z));
+
 	z = cacosf(z);
-	return CMPLXF(-cimagf(z), crealf(z));
+	if (zineg) return CMPLXF(cimagf(z), -crealf(z));
+	else       return CMPLXF(-cimagf(z), crealf(z));
 }
diff --git a/src/complex/cacoshl.c b/src/complex/cacoshl.c
index d3eaee20..3a284be9 100644
--- a/src/complex/cacoshl.c
+++ b/src/complex/cacoshl.c
@@ -8,7 +8,10 @@ long double complex cacoshl(long double complex z)
 #else
 long double complex cacoshl(long double complex z)
 {
+	int zineg = signbit(cimagl(z));
+
 	z = cacosl(z);
-	return CMPLXL(-cimagl(z), creall(z));
+	if (zineg) return CMPLXL(cimagl(z), -creall(z));
+	else       return CMPLXL(-cimagl(z), creall(z));
 }
 #endif