This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000] Add builtin support for vec_insert4b, vec_extract4b
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: Carl 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: Tue, 6 Feb 2018 09:47:34 -0600
- Subject: Re: [PATCH, rs6000] Add builtin support for vec_insert4b, vec_extract4b
- Authentication-results: sourceware.org; auth=none
- References: <1517513515.3596.25.camel@us.ibm.com>
Hi Carl,
On Thu, Feb 01, 2018 at 11:31:55AM -0800, Carl Love wrote:
> The following patch contains fixes for the builtins to add and extract
> a word from a char vector. The existing names for the builtins that do
> this are not consistent with the ABI. Additionally, the supported
> arguments are not all consistent with the ABI. This patch fixes the
> support for the insert and extract word builtins so they are consistent
> with the "64-Bit ELF V2 ABI Specification".
The patch is hard to review because it does multiple things at once.
Would have been easier if you first add the new builtins and then delete
the old one. But I'll try :-)
> Let me know if the patch looks OK or not. Let me know if you want to
> include it in stage 4 or wait for the next release. Thanks.
It should also go to GCC 7, right?
> 2018-01-31 Carl Love <cel@us.ibm.com>
>
> * config/rs6000/altivec.h: Change vec_vextract4b to vec_extract4b
You have a tab before vec_extract4b there.
> and vec_vinsert4b to vec_insert4b.
> * config/rs6000/rs6000-builtin.def: Remove VEXTRACT4B, VINSERT4B
> and VINSERT4B_DI definitions. Add INSERT4B and EXTRACT4B definitions.
Two spaces after dot (numerous times, this is the first one). You have
a tab before EXTRACT4B.
* config/rs6000/rs6000-builtin.def (VEXTRACT4B, VINSERT4B): Delete.
(EXTRACT4B, INSERT4B): New.
(And similar to all below).
> *doc/extend.texi: Remove documentation for the vec_vextract4b. Fix
> documentation for vec_insert4b. Add documentation for vec_extract4b.
Space after *.
> 2018-01-31 Carl Love <cel@us.ibm.com>
> * gcc.target/powerpc/builtins-7-p9-runnable.c: New runnable test file.
> * gcc.target/powerpc/p9_vinsert4b-1.c: Remove file.
> * gcc.target/powerpc/p9_vinsert4b-2.c: Remove file.
The files are called p9-vinsert* in fact. Is all what they tested now in
builtins-7-p9-runnable.c ?
> +(define_insn "extract4b"
> + [(set (match_operand:V2DI 0 "vsx_register_operand")
> + (unspec:V2DI [(match_operand:V16QI 1 "vsx_register_operand" "wa")
> + (match_operand:QI 2 "const_0_to_12_operand" "n")]
> + UNSPEC_XXEXTRACTUW))]
> "TARGET_P9_VECTOR"
> {
> if (!VECTOR_ELT_ORDER_BIG)
> operands[2] = GEN_INT (12 - INTVAL (operands[2]));
>
> + return "xxextractuw %0,%1,%2";
> +})
You need %x0 and %x1 here I think?
> -vector signed char vec_insert4b (vector int, vector signed char, const int);
> +vector unsigned char vec_insert4b (vector int, vector unsigned char,
> + const signed int);
Maybe just write "int" instead of "signed int"?
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/builtins-7-p9-runnable.c
> @@ -0,0 +1,168 @@
> +/* { dg-do run { target { powerpc64*-*-* && p9vector_hw } } } */
As always: why powerpc64* instead of powerpc*?
Segher