[PATCH, rs6000] Add Power 9 support for vec_first builtins
GCC maintainers: The following patch adds support for the vec_first_match_index, vec_first_match_or_eos_index, vec_first_mismatch_index, and vec_first_mismatch_or_eos_index builtins for ISA 3.0. The patch has been tested on: powerpc64le-unknown-linux-gnu (Power 8 LE) powerpc64le-unknown-linux-gnu (Power 9 LE) without regressions. Please let me know if the following patch is acceptable. Thanks. Carl Love gcc/ChangeLog: 2017-10-19 Carl Love * config/rs6000/rs6000-c.c: Add support for builtins: unsigned int vec_first_match_index (vector signed char, vector signed char); unsigned int vec_first_match_index (vector unsigned char, vector unsigned char); unsigned int vec_first_match_index (vector signed int, vector signed int); unsigned int vec_first_match_index (vector unsigned int, vector unsigned int); unsigned int vec_first_match_index (vector signed short, vector signed short); unsigned int vec_first_match_index (vector unsigned short, vector unsigned short); unsigned int vec_first_match_or_eos_index (vector signed char, vector signed char); unsigned int vec_first_match_or_eos_index (vector unsigned char, vector unsigned char); unsigned int vec_first_match_or_eos_index (vector signed int, vector signed int); unsigned int vec_first_match_or_eos_index (vector unsigned int, vector unsigned int); unsigned int vec_first_match_or_eos_index (vector signed short, vector signed short); unsigned int vec_first_match_or_eos_index (vector unsigned short, vector unsigned short); unsigned int vec_first_mismatch_index (vector signed char, vector signed char); unsigned int vec_first_mismatch_index (vector unsigned char, vector unsigned char); unsigned int vec_first_mismatch_index (vector signed int, vector signed int); unsigned int vec_first_mismatch_index (vector unsigned int, vector unsigned int); unsigned int vec_first_mismatch_index (vector signed short, vector signed short); unsigned int vec_first_mismatch_index (vector unsigned short, vector unsigned short); unsigned int vec_first_mismatch_or_eos_index (vector signed char, vector signed char); unsigned int vec_first_mismatch_or_eos_index (vector unsigned char, vector unsigned char); unsigned int vec_first_mismatch_or_eos_index (vector signed int, vector signed int); unsigned int vec_first_mismatch_or_eos_index (vector unsigned int, vector unsigned int); unsigned int vec_first_mismatch_or_eos_index (vector signed short, vector signed short); unsigned int vec_first_mismatch_or_eos_index (vector unsigned short, vector unsigned short); * config/rs6000/rs6000-builtin.def (VFIRSTMATCHINDEX, VFIRSTMATCHOREOSINDEX, VFIRSTMISMATCHINDEX, VFIRSTMISMATCHOREOSINDEX): Add BU_P9V_AV_2 expansions for the builtins. * config/rs6000/altivec.h (vec_first_match_index, vec_first_mismatch_index, vec_first_match_or_eos_index, vec_first_mismatch_or_eos_index): Add #defines for the builtins. * config/rs6000/rs6000-protos.h (bytes_in_mode): Add extern declaration. * config/rs6000/rs6000.c (bytes_in_mode): Add function to return mode size in bytes. * config/rs6000/vsx.md: (first_match_index_, first_match_or_eos_index_, first_mismatch_index_, first_mismatch_or_eos_index_): Add define expand to implement the builtins. (vctzlsbb_): Add mode field to define_insn for vctzlsbb. * doc/extend.texi: Update the built-in documenation file for the new built-in functions. gcc/testsuite/ChangeLog: 2017-10-19 Carl Love
Re: [PATCH, rs6000] Add Power 9 support for vec_first builtins
Hi Carl, On Thu, Oct 19, 2017 at 04:31:13PM -0700, Carl Love wrote: > * config/rs6000/rs6000-builtin.def (VFIRSTMATCHINDEX, > VFIRSTMATCHOREOSINDEX, VFIRSTMISMATCHINDEX, VFIRSTMISMATCHOREOSINDEX): > Add BU_P9V_AV_2 expansions for the builtins. Those names are a bit unwieldy ;-) > +BU_P9V_OVERLOAD_2 (VFIRSTMISMATCHOREOSINDEX, 'first_mismatch_or_eos_index") How did this compile? Segher
Re: [PATCH, rs6000] Add Power 9 support for vec_first builtins
GCC maintainers: The patch has been updated to fix the typo and retested. Unfortunately the names of the builtins are rather long and unwieldy to begin with. Please let me know if the following patch is acceptable. Thanks. Carl Love - gcc/ChangeLog: 2017-10-31 Carl Love * config/rs6000/rs6000-c.c: Add support for builtins: unsigned int vec_first_match_index (vector signed char, vector signed char); unsigned int vec_first_match_index (vector unsigned char, vector unsigned char); unsigned int vec_first_match_index (vector signed int, vector signed int); unsigned int vec_first_match_index (vector unsigned int, vector unsigned int); unsigned int vec_first_match_index (vector signed short, vector signed short); unsigned int vec_first_match_index (vector unsigned short, vector unsigned short); unsigned int vec_first_match_or_eos_index (vector signed char, vector signed char); unsigned int vec_first_match_or_eos_index (vector unsigned char, vector unsigned char); unsigned int vec_first_match_or_eos_index (vector signed int, vector signed int); unsigned int vec_first_match_or_eos_index (vector unsigned int, vector unsigned int); unsigned int vec_first_match_or_eos_index (vector signed short, vector signed short); unsigned int vec_first_match_or_eos_index (vector unsigned short, vector unsigned short); unsigned int vec_first_mismatch_index (vector signed char, vector signed char); unsigned int vec_first_mismatch_index (vector unsigned char, vector unsigned char); unsigned int vec_first_mismatch_index (vector signed int, vector signed int); unsigned int vec_first_mismatch_index (vector unsigned int, vector unsigned int); unsigned int vec_first_mismatch_index (vector signed short, vector signed short); unsigned int vec_first_mismatch_index (vector unsigned short, vector unsigned short); unsigned int vec_first_mismatch_or_eos_index (vector signed char, vector signed char); unsigned int vec_first_mismatch_or_eos_index (vector unsigned char, vector unsigned char); unsigned int vec_first_mismatch_or_eos_index (vector signed int, vector signed int); unsigned int vec_first_mismatch_or_eos_index (vector unsigned int, vector unsigned int); unsigned int vec_first_mismatch_or_eos_index (vector signed short, vector signed short); unsigned int vec_first_mismatch_or_eos_index (vector unsigned short, vector unsigned short); * config/rs6000/rs6000-builtin.def (VFIRSTMATCHINDEX, VFIRSTMATCHOREOSINDEX, VFIRSTMISMATCHINDEX, VFIRSTMISMATCHOREOSINDEX): Add BU_P9V_AV_2 expansions for the builtins. * config/rs6000/altivec.h (vec_first_match_index, vec_first_mismatch_index, vec_first_match_or_eos_index, vec_first_mismatch_or_eos_index): Add #defines for the builtins. * config/rs6000/rs6000-protos.h (bytes_in_mode): Add extern declaration. * config/rs6000/rs6000.c (bytes_in_mode): Add function to return mode size in bytes. * config/rs6000/vsx.md: (first_match_index_, first_match_or_eos_index_, first_mismatch_index_, first_mismatch_or_eos_index_): Add define expand to implement the builtins. (vctzlsbb_): Add mode field to define_insn for vctzlsbb. * doc/extend.texi: Update the built-in documenation file for the new built-in functions. gcc/testsuite/ChangeLog: 2017-10-31 Carl Love * gcc.target/powerpc/builtins-6-p9-runnable.c: Add runnable test for the new builtins. --- gcc/config/rs6000/altivec.h|4 +
Re: [PATCH, rs6000] Add Power 9 support for vec_first builtins
On Tue, Oct 31, 2017 at 08:46:09AM -0700, Carl Love wrote:
> 2017-10-31 Carl Love
>
> * config/rs6000/rs6000-c.c: Add support for builtins:
> unsigned int vec_first_match_index (vector signed char,
> vector signed char);
... The ChangeLog IMHO is not. It should be something like:
* config/rs6000/rs6000-c.c (altivec_overloaded_builtins): Add support
for vec_first_{,mis}match_{,or_eos_}index builtins with
vector {,un}signed {char,short,int} arguments.
Note (altivec_overloaded_builtins) to say what you've changed and
then just a sentence (in which IMHO wildcards are fine, but function
arguments in a way you wrote are not, because it is confusing with
what ()s mean in ChangeLog entry.
> * config/rs6000/rs6000-builtin.def (VFIRSTMATCHINDEX,
> VFIRSTMATCHOREOSINDEX, VFIRSTMISMATCHINDEX, VFIRSTMISMATCHOREOSINDEX):
> Add BU_P9V_AV_2 expansions for the builtins.
> * config/rs6000/altivec.h (vec_first_match_index,
> vec_first_mismatch_index, vec_first_match_or_eos_index,
> vec_first_mismatch_or_eos_index): Add #defines for the builtins.
> * config/rs6000/rs6000-protos.h (bytes_in_mode): Add extern
> declaration.
New declaration. ?
> * config/rs6000/rs6000.c (bytes_in_mode): Add function to return mode
> size in bytes.
New function. ? You don't need to explain what the function is for in
ChangeLog.
> * config/rs6000/vsx.md: (first_match_index_,
No : between filename and (what changed), please.
> first_match_or_eos_index_, first_mismatch_index_,
> first_mismatch_or_eos_index_): Add define expand to implement
> the builtins.
> (vctzlsbb_): Add mode field to define_insn for vctzlsbb.
> * doc/extend.texi: Update the built-in documenation file for the new
> built-in functions.
>
> gcc/testsuite/ChangeLog:
>
> 2017-10-31 Carl Love
>
> * gcc.target/powerpc/builtins-6-p9-runnable.c: Add runnable test for
> the new builtins.
New test.
Again, don't explain what the test is for.
For the rest I'll defer to PowerPC maintainers.
Jakub
Re: [PATCH, rs6000] Add Power 9 support for vec_first builtins
Hi Carl,
On Tue, Oct 31, 2017 at 08:46:09AM -0700, Carl Love wrote:
> gcc/ChangeLog:
See Jakub's comments (thanks!)
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -14286,6 +14286,34 @@ swap_selector_for_mode (machine_mode mode)
>return force_reg (V16QImode, gen_rtx_CONST_VECTOR (V16QImode, gen_rtvec_v
> (16, perm)));
> }
>
> +/* Return bytes in type */
Dot space space.
> +int
> +bytes_in_mode (machine_mode mode)
> +{
But what do you need this for? This is GET_MODE_SIZE (GET_MODE_INNER (mode))
(or similar).
> +;; Return first position of match between vectors
> +(define_expand "first_match_index_"
> + [(match_operand:SI 0 "register_operand")
> + (unspec: SI [(match_operand:VSX_EXTRACT_I 1 "register_operand")
No space in :SI please.
> + (match_operand:VSX_EXTRACT_I 2 "register_operand")]
This should indent to the same level as the previous match_operand.
> + UNSPEC_VSX_FIRST_MATCH_INDEX)]
(define_expand "first_match_index_"
[(match_operand:SI 0 "register_operand")
(unspec:SI [(match_operand:VSX_EXTRACT_I 1 "register_operand")
(match_operand:VSX_EXTRACT_I 2 "register_operand")]
UNSPEC_VSX_FIRST_MATCH_INDEX)]
> + emit_insn (gen_vcmpnez (cmp_result, operands[1],
> +operands[2]));
Indent with tabs for each each spaces; the "o" should align with the "c".
> + sh = bytes_in_mode(mode)/2;
Spaces around binary operators please, and before the opening paren of
parameter lists.
> + /* Vector with zeros in elements that correspond to zeros in operands. */
> + emit_insn (gen_xor3 (zero, zero, zero));
I think we have a helper for this? One that avoids xor if it can.
Segher
