Re: [PATCH, rs6000] (v2) Support for gimple folding of mergeh, mergel intrinsics

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hi!

On Tue, Jan 16, 2018 at 01:39:28PM -0600, Will Schmidt wrote:
> Sniff-tests of the target tests on a single system look OK.  Full regtests are
> currently running across assorted power systems.
> OK for trunk, pending successful results?

Just a few little things:

> 2018-01-16  Will Schmidt  <will_schmidt@vnet.ibm.com>
> 
> 	* config/rs6000/rs6000.c: (rs6000_gimple_builtin) Add gimple folding
> 	support for merge[hl].

The : goes after the ).

>  (define_insn "altivec_vmrghw_direct"
> -  [(set (match_operand:V4SI 0 "register_operand" "=v")
> -        (unspec:V4SI [(match_operand:V4SI 1 "register_operand" "v")
> -                      (match_operand:V4SI 2 "register_operand" "v")]
> -                     UNSPEC_VMRGH_DIRECT))]
> +  [(set (match_operand:V4SI 0 "register_operand" "=v,wa")
> +	(unspec:V4SI [(match_operand:V4SI 1 "register_operand" "v,wa")
> +		      (match_operand:V4SI 2 "register_operand" "v,wa")]
> +		     UNSPEC_VMRGH_DIRECT))]
>    "TARGET_ALTIVEC"
> -  "vmrghw %0,%1,%2"
> +  "@
> +  vmrghw %0,%1,%2
> +  xxmrghw %x0,%x1,%x2"

Those last two lines should be indented one more space, so that everything
aligns (with the @).

> +  "@
> +  vmrglw %0,%1,%2
> +  xxmrglw %x0,%x1,%x2"

Same here of course.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-1-be-folded.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile { target { powerpc-*-* } } } */

Do you want powerpc*-*-*?  That is default in gcc.target/powerpc; dg-do
compile is default, too, so you can either say

/* { dg-do compile } */

or nothing at all, to taste.

But it looks like you want to restrict to BE?  We still don't have a
dejagnu thingy for that; you could put some #ifdef around it all (there
are some examples in other testcases).  Not ideal, but works.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-float.c
> @@ -0,0 +1,26 @@
> +/* Verify that overloaded built-ins for vec_splat with float
> +   inputs produce the right code.  */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-maltivec -O2" } */

Either powerpc_altivec_ok or -mvsx?

> new file mode 100644
> index 0000000..ab5f54e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-longlong.c
> @@ -0,0 +1,48 @@
> +/* Verify that overloaded built-ins for vec_merge* with long long
> +   inputs produce the right code.  */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-mvsx -O2" } */

Either powerpc_vsx_ok or -mpower8-vector?

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/fold-vec-mergehl-pixel.c
> @@ -0,0 +1,24 @@
> +/* Verify that overloaded built-ins for vec_splat with pixel
> +   inputs produce the right code.  */
> +
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-maltivec -mvsx -O2" } */

-mvsx implies -maltivec (not wrong of course, just a bit weird).

Okay for trunk with those nits fixed.  Thanks!


Segher