On Wed, Jul 27, 2016 at 05:16:28PM -0400, Michael Meissner wrote:
>       * config/rs6000/vsx.md (UNSPEC_VSX_VSLO): New unspecs.
>       (UNSPEC_VSX_EXTRACT): Likewise.

"New unspec".

>       (VEC_EXTRACT_OPTIMIZE_P): New macro to say whether we can optmize
>       vec_extract on 64-bit ISA 2.07 systems and newer.

"optimize".

> --- gcc/config/rs6000/rs6000-protos.h (revision 238775)
> +++ gcc/config/rs6000/rs6000-protos.h (working copy)
> @@ -62,6 +62,7 @@ extern void rs6000_expand_vector_init (r
>  extern void paired_expand_vector_init (rtx, rtx);
>  extern void rs6000_expand_vector_set (rtx, rtx, int);
>  extern void rs6000_expand_vector_extract (rtx, rtx, rtx);
> +extern void rs6000_split_vec_extract_var (rtx, rtx, rtx, rtx, rtx);
>  extern bool altivec_expand_vec_perm_const (rtx op[4]);
>  extern void altivec_expand_vec_perm_le (rtx op[4]);
>  extern bool rs6000_expand_vec_perm_const (rtx op[4]);

This isn't in the changelog.

> +      /* For little endian, adjust element ordering.  For V2DI/V2DF, we can 
> use
> +      an XOR, otherwise we need to subtract.  The shift amount is so VSLO
> +      will shift the element into the upper position (adding 3 to convert a
> +      byte shift into a bit shift). */

Two spaces after dot.

> +;; Variable V2DI/V2DF extract
> +(define_insn_and_split "vsx_extract_<mode>_var"
> +  [(set (match_operand:<VS_scalar> 0 "gpc_reg_operand" "=v")
> +     (unspec:<VS_scalar> [(match_operand:VSX_D 1 "input_operand" "v")
> +                          (match_operand:DI 2 "gpc_reg_operand" "r")]
> +                         UNSPEC_VSX_EXTRACT))
> +   (clobber (match_scratch:DI 3 "=r"))
> +   (clobber (match_scratch:V2DI 4 "=&v"))]
> +  "VECTOR_MEM_VSX_P (<MODE>mode) && VEC_EXTRACT_OPTIMIZE_P"
> +  "#"
> +  "&& reload_completed"
> +  [(const_int 0)]
> +{
> +  rs6000_split_vec_extract_var (operands[0], operands[1], operands[2],
> +                             operands[3], operands[4]);
> +  DONE;
> +})

Why reload_completed?  Can it not run earlier?

> +/* Macro to say whether we can optimize vector extracts.  */
> +#define VEC_EXTRACT_OPTIMIZE_P       (TARGET_DIRECT_MOVE                     
> \
> +                              && TARGET_POWERPC64                    \
> +                              && TARGET_UPPER_REGS_DI)

I'm not a big fan of this name.  "Optimize" will quickly become dated,
everyone will take the current new hot thing for granted, and then when
you want to optimise even more (say, for ISA 6.0 or whatever) the name
is really out of place.

But I don't know a much better name either.

> --- gcc/testsuite/gcc.target/powerpc/vec-extract-1.c  (revision 0)
> +++ gcc/testsuite/gcc.target/powerpc/vec-extract-1.c  (revision 0)
> @@ -0,0 +1,27 @@
> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */

Maybe you can add a "run" test as well?

Looks good otherwise, okay for trunk with those nits fixed.

Thanks,


Segher

Reply via email to