This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000] gcc mainline, add builtin support for vec_doublee, vec_doubleo, vec_doublel builtins
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: "Carl E. Love" <cel at us dot ibm dot com>
- Cc: gcc-patches at gcc dot gnu dot org, David Edelsohn <dje dot gcc at gmail dot com>, Bill Schmidt <wschmidt at linux dot vnet dot ibm dot com>
- Date: Wed, 17 May 2017 16:26:40 -0500
- Subject: Re: [PATCH, rs6000] gcc mainline, add builtin support for vec_doublee, vec_doubleo, vec_doublel builtins
- Authentication-results: sourceware.org; auth=none
- References: <1495034738.26317.2.camel@us.ibm.com>
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