This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, rs6000] gcc mainline, add builtin support for vec_doublee, vec_doubleo, vec_doublel builtins


Hi Carl,

Bunch of things, I'm afraid.

On Wed, May 17, 2017 at 08:25:38AM -0700, Carl E. Love wrote:
>    * config/rs6000/rs6000-c: Add support for built-in functions
>    * config/rs6000/rs6000-builtin.def: Add definitions for
>    * config/rs6000/altivec.h: Add define for

Some parts are missing here.

> +(define_mode_iterator VM3 [V4SI V4SF])

This is VSX_W.  Is that not a useful name here?

> +(define_mode_attr     VM3_char [(V4SI "i")(V4SF "f")])

(There should be a space between ")(").
This is only used as "s<VM3_char>" -- that "s" should probably be part
of the attribute.  And that already exists: VEC_base.

> +(define_mode_attr     VM3_x [(V4SI "xw")(V4SF "p")])

This probably needs a better name...  not that I know one, mind :-)

> +     {
> +        /* Little endian word numbering for operand is 3 2 1 0.
> +           take (operand[1] operand[1]) and shift left one word
> +                 3 2 1 0    3 2 1 0  =>  2 1 0 3
> +           Input words 2 and 0 are now where they need to be for the
> +           conversion. */
> +        rtx rtx_tmp;
> +        rtx rtx_val = GEN_INT (4);
> +
> +        rtx_tmp = gen_reg_rtx (op_mode);
> +        emit_insn (gen_altivec_vsldoi_v4s<VM3_char> (rtx_tmp,
> +                  operands[1], operands[1], rtx_val));
> +        emit_insn (gen_vsx_xvcvs<VM3_x>dp (operands[0], rtx_tmp));
> +     }

I think you want xxsldwi instead?  That way, you can use all VSRs (for
the "xw" variants; "p" still needs VRs only).

> +;; Generate unsdoublee
> +;;    unsigned int to double convert words 0 and 2
> +(define_expand "unsdoubleev4si2"

Maybe you could use some iterator for the signed and unsigned version?
Many things seem to not do this, so maybe it is hard.

> +vector double vec_doublee (vector signed int);
> +vector double vec_doublee (vector unsigned int);
> +vector double vec_doublee (vector float);
> +
> +vector double vec_doubleh (vector signed int);
> +vector double vec_doubleh (vector unsigned int);
> +vector double vec_doubleh (vector float);
> +
> +vector double vec_doublel (vector signed int);
> +vector double vec_doublel (vector unsigned int);
> +vector double vec_doublel (vector float);
> +
> +vector double vec_doubleo (vector signed int);
> +vector double vec_doubleo (vector unsigned int);
> +vector double vec_doubleo (vector float);

It's more logical to group e/o and h/l together...  This list isn't
totally alphabetical in either case.  Fine like this as well, if you
don't agree.

> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-3-runnable.c
> @@ -0,0 +1,84 @@
> +/* { dg-do run { target { powerpc64*-*-* } } } */
> +/* { dg-require-effective-target powerpc_vsx_ok } */
> +/* { dg-options "-maltivec -mvsx" } */

Similar testcases have

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

powerpc64*-*-* does *not* mean it will only run for 64-bit targets:
it will only run if the compiler by default uses a 64-bit ABI.

I don't know if you need -linux.  If you don't use it you may well
have to exclude pretty much everything else.

I don't know if you need -maltivec.  I don't think you do though.

Runtime tests need vsx_hw; vsx_ok is enough for compile tests only.


Segher


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]