On 2025-02-18 13:30, Richard Biener wrote:
On Tue, Feb 18, 2025 at 1:54 PM Peter0x44 <peter0...@disroot.org> wrote:

18 Feb 2025 8:51:16 am Richard Biener <richard.guent...@gmail.com>:

> On Tue, Feb 18, 2025 at 1:21 AM Sam James <s...@gentoo.org> wrote:
>>
>> Peter Damianov <peter0...@disroot.org> writes:
>>
>>> POSIX says that sin and cos should set errno to EDOM when infinity is
>>> passed to
>>> them. Make sure this is accounted for in builtins.def, and add tests.
>>>
>>> gcc/
>>>       PR middle-end/80042
>>>       * builtins.def: (sin|cos)(f|l) can set errno.
>>> gcc/testsuite/
>>>       * gcc.dg/pr80042.c: New testcase.
>>> ---
>>> gcc/builtins.def               | 20 +++++-----
>>> gcc/testsuite/gcc.dg/pr80042.c | 71
>>> ++++++++++++++++++++++++++++++++++
>>> 2 files changed, 82 insertions(+), 9 deletions(-)
>>> create mode 100644 gcc/testsuite/gcc.dg/pr80042.c
>>>
>>> [...]
>>> diff --git a/gcc/testsuite/gcc.dg/pr80042.c
>>> b/gcc/testsuite/gcc.dg/pr80042.c
>>> new file mode 100644
>>> index 00000000000..cc578ae67e2
>>> --- /dev/null
>>> +++ b/gcc/testsuite/gcc.dg/pr80042.c
>>> @@ -0,0 +1,71 @@
>>> +/* dg-do run */
>>> +/* dg-options "-O2 -lm" */
>>
>> These two lines are missing {}. Please double check the logs from your
>> testsuite run to make sure newly added/changed tests are executed (and
>> in the way you expect).
>
> This test will also FAIL on *BSD IIRC as that doesn't set errno for any
> math
> functions.

So what do you suggest I do about it? Drop the test, or only enable it
for certain known good targets?
I don't use BSD so cannot test it.

Good question.  It's also that old glibc did not set errno here.

>
> I'll note GCC models sincos as cexpi which does not set errno, and will
> eventually expand that to sincos or cexp.  It does that without any
> restriction on -fno-math-errno.

Is this a problem? Would I need to disable expansion to cexp with
-fmath-errno make this work?

I think that the code might assume sin()/cos() is always CONST/PURE
and that for "POSIX-y correctness" we'd have to guard the transform
with -fno-math-errno.

Okay. I will look at doing that.


> I'll also note the C standard does not document any domain error on +-
> Inf arguments.
> Instead it documents a range error for sin(x) and nonzero x too close
> to zero.

https://pubs.opengroup.org/onlinepubs/9699919799/functions/sin.html
POSIX does specify it should be a domain error, but C itself doesn't seem
to say anything regarding it other than basically "implementations are
allowed to invent errors for this case".

So what's the point of your patch?  That GCC does not assume sin/cos
will not clobber errno? Maybe the testcase can be rewritten to consider that? Like check that we did not fold the != EDOM checks at compile-time
instead of hard-requiring the library to set that error?

Yes, that's the point. I'm not really sure how to check that specifically instead of executing the code, but I should figure it out.

I think a test written in this way would also avoid the mentioned problems of the libraries which don't set errno.


Richard.

>
> Richard.
>
>>
>>> [...]

Reply via email to