On Wed, 10 May 2023 08:24:50 PDT (-0700), rdapp....@gmail.com wrote:
> Hi,
>
> this patch splits off the shift patterns of the binop patterns.
> This is necessary as the scalar shifts require a Pmode operand
> as shift count. To this end, a new iterator any_int_binop_no_shift
> is introduced. At a later point when the binops are split up
> further in commutative and non-commutative patterns (which both
> do not include the shift patterns) we might not need this anymore.
>
> Bootstrapped and regtested.
>
> Regards
> Robin
>
> --
>
> gcc/ChangeLog:
>
> * config/riscv/autovec.md (<optab><mode>3): Add scalar shift
> pattern.
> (v<optab><mode>3): Add vector shift pattern.
> * config/riscv/vector-iterators.md: New iterator.
> ---
> gcc/config/riscv/autovec.md | 40 +++++++++++++++++++++++++++-
> gcc/config/riscv/vector-iterators.md | 4 +++
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index 8347e42bb9c..2da4fc67d51 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -65,7 +65,7 @@ (define_expand "movmisalign<mode>"
>
> (define_expand "<optab><mode>3"
> [(set (match_operand:VI 0 "register_operand")
> - (any_int_binop:VI
> + (any_int_binop_no_shift:VI
> (match_operand:VI 1 "<binop_rhs1_predicate>")
> (match_operand:VI 2 "<binop_rhs2_predicate>")))]
> "TARGET_VECTOR"
> @@ -91,3 +91,41 @@ (define_expand "<optab><mode>3"
> NULL_RTX, <VM>mode);
> DONE;
> })
> +
> +;; =========================================================================
> +;; == Binary integer shifts by scalar.
> +;; =========================================================================
> +
> +(define_expand "<optab><mode>3"
> + [(set (match_operand:VI 0 "register_operand")
> + (any_shift:VI
> + (match_operand:VI 1 "register_operand")
> + (match_operand:<VEL> 2 "csr_operand")))]
I don't think VEL is _wrong_ here, as it's an integer type that's big
enough to hold the shift amount, but we might get some odd generated
code for the QI and HI flavors as we frequently don't handle the shorter
types well.
"csr_operand" does seem wrong, though, as that just accepts constants.
Maybe "arith_operand" is the way to go? I haven't looked at the
V immediates though.
> + "TARGET_VECTOR"
> +{
> + if (!CONST_SCALAR_INT_P (operands[2]))
> + operands[2] = gen_lowpart (Pmode, operands[2]);
> + riscv_vector::emit_len_binop (code_for_pred_scalar
> + (<BINOP_TO_UPPERCASE>, <MODE>mode),
> + operands[0], operands[1], operands[2],
> + NULL_RTX, <VM>mode, Pmode);
> + DONE;
> +})
> +
> +;; =========================================================================
> +;; == Binary integer shifts by vector.
> +;; =========================================================================
> +
> +(define_expand "v<optab><mode>3"
> + [(set (match_operand:VI 0 "register_operand")
> + (any_shift:VI
> + (match_operand:VI 1 "register_operand")
> + (match_operand:VI 2 "vector_shift_operand")))]
> + "TARGET_VECTOR"
> +{
> + riscv_vector::emit_len_binop (code_for_pred
> + (<BINOP_TO_UPPERCASE>, <MODE>mode),
> + operands[0], operands[1], operands[2],
> + NULL_RTX, <VM>mode);
> + DONE;
> +})
> diff --git a/gcc/config/riscv/vector-iterators.md
> b/gcc/config/riscv/vector-iterators.md
> index 42848627c8c..fdb0bfbe3b1 100644
> --- a/gcc/config/riscv/vector-iterators.md
> +++ b/gcc/config/riscv/vector-iterators.md
> @@ -1429,6 +1429,10 @@ (define_code_iterator any_commutative_binop [plus and
> ior xor
>
> (define_code_iterator any_non_commutative_binop [minus div udiv mod umod])
>
> +(define_code_iterator any_int_binop_no_shift
> + [plus minus and ior xor smax umax smin umin mult div udiv mod umod
> +])
> +
> (define_code_iterator any_immediate_binop [plus minus and ior xor])
>
> (define_code_iterator any_sat_int_binop [ss_plus ss_minus us_plus us_minus])
> --
> 2.40.0
It'd be great to have test cases for the patterns we're adding, at least
for some of the stickier ones.
