This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000] Add Power 9 support for vec_first builtins
- 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: Wed, 1 Nov 2017 16:31:21 -0500
- Subject: Re: [PATCH, rs6000] Add Power 9 support for vec_first builtins
- Authentication-results: sourceware.org; auth=none
- References: <1508455873.16660.9.camel@us.ibm.com> <20171023231432.GF4406@gate.crashing.org> <1509464769.9048.65.camel@us.ibm.com>
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_<mode>"
> + [(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_<mode>"
[(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<VSX_EXTRACT_WIDTH> (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>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_xor<mode>3 (zero, zero, zero));
I think we have a helper for this? One that avoids xor if it can.
Segher