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)
- From: Jeff Law <law at redhat dot com>
- To: Christophe Lyon <christophe dot lyon at linaro dot org>, Richard Biener <richard dot guenther at gmail dot com>
- Cc: giuliano dot belinassi at usp dot br, gcc Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 12 Oct 2018 08:14:39 -0600
- Subject: Re: [PATCH] Optimize sin(atan(x)) and cos(atan(x)), take 3 (PR tree-optimization/86829)
- References: <CAEFO=4DcnN6u_aXHiE=r8MUmmPuaBqYLYsw7Yg3fHggQC3_c6g@mail.gmail.com> <e0345985-2d6d-0f22-4a31-c3b2e9885a7c@redhat.com> <CAKdteOYfki8QEv5DQNJZVWTdLCxF90edY0vGN17G1DWSU3AbEg@mail.gmail.com> <CAFiYyc1WiTmyY8Dfxo5eNTBDXxxbPwKjTCfuEpZW==augqDodQ@mail.gmail.com> <CAKdteObPXCJi2Se8gTAjF=90xWENs+1kUjQxQZqhQf0SBi-18Q@mail.gmail.com>
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.
jeff