[13/14,x86_64] Vector math functions (sincos and tests)

Message ID CAMXFM3tzxuVj06vQwWWc4JBXd8MAtLJYOia3mpqViY7w6Bt08g@mail.gmail.com
State Superseded
Headers

Commit Message

Andrew Senkevich June 19, 2015, 12:13 p.m. UTC
  2015-06-19 14:25 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
> On Fri, 19 Jun 2015, Andrew Senkevich wrote:
>
>> Proposal is fix it so:
>>
>> diff --git a/bits/libm-simd-decl-stubs.h b/bits/libm-simd-decl-stubs.h
>> index ec1fa69..6d0558a 100644
>> --- a/bits/libm-simd-decl-stubs.h
>> +++ b/bits/libm-simd-decl-stubs.h
>> @@ -45,6 +45,10 @@
>>  #define __DECL_SIMD_sincosf
>>  #define __DECL_SIMD_sincosl
>>
>> +/* This is needed because of definition of sincos
>> +   in sysdeps/ieee754/ldbl-opt/s_sin.c.  */
>> +# define __DECL_SIMD_sincos_disable
>> +
>
> It would seem better to me to put this in the .c file that needs it,
> rather than in an installed header - it should only be relevant when
> building glibc, not when using the installed library.


Ok?


--
WBR,
Andrew
  

Comments

Tulio Magno Quites Machado Filho June 19, 2015, 1:33 p.m. UTC | #1
Andrew Senkevich <andrew.n.senkevich@gmail.com> writes:

> 2015-06-19 14:25 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>> On Fri, 19 Jun 2015, Andrew Senkevich wrote:
>>
>>> Proposal is fix it so:
>>>
>>> diff --git a/bits/libm-simd-decl-stubs.h b/bits/libm-simd-decl-stubs.h
>>> index ec1fa69..6d0558a 100644
>>> --- a/bits/libm-simd-decl-stubs.h
>>> +++ b/bits/libm-simd-decl-stubs.h
>>> @@ -45,6 +45,10 @@
>>>  #define __DECL_SIMD_sincosf
>>>  #define __DECL_SIMD_sincosl
>>>
>>> +/* This is needed because of definition of sincos
>>> +   in sysdeps/ieee754/ldbl-opt/s_sin.c.  */
>>> +# define __DECL_SIMD_sincos_disable
>>> +
>>
>> It would seem better to me to put this in the .c file that needs it,
>> rather than in an installed header - it should only be relevant when
>> building glibc, not when using the installed library.
>
> diff --git a/ChangeLog b/ChangeLog
> index 7dedfb4..75f926c 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2015-06-19  Andrew Senkevich  <andrew.senkevich@intel.com>
> +
> +       * sysdeps/ieee754/ldbl-opt/s_sin.c (__DECL_SIMD_sincos_disable): Added
> +       empty definition for proper unfolding of __MATHDECL_VEC.
> +
>  2015-06-19  Stefan Liebler  <stli@linux.vnet.ibm.com>
>
>         * benchtests/bench-strcoll.c:
> diff --git a/sysdeps/ieee754/ldbl-opt/s_sin.c b/sysdeps/ieee754/ldbl-opt/s_sin.c
> index a11d5a3..884bc51 100644
> --- a/sysdeps/ieee754/ldbl-opt/s_sin.c
> +++ b/sysdeps/ieee754/ldbl-opt/s_sin.c
> @@ -1,5 +1,7 @@
>  /* dbl-64/s_sin.c uses NAN and sincos identifiers internally.  */
>  #define sincos sincos_disable
> +/* Stab for proper __MATHDECL_VEC unfolding.  */
> +#define __DECL_SIMD_sincos_disable
>  #include <math_ldbl_opt.h>
>  #undef NAN
>  #undef sincos
>
> Ok?

It doesn't fix the problem.
Which part of the code will use this macro?
  
