Hi Bohan,
Sorry for the long delay.
> This patch adds intrinsic and auto vectorization (vec_perm) support for
> xxtvarith, which is one of XuanTie's vendor vector extensions [1]. This
> extension include xt.vile (vector interleave even) and xt.vilo (vector
> interleave odd) instructions, that can be used for vector permutations
> like [0, 8, 2, 10, 4, 12, 6, 14]. When SEW is smaller than ELEN, a
> wider SEW may be used to generate code for permutations like [0, 1, 8,
> 9, 4, 5, 12, 13]. They can be useful for transpose operations in
> workloads like x264.
This looks generally reasonable to me even though I didn't go through
all the intrinsics plumbing. I trust you have done the necessary
testing for those.
> +Mask(XXTVARITH) Var(riscv_xxt_subext)
> +
> diff --git a/gcc/config/riscv/riscv-v.cc b/gcc/config/riscv/riscv-v.cc
> index d98f0a94023..83ab3be7ad0 100644
> --- a/gcc/config/riscv/riscv-v.cc
> +++ b/gcc/config/riscv/riscv-v.cc
> @@ -3650,6 +3650,85 @@ shuffle_consecutive_patterns (struct expand_vec_perm_d
> *d)
> return true;
> }
>
> +/* Recognize the patterns of th.vile and th.vilo. */
> +
> +static bool
> +shuffle_interleave_even_odd_patterns (struct expand_vec_perm_d *d)
> +{
> + machine_mode vmode = d->vmode;
> + poly_int64 vec_len = d->perm.length ();
> +
> + if (!TARGET_XXTVARITH)
> + return false;
> +
> + if (!vec_len.is_constant ())
> + return false;
Wouldn't even/odd work for VLA just as well? Could even say it's more
natural for them?
> +
> + bool even = false, elen_is_2x = false;
> + if (d->perm.series_p (0, 2, 0, 2) && d->perm.series_p (1, 2, vlen, 2))
> + {
> + even = true;
> + elen_is_2x = false;
> + }
> + else if (d->perm.series_p (0, 2, 1, 2)
> + && d->perm.series_p (1, 2, vlen + 1, 2))
> + {
> + even = false;
> + elen_is_2x = false;
> + }
> + else if (d->perm.series_p (0, 4, 0, 4) && d->perm.series_p (1, 4, 1, 4)
> + && d->perm.series_p (2, 4, vlen, 4)
> + && d->perm.series_p (3, 4, vlen + 1, 4))
> + {
> + even = true;
> + elen_is_2x = true;
> + }
> + else if (d->perm.series_p (0, 4, 2, 4) && d->perm.series_p (1, 4, 3, 4)
> + && d->perm.series_p (2, 4, vlen + 2, 4)
> + && d->perm.series_p (3, 4, vlen + 3, 4))
> + {
> + even = false;
> + elen_is_2x = true;
> + }
> + else
> + {
> + return false;
> + }
Nit: Redundant braces.
I like the "2x" trick here and wonder if we couldn't use it in the other
pattern recognizers as well. Also, IIRC the 4x pattern also occurs in
x264 codegen. Maybe it's possible to rewrite the perm somehow in a
separate re-usable function?
> + gcc_assert (TARGET_VECTOR_ELEN_64);
> + if (elen_is_2x && GET_MODE_BITSIZE (GET_MODE_INNER (vmode)) >= 64)
> + return false;
I think it would be better to check if the necessary (2x) vector mode
exists instead of asserting. We need the mode anyway.
> +(define_insn "@xt_pred_<xxtvarith_ins_name><mode>"
> + [(set (match_operand:VI_Q 0 "register_operand" "=&vr")
> + (if_then_else:VI_Q
> + (unspec:<VM>
> + [(match_operand 4 "vector_length_operand" " rK")
> + (match_operand 5 "const_int_operand" " i")
> + (match_operand 6 "const_int_operand" " i")
> + (reg:SI VL_REGNUM)
> + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
> + (unspec:VI_Q
> + [(match_operand:VI_Q 1 "register_operand" " 0")
> + (match_operand:VI_Q 2 "register_operand" "vr")
> + (match_operand:VI_Q 3 "register_operand" "vr")] UNSPEC_XT_VGMULXOR_IT)
> + (match_dup 1)))]
> + "TARGET_VECTOR && TARGET_XXTVARITH"
> + "xt.<xxtvarith_ins_name>.vv\t%0,%2,%3"
> + [(set_attr "type" "vcompress")
> + (set_attr "mode" "<MODE>")])
> +
> +(define_insn "@xt_pred_<xxtvarith_ins_name><mode>"
> + [(set (match_operand:V_VLS 0 "register_operand" "=vr, vr")
> + (if_then_else:V_VLS
> + (unspec:<VM>
> + [(match_operand 4 "vector_length_operand" " rK, rK")
> + (match_operand 5 "const_int_operand" " i, i")
> + (match_operand 6 "const_int_operand" " i, i")
> + (reg:SI VL_REGNUM)
> + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
> + (unspec:V_VLS
> + [(match_operand:V_VLS 2 "register_operand" "vr,vr")
> + (match_operand:V_VLS 3 "register_operand" "vr,vr")] UNSPEC_XT_VIL_IT)
> + (match_operand:V_VLS 1 "vector_merge_operand" "vu,0")))]
> + "TARGET_VECTOR && TARGET_XXTVARITH"
> + "xt.<xxtvarith_ins_name>.vv\t%0,%2,%3"
> + [(set_attr "type" "vcompress")
> + (set_attr "mode" "<MODE>")
> + (set_attr "merge_op_idx" "1")
> + (set_attr "vl_op_idx" "4")
> + (set (attr "ta") (symbol_ref "riscv_vector::get_ta(operands[5])"))
> + (set (attr "avl_type_idx") (const_int 6))])
> +
> +(define_insn "@xt_pred_<xxtvarith_ins_name><mode>"
> + [(set (match_operand:VI_D 0 "register_operand" "=vr")
> + (if_then_else:VI_D
> + (unspec:<VM>
> + [(match_operand 4 "vector_length_operand" "rK")
> + (match_operand 5 "const_int_operand" " i")
> + (match_operand 6 "const_int_operand" " i")
> + (reg:SI VL_REGNUM)
> + (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
> + (unspec:VI_D
> + [(match_operand:VI_D 1 "register_operand" " 0")
> + (match_operand:VI_D 2 "register_operand" "vr")
> + (match_operand:VI_D 3 "register_operand" "vr")]
> + UNSPEC_XT_VCRCFOLD_IT)
> + (match_dup 1)))]
> + "TARGET_VECTOR && TARGET_XXTVARITH"
> + "xt.<xxtvarith_ins_name>.vv\t%0,%2,%3"
> + [(set_attr "type" "vcompress")
> + (set_attr "mode" "<MODE>")])
Any reason why the first insn has an earlyclobber but the others don't?
Is there overlap? Also, why vcompress rather than something else? I
guess you just didn't want to introduce a new type? All this type
matching should go away anyway so it might not be too bad but it's a bit
misleading. Not a blocker I'd say.
> --- /dev/null
> +++
> b/gcc/testsuite/gcc.target/riscv/rvv/autovec/vls-vlmax/shuffle-interleave-evenodd-run.c
> @@ -0,0 +1,122 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target riscv_v_ok } */
Why not rv64 here? Or maybe, why rv64 in the other test? :)
Also, why no separate xth run tests?
> +/* { dg-add-options riscv_v } */
> +/* { dg-additional-options "-O3 -mrvv-max-lmul=m8 -std=gnu99" } */
Are these supposed to be baseline tests? I'm not against that but would
have expected them to execute the new functionality.
--
Regards
Robin
