Hi Bohan,
Thanks for tackling this. The patches looks mostly good but I'd like to
note a few more things.
First, git-check-commit says:
ERR: unchanged file mentioned in a ChangeLog (did you mean
"gcc/config/riscv/riscv-vector-builtins-bases.cc"?):
"gcc/config/riscv/riscv-vector-builtins-shapes.cc"
ERR: unchanged file mentioned in a ChangeLog (did you mean
"gcc/config/riscv/riscv-vector-builtins-bases.h"?):
"gcc/config/riscv/riscv-vector-builtins-shapes.h"
> +/* A list of args for vector_type func (vector_type, signed double
> demote type,
> + signed double demote type) function. */
> +static constexpr const rvv_arg_type_info signed_narrow_wwvv_args[]
> + = {rvv_arg_type_info (RVV_BASE_vector),
> + rvv_arg_type_info (RVV_BASE_double_trunc_signed_vector),
> + rvv_arg_type_info (RVV_BASE_double_trunc_signed_vector),
> + rvv_arg_type_info_end};
Nit: I think we use "narrow" only for real narrowing insns while here it
just refers to the dest/src relation. Therefore I'd rather skip it
because wwvv already implies that relation. Similar with signed, we can
just use "ss" to be consistent.
> (define_insn "@pred_widen_abd_plus<su><mode>"
> - [(set (match_operand:VWEXTI_HS 0 "register_operand"
> "+&vd,&vd,&vr,&vr")
> + [(set (match_operand:VWEXTI_HS 0 "register_operand" "=&vr")
> (if_then_else:VWEXTI_HS
> (unspec:<VM>
> - [(match_operand:<VM> 1 "vector_mask_operand" "vm,vm,Wc1,Wc1")
> - (match_operand 5 "vector_length_operand" "rK,rK,rK,rK")
> - (match_operand 6 "const_int_operand" "i,i,i,i")
> - (match_operand 7 "const_int_operand" "i,i,i,i")
> - (match_operand 8 "const_int_operand" "i,i,i,i")
> + [(match_operand:<VM> 1 "vector_mask_operand" "vmWc1")
> + (match_operand 5 "vector_length_operand" " rK")
> + (match_operand 6 "const_int_operand" " i")
> + (match_operand 7 "const_int_operand" " i")
> + (match_operand 8 "const_int_operand" " i")
> (reg:SI VL_REGNUM)
> (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
> (unspec:VWEXTI_HS
> - [(match_operand:<V_DOUBLE_TRUNC> 3 "register_operand" "vr,vr,vr,vr")
> - (match_operand:<V_DOUBLE_TRUNC> 4 "register_operand" "vr,vr,vr,vr")
> - (match_dup 0)]
> + [(match_operand:<V_DOUBLE_TRUNC> 3 "register_operand" " vr")
> + (match_operand:<V_DOUBLE_TRUNC> 4 "register_operand" " vr")
> + (match_operand:VWEXTI_HS 2 "register_operand" " 0")]
> UNSPEC_VABDA)
> - (match_operand:VWEXTI_HS 2 "vector_merge_operand" "vu,0,vu,0")))]
> + (match_dup 2)))]
Can you try using the new overlap constraints Pan Li is implementing
here? That way we could avoid the earlyclobber (but would need four
alternatives again). Then, let's use "vl" instead of "rK" even though
I'm not expecting theadvector to gain this insn :)
>
> -#if defined (__riscv_vector) || defined (__riscv_zvbb) \
> +#if defined (__riscv_vector) || defined (__riscv_zvabd) \
> + || defined (__riscv_zvbb) \
> || defined (__riscv_zve32f) || defined (__riscv_zve32x) \
> || defined (__riscv_zve64d) || defined (__riscv_zve64f) \
> || defined (__riscv_zve64x) \
> @@ -24,6 +25,7 @@
> || !defined (__riscv_intrinsic_zve64d) \
> || !defined (__riscv_intrinsic_zve64f) \
> || !defined (__riscv_intrinsic_zve64x) \
> + || !defined (__riscv_intrinsic_zvabd) \
> || !defined (__riscv_intrinsic_zvbb) \
> || !defined (__riscv_intrinsic_zvbc) \
> || !defined (__riscv_intrinsic_zvfbfmin) \
> @@ -60,6 +62,7 @@
> || __riscv_intrinsic_zve64d != 1 \
> || __riscv_intrinsic_zve64f != 1 \
> || __riscv_intrinsic_zve64x != 1 \
> + || __riscv_intrinsic_zvabd != 1 \
I'm getting a build error here, I guess we need to define it. Which
obviously leads to the question of how the patches were tested :)
> +vuint16m1_t
> +test_vwabdau_vv_u16m1 (vuint16m1_t vd, vuint8mf2_t vs2,
> + vuint8mf2_t vs1, size_t vl)
> +{
> + return __riscv_vwabdau_vv_u16m1 (vd, vs2, vs1, vl);
> +}
> +
> +/* { dg-final { scan-assembler-times {\tvabs\.v} 2 } } */
> +/* { dg-final { scan-assembler-times {\tvabd\.vv} 2 } } */
> +/* { dg-final { scan-assembler-times {\tvabdu\.vv} 1 } } */
> +/* { dg-final { scan-assembler-times {\tvwabda\.vv} 1 } } */
> +/* { dg-final { scan-assembler-times {\tvwabdau\.vv} 1 } } */
I think a few more tests would be useful, in particular some that test
the merge = dest cases.
--
Regards
Robin
