Re: [PATCH 1/3] Update s_sincosf.c and x86-64 s_sincosf-fma.c

[Szabolcs Nagy <Szabolcs dot Nagy at arm dot com>]

On 30/11/18 21:31, H.J. Lu wrote:
> On Fri, Nov 30, 2018 at 12:40 PM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>> On 21/11/2018 16:11, H.J. Lu wrote:
>>> Include <s_sincosf.h> in s_sincosf.c, instead of "s_sincosf.h", to allow
>>> x86-64 s_sincosf.h with vectorized sincosf_poly.  Update __sincosf_table
...
>>> --- a/sysdeps/ieee754/flt-32/s_sincosf.h
>>> +++ b/sysdeps/ieee754/flt-32/s_sincosf.h
>>> @@ -31,8 +31,24 @@ typedef struct
>>>    double sign[4];            /* Sign of sine in quadrants 0..3.  */
>>>    double hpi_inv;            /* 2 / PI ( * 2^24 if !TOINT_INTRINSICS).  */
>>>    double hpi;                        /* PI / 2.  */
>>> -  double c0, c1, c2, c3, c4; /* Cosine polynomial.  */
>>> -  double s1, s2, s3;         /* Sine polynomial.  */
>>> +  /* Cosine polynomial: c0, c1, c2, c3, c4.
>>> +     Sine polynomial: s1, s2, s3.  */
>>> +  double c0, c1;
>>> +  struct
>>> +    {
>>> +      double s1;
>>> +      double c2;
>>> +    } s1c2;
>>> +  struct
>>> +    {
>>> +      double s2;
>>> +      double c3;
>>> +    } s2c3;
>>> +  struct
>>> +    {
>>> +      double s3;
>>> +      double c4;
>>> +    } s3c4;
>>>  } sincos_t;
>>>
>>
>> I don't think this should be a problem for other architectures, do you see
>> any possible issue about changing the layout?
> 
> Not I can think of.

if you change the layout it's likely a measurable
regression on aarch64 targets because it was optimized
such that ldp (load pair) can be used when loading
coeffs (although this should not have a huge effect).

i think it should be possible to override the header
and data files with target specific definitions (or
use ifdefs based on some target macros) so we can
have both layouts.