Re: [PATCH v4] rs6000: Update the vsx-vector-6.* tests.

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Carl,

on 2023/7/6 23:33, Carl Love wrote:
> GCC maintainers:
> 
> Ver 4. Fixed a few typos.  Redid the tests to create separate run and
> compile tests.

Thanks!  This new version looks good, excepting that we need vsx_hw
for run and two nits, see below.

> 
> Ver 3.  Added __attribute__ ((noipa)) to the test files.  Changed some
> of the scan-assembler-times checks to cover multiple similar
> instructions.  Change the function check macro to a macro to generate a
> function to do the test and check the results.  Retested on the various
> processor types and BE/LE versions.
> 
> Ver 2.  Switched to using code macros to generate the call to the
> builtin and test the results.  Added in instruction counts for the key
> instruction for the builtin.  Moved the tests into an additional
> function call to ensure the compile doesn't replace the builtin call
> code with the statically computed results.  The compiler was doing this
> for a few of the simpler tests.  
> 
> The following patch takes the tests in vsx-vector-6-p7.h,  vsx-vector-
> 6-p8.h, vsx-vector-6-p9.h and reorganizes them into a series of smaller
> test files by functionality rather than processor version.
> 
> Tested the patch on Power 8 LE/BE, Power 9 LE/BE and Power 10 LE with
> no regresions.
> 
> Please let me know if this patch is acceptable for mainline.  Thanks.
> 
>                        Carl
> 
> 
> 
> -----------------------------------------------------------------
> rs6000: Update the vsx-vector-6.* tests.
> 
> The vsx-vector-6.h file is included into the processor specific test files
> vsx-vector-6.p7.c, vsx-vector-6.p8.c, and vsx-vector-6.p9.c.  The .h file
> contains a large number of vsx vector builtin tests.  The processor
> specific files contain the number of instructions that the tests are
> expected to generate for that processor.  The tests are compile only.
> 
> This patch reworks the tests into a series of files for related tests.
> The new tests consist of a runnable test to verify the builtin argument
> types and the functional correctness of each builtin.  There is also a
> compile only test that verifies the builtins generate the expected number
> of instructions for the various builtin tests.
> 
> gcc/testsuite/
> 	* gcc.target/powerpc/vsx-vector-6-func-1op.h: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-1op-run.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-1op-compile.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-2lop.h: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-2lop-run.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-2lop-compile.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-2op.h: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-2op-run.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-2op-compile.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-3op.h: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-3op-run.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-3op-compile.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-cmp-all.h: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-cmp-all-run.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-cmp-all-compile.c: New test
> 	file.
> 	* gcc.target/powerpc/vsx-vector-6-func-cmp.h: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-cmp-run.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6-func-cmp-compile.c: New test file.
> 	* gcc.target/powerpc/vsx-vector-6.h: Remove test file.
> 	* gcc.target/powerpc/vsx-vector-6.p7.c: Remove test file.
> 	* gcc.target/powerpc/vsx-vector-6.p8.c: Remove test file.
> 	* gcc.target/powerpc/vsx-vector-6.p9.c: Remove test file.
> ---
>  .../powerpc/vsx-vector-6-func-1op-compile.c   |  22 ++
>  .../powerpc/vsx-vector-6-func-1op-run.c       |  98 ++++++++
>  .../powerpc/vsx-vector-6-func-1op.h           |  43 ++++
>  .../powerpc/vsx-vector-6-func-2lop-compile.c  |  14 ++
>  .../powerpc/vsx-vector-6-func-2lop-run.c      | 177 ++++++++++++++
>  .../powerpc/vsx-vector-6-func-2lop.h          |  47 ++++
>  .../powerpc/vsx-vector-6-func-2op-compile.c   |  21 ++
>  .../powerpc/vsx-vector-6-func-2op-run.c       |  96 ++++++++
>  .../powerpc/vsx-vector-6-func-2op.h           |  42 ++++
>  .../powerpc/vsx-vector-6-func-3op-compile.c   |  17 ++
>  .../powerpc/vsx-vector-6-func-3op-run.c       | 229 ++++++++++++++++++
>  .../powerpc/vsx-vector-6-func-3op.h           |  73 ++++++
>  .../vsx-vector-6-func-cmp-all-compile.c       |  17 ++
>  .../powerpc/vsx-vector-6-func-cmp-all-run.c   | 147 +++++++++++
>  .../powerpc/vsx-vector-6-func-cmp-all.h       |  76 ++++++
>  .../powerpc/vsx-vector-6-func-cmp-compile.c   |  16 ++
>  .../powerpc/vsx-vector-6-func-cmp-run.c       |  92 +++++++
>  .../powerpc/vsx-vector-6-func-cmp.h           |  40 +++
>  .../gcc.target/powerpc/vsx-vector-6.h         | 154 ------------
>  .../gcc.target/powerpc/vsx-vector-6.p7.c      |  43 ----
>  .../gcc.target/powerpc/vsx-vector-6.p8.c      |  43 ----
>  .../gcc.target/powerpc/vsx-vector-6.p9.c      |  42 ----
>  22 files changed, 1267 insertions(+), 282 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-compile.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-run.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op.h
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-2lop-compile.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-2lop-run.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-2lop.h
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-2op-compile.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-2op-run.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-2op.h
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-3op-compile.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-3op-run.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-3op.h
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-cmp-all-compile.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-cmp-all-run.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-cmp-all.h
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-cmp-compile.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-cmp-run.c
>  create mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-cmp.h
>  delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.h
>  delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p7.c
>  delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p8.c
>  delete mode 100644 gcc/testsuite/gcc.target/powerpc/vsx-vector-6.p9.c
> 
> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-compile.c b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-compile.c
> new file mode 100644
> index 00000000000..6b7d73ed66c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-compile.c

Nit: Maybe remove "-compile" from the name as when there is "-run" variant people
are easy to realize this is for compilation, the name without "-compile" seems
more neat.  With this name change, you have to update the comment referring it in
its related header file accordingly.  ("sed -i 's/-compile//g' vsx-vector-6-func-*.h"
recommended, similar patterns could be used for the two other comments below.)

> @@ -0,0 +1,22 @@
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-O2 -save-temps -mvsx" } */

Nit: We don't need "-save-temps" any more for all the test cases in this patch.

> +
> +/* This file just generates calls to the various builtins and verifies the
> +   expected number of instructions for each builtin were generated.  */
> +
> +#include "vsx-vector-6-func-1op.h"
> +
> +/* { dg-final { scan-assembler-times {\mxvabssp\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrspip\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrspim\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrspi\M} 1 } } */ 
> +/* { dg-final { scan-assembler-times {\mxvrspic\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrspiz\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvabsdp\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrdpip\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrdpim\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrdpi\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrdpic\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvrdpiz\M} 1 } } */
> +/* { dg-final { scan-assembler-times {\mxvsqrtdp\M} 1 } } */
> diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-run.c b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-run.c
> new file mode 100644
> index 00000000000..150e372e428
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-func-1op-run.c
> @@ -0,0 +1,98 @@
> +/* { dg-do run { target lp64 } } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */

We need vsx_hw for those *-run.c cases instead, as powerpc_vsx_ok
doesn't guarantee the test env can support vsx instructions, it just
ensures it can be compiled.

/* { dg-require-effective-target vsx_hw } */

All "*-run.c" cases need changes.

BR,
Kewen