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.

> > 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?

Richard.

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

Reply via email to