Andrew Senkevich June 19, 2015, 2:46 p.m. UTC | #2
2015-06-19 16:33 GMT+03:00 Tulio Magno Quites Machado Filho
<tuliom@linux.vnet.ibm.com>:
> Andrew Senkevich <andrew.n.senkevich@gmail.com> writes:
>
>> 2015-06-19 14:25 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>>> On Fri, 19 Jun 2015, Andrew Senkevich wrote:
>>>
>>>> Proposal is fix it so:
>>>>
>>>> diff --git a/bits/libm-simd-decl-stubs.h b/bits/libm-simd-decl-stubs.h
>>>> index ec1fa69..6d0558a 100644
>>>> --- a/bits/libm-simd-decl-stubs.h
>>>> +++ b/bits/libm-simd-decl-stubs.h
>>>> @@ -45,6 +45,10 @@
>>>>  #define __DECL_SIMD_sincosf
>>>>  #define __DECL_SIMD_sincosl
>>>>
>>>> +/* This is needed because of definition of sincos
>>>> +   in sysdeps/ieee754/ldbl-opt/s_sin.c.  */
>>>> +# define __DECL_SIMD_sincos_disable
>>>> +
>>>
>>> It would seem better to me to put this in the .c file that needs it,
>>> rather than in an installed header - it should only be relevant when
>>> building glibc, not when using the installed library.
>>
>> diff --git a/ChangeLog b/ChangeLog
>> index 7dedfb4..75f926c 100644
>> --- a/ChangeLog
>> +++ b/ChangeLog
>> @@ -1,3 +1,8 @@
>> +2015-06-19  Andrew Senkevich  <andrew.senkevich@intel.com>
>> +
>> +       * sysdeps/ieee754/ldbl-opt/s_sin.c (__DECL_SIMD_sincos_disable): Added
>> +       empty definition for proper unfolding of __MATHDECL_VEC.
>> +
>>  2015-06-19  Stefan Liebler  <stli@linux.vnet.ibm.com>
>>
>>         * benchtests/bench-strcoll.c:
>> diff --git a/sysdeps/ieee754/ldbl-opt/s_sin.c b/sysdeps/ieee754/ldbl-opt/s_sin.c
>> index a11d5a3..884bc51 100644
>> --- a/sysdeps/ieee754/ldbl-opt/s_sin.c
>> +++ b/sysdeps/ieee754/ldbl-opt/s_sin.c
>> @@ -1,5 +1,7 @@
>>  /* dbl-64/s_sin.c uses NAN and sincos identifiers internally.  */
>>  #define sincos sincos_disable
>> +/* Stab for proper __MATHDECL_VEC unfolding.  */
>> +#define __DECL_SIMD_sincos_disable
>>  #include <math_ldbl_opt.h>
>>  #undef NAN
>>  #undef sincos
>>
>> Ok?
>
> It doesn't fix the problem.

Could you please explain why with more details (exact compilation
command caused fail will the best help)?

> Which part of the code will use this macro?

Macro needed for unfolding of __MATHDECL_VEC from
math/bits/mathcalls.h included from math.h included from
math_ldbl_opt.h here.


--
WBR,
Andrew
  
Tulio Magno Quites Machado Filho June 19, 2015, 4:21 p.m. UTC | #3
Andrew Senkevich <andrew.n.senkevich@gmail.com> writes:

