This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Optimize sin(atan(x)) and cos(atan(x)), take 3 (PR tree-optimization/86829)
On Fri, 12 Oct 2018 at 16:14, Jeff Law <law@redhat.com> wrote:
>
> On 10/12/18 8:11 AM, Christophe Lyon wrote:
> > On Fri, 12 Oct 2018 at 13:27, Richard Biener <richard.guenther@gmail.com> wrote:
> >>
> >> On Fri, Oct 12, 2018 at 10:08 AM Christophe Lyon
> >> <christophe.lyon@linaro.org> wrote:
> >>>
> >>> On Thu, 11 Oct 2018 at 23:07, Jeff Law <law@redhat.com> wrote:
> >>>>
> >>>> On 10/9/18 5:29 PM, Giuliano Augusto Faulin Belinassi wrote:
> >>>>> Fixed all issues pointed in the previous iteration.
> >>>>> There is now a significant change regarding how the sin(atan(x))
> >>>>> constant is calculated, as now it checks for which values such that
> >>>>> computing 1 + x*x won't overflow. There are two reasons for this
> >>>>> change: (1) Avoid an intermediate infinity value when optimizing
> >>>>> cos(atan(x)), and (2) avoid the requirement of separate constants for
> >>>>> sin(atan(x)) and cos(atan(x)), thus making easier to maintain the
> >>>>> code.
> >>>>>
> >>>>> gcc/ChangeLog
> >>>>>
> >>>>> 2018-10-09 Giuliano Belinassi <giuliano.belinassi@usp.br>
> >>>>>
> >>>>> PR tree-optimization/86829
> >>>>> * match.pd: Added sin(atan(x)) and cos(atan(x)) simplification rules.
> >>>>> * real.c (build_sinatan_real): New function to build a constant equal to the
> >>>>> largest value c such that 1 + c*c will not overflow.
> >>>>> * real.h (build_sinatan_real): Allows this function to be called externally.
> >>>>>
> >>>>> gcc/testsuite/gcc.dg/ChangeLog
> >>>>>
> >>>>> 2018-10-09 Giuliano Belinassi <giuliano.belinassi@usp.br>
> >>>>>
> >>>>> PR tree-optimization/86829
> >>>>> * gcc.dg/sinatan-1.c: New test.
> >>>>> * gcc.dg/sinatan-2.c: New test.
> >>>>> * gcc.dg/sinatan-3.c: New test.
> >>>>>
> >>>>> There are no tests broken in trunk that seems related to this PR.
> >>>> THanks. I've installed this onto the trunk. It's right at the
> >>>> borderline of what would require a copyright assignment. So if you're
> >>>> going to do further work on GCC you should go ahead and start the
> >>>> copyright assignment process.
> >>>>
> >>>> Jeff
> >>>
> >>> Hi!
> >>>
> >>> The new sinatan-1.c test fails to link against newlib on aarch64-elf:
> >>> /tmp/ccmp5fP4.o: In function `sinatanl':
> >>> sinatan-1.c:(.text+0x68): undefined reference to `atanl'
> >>> sinatan-1.c:(.text+0x70): undefined reference to `sinl'
> >>> /tmp/ccmp5fP4.o: In function `cosatanl':
> >>> sinatan-1.c:(.text+0x80): undefined reference to `atanl'
> >>> sinatan-1.c:(.text+0x88): undefined reference to `cosl'
> >>> collect2: error: ld returned 1 exit status
> >>>
> >>> I'm not familiar enough with newlib to know if it's a newlib bug, or
> >>> if we should skip the test?
> >>>
> >>> On arm-eabi, the same test fails at runtime, but I haven't yet taken
> >>> the time to reproduce it manually.
> >>
> >> target_c99_math might do the trick
> >>
> >
> > Yes, if we want to skip the test, but I'm not sure about that?
> > On arm-eabi, adding some traces in the test indicates that the outputs
> > of cosatanf, cosatan and cosatanl are wrong.
> I think skipping on the newlib targets is fine. I'm much more concerned
> about the execution tests -- even if it's the library implementation
> that is wrong, it's going to be painful to manage the xfails over time.
>
The two problems I noticed are both with newlib:
- link failure on aarch64-elf
- runtime error on arm-eabi
A little bit more info on the arm-eabi problem:
The testcase has:
float fc = nextafterf (sqrtf (__FLT_MAX__ - 1), __FLT_MAX__);
[...]
fy = cosatanf (fc);
fc is built with:
mov r0, #0
movt r0, 24448
so r0=0x5f800000 (1.8446744E19) which looks ok
but then cosatanf is computed as (ie there's not call to cosatanf()):
movw r3, #48430
movt r3, 45883
so r3=0xb33bbd2e (-4.371139E-8) which is not zero.
cosatan and cosatanl are called and seem to return wrong values too,
but I haven't stepped into them yet.
Christophe