Re: [RFC] How to add vector math functions to Glibc

[Andrew Senkevich <andrew dot n dot senkevich at gmail dot com>]

2014-10-24 1:37 GMT+04:00 Joseph S. Myers <joseph@codesourcery.com>:

>> +static int avx2_usable;              /* Set to 1 if AVX2 supported */
>
> Given that we expect multiple architectures to have vector functions,
> this belongs in some architecture-specific file that libm-test.inc
> includes, rather than directly in libm-test.inc (which shouldn't refer
> directly to AVX at all).

>which shouldn't refer directly to AVX at all
Do you mean to place avx2_usable initialization in procedure in
architecture-specific *.c file and have generic stub, call it from
test main() and change build accordingly?
May be simply stay __cpu_features.feature[index_AVX2_Usable] &
bit_AVX2_Usable in every test function inserted through macros? It
don't require so big changes and don't affect performance
significantly. Or insert initialization in test's main() through
macros also.

>> -#define RUN_TEST_f_f(ARG_STR, FUNC_NAME, ARG, EXPECTED,                      \
>> -                  EXCEPTIONS)                                        \
>> -  do                                                                 \
>> -    if (enable_test (EXCEPTIONS))                                    \
>> -      {                                                                      \
>> -     COMMON_TEST_SETUP (ARG_STR);                                    \
>> -     check_float (test_name, FUNC (FUNC_NAME) (ARG), EXPECTED,       \
>> -                  EXCEPTIONS);                                       \
>> -     COMMON_TEST_CLEANUP;                                            \
>> -      }                                                                      \
>> +#define RUN_TEST_f_f(ARG_STR, FUNC_NAME, ARG, EXPECTED,                              \
>> +                  EXCEPTIONS)                                                \
>> +  do                                                                         \
>> +    if (enable_test (EXCEPTIONS))                                            \
>> +      {                                                                              \
>> +     COMMON_TEST_SETUP (ARG_STR);                                            \
>> +     check_float (test_name,                                                 \
>> +                  CONCAT (CONCAT3_1 (VEC_PREFIX_, FUNC_NAME, FUNC ( )),      \
>> +                          FUNC (FUNC_NAME)) (ARG),                           \
>> +                          EXPECTED,                                          \
>> +                  EXCEPTIONS);                                               \
>> +     COMMON_TEST_CLEANUP;                                                    \
>> +      }                                                                              \
>
> I think it would be better for FUNC to be defined, in the test file that
> includes libm-test.inc, in a way that avoids the need for the CONCAT*
> calls here.  (To avoid warnings / errors about undeclared functions, I
> suppose the generated header might then need to redefine e.g. vec_sin to
> sin if there isn't a vector version of sin.)

Not good idea to change FUNC definition since it used in libm-test.c
not only in test macros (so it may cause usage of vector function with
not vector parameter). But it is possible to reduce number of
concatenation if change generated definitions in way your have
proposed.

Not all functions tested trough ALL_RM_TEST - cexp, tgamma and jn
tested in not all rounding modes, so definitions for them we have to
generate in script manually.

>> diff --git a/sysdeps/x86_64/fpu/Makefile b/sysdeps/x86_64/fpu/Makefile
>> new file mode 100644
>> index 0000000..d585fa0
>> --- /dev/null
>> +++ b/sysdeps/x86_64/fpu/Makefile
>> @@ -0,0 +1,33 @@
>> +ifeq ($(subdir),mathvec)
>> +libmvec-support += svml_d_cos4_core svml_d_cos_data
>> +endif
>> +
>> +# Rules for libmvec tests
>> +ifeq ($(subdir),math)
>> +ifneq ($(PERL),no)
>> +ifeq ($(build-mathvec),yes)
>> +libm-tests += test-double-vlen4 test-float-vlen8
>> +
>> +CFLAGS-test-double-vlen4-wrapper.c = -fno-inline -ffloat-store -fno-builtin \
>> +                                  -frounding-math -mavx2
>> +CFLAGS-test-float-vlen8-wrapper.c = -fno-inline -ffloat-store -fno-builtin \
>> +                                 -frounding-math -mavx2
>
> I think the sysdeps makefile should actually just define that double-vlen4
> and float-vlen8 are the vector lengths for which testing should take
> place, with all the other testing rules being arranged in an
> architecture-independent way.

Do you mean to stay in sysdeps/x86_64/fpu/Makefile only CFLAGS-*
definitions or to setup some variable which will be used in common
makefile for build vector tests?

>> diff --git a/sysdeps/x86_64/fpu/test-double-vlen4-wrapper.c b/sysdeps/x86_64/fpu/test-double-vlen4-wrapper.c
>> new file mode 100644
>> index 0000000..0778e23
>> --- /dev/null
>> +++ b/sysdeps/x86_64/fpu/test-double-vlen4-wrapper.c
>
> This file may well need to be architecture-specific, at least as written,
> but ...
>
>> diff --git a/sysdeps/x86_64/fpu/test-double-vlen4.c b/sysdeps/x86_64/fpu/test-double-vlen4.c
>> new file mode 100644
>> index 0000000..4d3d9a3
>> --- /dev/null
>> +++ b/sysdeps/x86_64/fpu/test-double-vlen4.c
>
>  ... it's not at all clear that this one should need to be.  At present it
> has some architecture-specific bits
>
>> +#define CHECK_ARCH_EXT if (!avx2_usable) return;
>> +
>> +extern FLOAT WRAPPER_NAME (cos) (FLOAT);
>
> but I'd think those are all that needs to go somewhere
> architecture-specific and the rest is pretty generic to any architecture
> supporting vector functions for vectors of 4 doubles.

Then lets have math/test-double-vlen4.h with common definitions and
sysdeps/x86_64/fpu/test-double-vlen4.c containing wrapper.
Attached patch contains only discussed here changes.


--
WBR,
Andrew

Attachment: tmp29.10.patch
Description: Binary data