> 2015-06-19 16:33 GMT+03:00 Tulio Magno Quites Machado Filho
> <tuliom@linux.vnet.ibm.com>:
>> Andrew Senkevich <andrew.n.senkevich@gmail.com> writes:
>>
>>> 2015-06-19 14:25 GMT+03:00 Joseph Myers <joseph@codesourcery.com>:
>>>> On Fri, 19 Jun 2015, Andrew Senkevich wrote:
>>>>
>>>>> Proposal is fix it so:
>>>>>
>>>>> diff --git a/bits/libm-simd-decl-stubs.h b/bits/libm-simd-decl-stubs.h
>>>>> index ec1fa69..6d0558a 100644
>>>>> --- a/bits/libm-simd-decl-stubs.h
>>>>> +++ b/bits/libm-simd-decl-stubs.h
>>>>> @@ -45,6 +45,10 @@
>>>>>  #define __DECL_SIMD_sincosf
>>>>>  #define __DECL_SIMD_sincosl
>>>>>
>>>>> +/* This is needed because of definition of sincos
>>>>> +   in sysdeps/ieee754/ldbl-opt/s_sin.c.  */
>>>>> +# define __DECL_SIMD_sincos_disable
>>>>> +
>>>>
>>>> It would seem better to me to put this in the .c file that needs it,
>>>> rather than in an installed header - it should only be relevant when
>>>> building glibc, not when using the installed library.
>>>
>>> diff --git a/ChangeLog b/ChangeLog
>>> index 7dedfb4..75f926c 100644
>>> --- a/ChangeLog
>>> +++ b/ChangeLog
>>> @@ -1,3 +1,8 @@
>>> +2015-06-19  Andrew Senkevich  <andrew.senkevich@intel.com>
>>> +
>>> +       * sysdeps/ieee754/ldbl-opt/s_sin.c (__DECL_SIMD_sincos_disable): Added
>>> +       empty definition for proper unfolding of __MATHDECL_VEC.
>>> +
>>>  2015-06-19  Stefan Liebler  <stli@linux.vnet.ibm.com>
>>>
>>>         * benchtests/bench-strcoll.c:
>>> diff --git a/sysdeps/ieee754/ldbl-opt/s_sin.c b/sysdeps/ieee754/ldbl-opt/s_sin.c
>>> index a11d5a3..884bc51 100644
>>> --- a/sysdeps/ieee754/ldbl-opt/s_sin.c
>>> +++ b/sysdeps/ieee754/ldbl-opt/s_sin.c
>>> @@ -1,5 +1,7 @@
>>>  /* dbl-64/s_sin.c uses NAN and sincos identifiers internally.  */
>>>  #define sincos sincos_disable
>>> +/* Stab for proper __MATHDECL_VEC unfolding.  */
>>> +#define __DECL_SIMD_sincos_disable
>>>  #include <math_ldbl_opt.h>
>>>  #undef NAN
>>>  #undef sincos
>>>
>>> Ok?
>>
>> It doesn't fix the problem.
>
> Could you please explain why with more details (exact compilation
> command caused fail will the best help)?

Sure!
All the log is available in the community BuildBot at
http://glibc-build.hack.frob.com/waterfall

The log of the build is here:
http://130.211.48.148:8080/builders/glibc-power8-linux/builds/0/steps/annotate/logs/stdio
  
Joseph Myers June 19, 2015, 5:06 p.m. UTC | #4
On Fri, 19 Jun 2015, Tulio Magno Quites Machado Filho wrote:

> >> It doesn't fix the problem.
> >
> > Could you please explain why with more details (exact compilation
> > command caused fail will the best help)?
> 
> Sure!
> All the log is available in the community BuildBot at
> http://glibc-build.hack.frob.com/waterfall
> 
> The log of the build is here:
> http://130.211.48.148:8080/builders/glibc-power8-linux/builds/0/steps/annotate/logs/stdio

Preprocessed source might help as well, to show what the code looks like 
that produces the parse errors and where it appears in the sequence of 
nested includes.  I wonder if, as well as __DECL_SIMD_sincos_disable, 
__DECL_SIMD_sincos_disablef and __DECL_SIMD_sincos_disablel need to be 
defined.
  

Patch

diff --git a/ChangeLog b/ChangeLog
index 7dedfb4..75f926c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@ 
+2015-06-19  Andrew Senkevich  <andrew.senkevich@intel.com>
+
+       * sysdeps/ieee754/ldbl-opt/s_sin.c (__DECL_SIMD_sincos_disable): Added
+       empty definition for proper unfolding of __MATHDECL_VEC.
+
 2015-06-19  Stefan Liebler  <stli@linux.vnet.ibm.com>

        * benchtests/bench-strcoll.c:
diff --git a/sysdeps/ieee754/ldbl-opt/s_sin.c b/sysdeps/ieee754/ldbl-opt/s_sin.c
index a11d5a3..884bc51 100644
--- a/sysdeps/ieee754/ldbl-opt/s_sin.c
+++ b/sysdeps/ieee754/ldbl-opt/s_sin.c
@@ -1,5 +1,7 @@ 
 /* dbl-64/s_sin.c uses NAN and sincos identifiers internally.  */
 #define sincos sincos_disable
+/* Stab for proper __MATHDECL_VEC unfolding.  */
+#define __DECL_SIMD_sincos_disable
 #include <math_ldbl_opt.h>
 #undef NAN
 #undef sincos