musl - RE: Patch for cacosh

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d90ed963baad4b4fa5475b7b7643a0c5@tachyum.com>
Date: Wed, 2 Oct 2019 16:45:14 +0000
From: Michael Morrell <mmorrell@...hyum.com>
To: "musl@...ts.openwall.com" <musl@...ts.openwall.com>
Subject: RE: Patch for cacosh

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@...ifal.cx> On Behalf Of Rich Felker
Sent: Tuesday, October 1, 2019 4:28 PM
To: musl@...ts.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

Download attachment "cacosh.patch" of type "application/octet-stream" (1281 bytes)

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.