Re: [PATCH 2/14] [x86_64] Vector math functions (added vector cos tests)

[Joseph Myers <joseph at codesourcery dot com>]

On Thu, 30 Apr 2015, Andrew Senkevich wrote:

> This is update of test suite infrastracture with addition of tests for
> vectorized cos.

Could you please give a more detailed explanation of what the patch does 
and the rationale for the choices made?  Even if it was included in 
discussion of previous versions of this patch, every patch submission / 
resubmission should best include a self-contained write-up of the patch 
that will go in the git commit message.  (See examples of other patch 
submissions that clearly distinguish the self-contained write-up for the 
current patch - for the git commit message - and the description of 
changes from previous versions of the patch - not for the git commit 
message.)

>         * math/Makefile: Added rules for tests.
>         * sysdeps/x86_64/fpu/Makefile: Likewise.
>         * math/test-double-vlen2.h: New file.
>         * math/test-double-vlen4.h: New file.
>         * math/test-double-vlen8.h: New file.
>         * math/test-vec-loop.h: New file.
>         * sysdeps/x86_64/fpu/test-double-vlen2.c: New file.
>         * sysdeps/x86_64/fpu/test-double-vlen4-avx2.c: New file.
>         * sysdeps/x86_64/fpu/test-double-vlen4.c: New file.
>         * sysdeps/x86_64/fpu/test-double-vlen8.c: New file.
>         * sysdeps/x86_64/fpu/test-double-vlen8-wrappers.c: New file.

Why does vlen8 need a separate -wrappers file when others don't?

> diff --git a/math/Makefile b/math/Makefile

> +$(objpfx)test-double-vlen2.o: $(objpfx)libm-test.stmp
> +$(objpfx)test-double-vlen4.o: $(objpfx)libm-test.stmp
> +
> +$(objpfx)test-double-vlen2: $(libmvec)
> +$(objpfx)test-double-vlen4: $(libmvec)

Why are only vlen2 and vlen4 mentioned here, not vlen8?  I don't see any 
good architecture-independent reason for such a difference.  Indeed, with 
appropriate use of GNU make text-manipulation functions, it should be 
possible to write

$(something): $(objpfx)libm-test.stmp
$(something): $(libmvec)

where each $(something) is computed from $(libmvec-tests), and so the 
dependencies are automatically present for all the objects, including the 
architecture-specific ones such as vlen4-avx2.

> +CFLAGS-test-double-vlen8-wrappers.c = $(double-vlen8-arch-ext-cflags)

For this to go in math/Makefile, there needs to be an 
architecture-independent reason for such a -wrappers file to exist in the 
first place.

> +/* Copyright (C) 2014 Free Software Foundation, Inc.

2014-2015, in all new files.

-- 
Joseph S. Myers
joseph@codesourcery.com