This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 9/N v2] [x86_64] Vectorized math functions
- From: Joseph Myers <joseph at codesourcery dot com>
- To: Andrew Senkevich <andrew dot n dot senkevich at gmail dot com>
- Cc: libc-alpha <libc-alpha at sourceware dot org>
- Date: Wed, 3 Dec 2014 18:26:52 +0000
- Subject: Re: [PATCH 9/N v2] [x86_64] Vectorized math functions
- Authentication-results: sourceware.org; auth=none
- References: <CAMXFM3uqG=KhwofMe7w-ToPHL0ur+apMT0sh4W82KYHyXnEEKg at mail dot gmail dot com>
On Tue, 2 Dec 2014, Andrew Senkevich wrote:
> +$(objpfx)test-double-vlen2: $(common-objpfx)mathvec/libmvec.so \
> + $(objpfx)init-arch.o
> +$(objpfx)test-double-vlen4: $(common-objpfx)mathvec/libmvec.so \
> + $(objpfx)init-arch.o
Depending on $(common-objpfx)mathvec/libmvec.so would break testing
--disable-shared (it may be broken anyway, but there's no need to add more
breakage). It's better to add a variable libmvec to Makeconfig like the
existing libm variable, then use it everywhere you have a test using this
library. Abstracting out paths to libraries like that may also make it
easier to support testing installed glibc in future.
> +CFLAGS-test-double-vlen2.c = -fno-inline -ffloat-store -fno-builtin
> -frounding-math \
> + -D__FAST_MATH__ -DTEST_FAST_MATH -D_OPENMP=201307 \
> + -Wno-unknown-pragmas
> +CFLAGS-test-double-vlen4.c = -fno-inline -ffloat-store -fno-builtin
> -frounding-math \
> + -D__FAST_MATH__ -DTEST_FAST_MATH -D_OPENMP=201307 \
> + -Wno-unknown-pragmas $(arch-ext-cflags)
Why is $(arch-ext-cflags) only in one of these two settings? Even if you
don't in fact need those flags for x86_64, this is a generic file, so it
would seem appropriate to be consistent here.
I think some refactoring is needed to reduce duplication of such settings,
here and in sysdeps makefiles.
* First a preliminary patch could eliminate existing references to
-frounding-math other than the global one in Makeconfig - with that global
one, there's no need for it to be specified for individual files as well.
* Then the existing flag settings could be refactored so that settings
used for groups of related tests use a single variable rather than being
duplicated in lots of CFLAGS-* variables.
* Then you might have a libm-test-vector-cflags variable (for example),
that's based on a common libm-test-fast-math-cflags variable (for
example), also used for the ifloat.c etc. tests, and adds just the
vector-test-specific settings such as -D_OPENMP=201307. Both
CFLAGS-test-double-vlen2.c and CFLAGS-test-double-vlen4.c could just be
set to $(libm-test-vector-cflags), as would
CFLAGS-test-double-vlen4-avx2.c in the sysdeps makefile.
> +#define TEST_VEC_LOOP(len) \
> + do \
> + { \
> + for (i=1; i<len; i++) \
> + { \
> + if (((FLOAT *) &mr)[0] != ((FLOAT *) &mr)[i]) \
> + { \
> + return ((FLOAT *) &mr)[0]+0.1; \
> + } \
> + } \
> + return ((FLOAT *) &mr)[0]; \
> + } \
> + while (0)
Spaces around binary operators ('=', '<', '+'). Put this function in a
common header rather than duplicating it in test-double-vlen2.h and
test-double-vlen4.h.
> + TEST_VEC_LOOP(2); \
Space before '('.
> + TEST_VEC_LOOP(4); \
Likewise.
> + TEST_VEC_LOOP(4); \
Likewise.
--
Joseph S. Myers
joseph@codesourcery.com