Hi Robin,
>
> 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"
>
In fact the patch was first implemented on a internal branch and cherry picked
then... I'll clean that.
> > +/* 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.
I'll fix the names.
>
> > (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 :)
>
Okay, though I guess it's two alternatives? We don't support an undefined
merge operand in vwabda.
> >
> > -#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 :)
>
Oops. I manually tested the intrinsic cases but missed this one. I'll define
it in riscv_vector.h.
> > +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.
Okay. In particular I'll add the tail-undisturbed tests.
>
> --
> Regards
> Robin
Thanks,
Bohan
