Re: [PATCH v3, rs6000] Add vec_reve support

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

Hi Carl,

On Fri, Jun 23, 2017 at 02:59:05PM -0700, Carl Love wrote:
> +(define_expand "altivec_vreve<mode>2"
> +  [(set (match_operand:VEC_A 0 "register_operand" "=v")
> +	(unspec:VEC_A [(match_operand:VEC_A 1 "register_operand" "v")]
> +		      UNSPEC_VREVEV))]
> +  "TARGET_ALTIVEC"
> +{
> +  int i, j, size, num_elements;
> +  rtvec v = rtvec_alloc (16);
> +  rtx mask = gen_reg_rtx (V16QImode);
> +
> +  size = GET_MODE_UNIT_SIZE (<MODE>mode);
> +  num_elements = GET_MODE_NUNITS (<MODE>mode);
> +
> +  for (j = num_elements - 1; j >= 0; j--)

You're still running this loop backwards, is that on purpose?  If not,
please fix.

> +    for (i = 0; i < size; i++)
> +      RTVEC_ELT (v, i + j * size)
> +	=  gen_rtx_CONST_INT (QImode, i + (num_elements - 1 - j) * size);

Why not just GEN_INT?

> +/* { dg-do run { target { powerpc*-*-linux* } } } */
> +/* { dg-require-effective-target vsx_hw } */
> +/* { dg-options "-O2 -mvsx" } */

Does it actually use VSX?  The condition on the expander is just
TARGET_ALTIVEC.  Or won't the testcase not work without VSX for some
other reason?

The rest looks good.  Okay for trunk if you can take care of these final
few things.

Thanks!


Segher