Ok, sounds good. Thank you!
Pan
-----Original Message-----
From: Kito Cheng <kito.ch...@sifive.com>
Sent: Friday, May 5, 2023 8:37 PM
To: Li, Pan2 <pan2...@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang
<yanzhang.w...@intel.com>
Subject: Re: [PATCH] RISC-V: Allow RVV VMS{Compare}(V1, V1) simplify to VMSET
I will take V1 and commit to trunk after my local test is done :)
On Fri, May 5, 2023 at 8:30 PM Li, Pan2 <pan2...@intel.com> wrote:
>
> Hi kito,
>
> Could you please help to share any suggestion about the PATCH? Comparing the
> V1 and V2.
>
> Pan
>
>
> -----Original Message-----
> From: Li, Pan2
> Sent: Wednesday, May 3, 2023 7:18 PM
> To: Jeff Law <jeffreya...@gmail.com>; Kito Cheng
> <kito.ch...@sifive.com>
> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; Wang, Yanzhang
> <yanzhang.w...@intel.com>; Andrew Waterman <and...@sifive.com>
> Subject: RE: [PATCH] RISC-V: Allow RVV VMS{Compare}(V1, V1) simplify
> to VMSET
>
> Thanks all for comments, will work with kito to make it happen.
>
> Pan
>
> -----Original Message-----
> From: Jeff Law <jeffreya...@gmail.com>
> Sent: Wednesday, May 3, 2023 12:28 AM
> To: Kito Cheng <kito.ch...@sifive.com>
> Cc: Li, Pan2 <pan2...@intel.com>; gcc-patches@gcc.gnu.org;
> juzhe.zh...@rivai.ai; Wang, Yanzhang <yanzhang.w...@intel.com>; Andrew
> Waterman <and...@sifive.com>
> Subject: Re: [PATCH] RISC-V: Allow RVV VMS{Compare}(V1, V1) simplify
> to VMSET
>
>
>
> On 4/29/23 19:40, Kito Cheng wrote:
> > Hi Jeff:
> >
> > The RTL pattern already models tail element and vector length well,
> > so I don't feel the first version of Pan's patch has any problem?
> >
> > Input RTL pattern:
> >
> > #(insn 10 7 12 2 (set (reg:VNx2BI 134 [ _1 ])
> > # (if_then_else:VNx2BI (unspec:VNx2BI [
> > # (const_vector:VNx2BI repeat [
> > # (const_int 1 [0x1])
> > # ]) # all-1 mask
> > # (reg:DI 143) # AVL reg, or vector length
> > # (const_int 2 [0x2]) # mask policy
> > # (const_int 0 [0]) # avl type
> > # (reg:SI 66 vl)
> > # (reg:SI 67 vtype)
> > # ] UNSPEC_VPREDICATE)
> > # (geu:VNx2BI (reg/v:VNx2QI 137 [ v1 ])
> > # (reg/v:VNx2QI 137 [ v1 ]))
> > # (unspec:VNx2BI [
> > # (reg:SI 0 zero)
> > # ] UNSPEC_VUNDEF))) # maskoff and tail operand
> > # (expr_list:REG_DEAD (reg:DI 143)
> > # (expr_list:REG_DEAD (reg/v:VNx2QI 137 [ v1 ])
> > # (nil))))
> >
> > And the split pattern, only did on tail/maskoff element with undefined
> > value:
> >
> > (define_split
> > [(set (match_operand:VB 0 "register_operand")
> > (if_then_else:VB
> > (unspec:VB
> > [(match_operand:VB 1 "vector_all_trues_mask_operand")
> > (match_operand 4 "vector_length_operand")
> > (match_operand 5 "const_int_operand")
> > (match_operand 6 "const_int_operand")
> > (reg:SI VL_REGNUM)
> > (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
> > (match_operand:VB 3 "vector_move_operand")
> > (match_operand:VB 2 "vector_undef_operand")))] # maskoff
> > and tail operand, only match undef value
> >
> > Then it turns into vmset, and also discard mask policy operand
> > (since maskoff is undef means don't care IMO):
> >
> > (insn 10 7 12 2 (set (reg:VNx2BI 134 [ _1 ])
> > (if_then_else:VNx2BI (unspec:VNx2BI [
> > (const_vector:VNx2BI repeat [
> > (const_int 1 [0x1])
> > ]) # all-1 mask
> > (reg:DI 143) # AVL reg, or vector length
> > (const_int 2 [0x2]) # mask policy
> > (reg:SI 66 vl)
> > (reg:SI 67 vtype)
> > ] UNSPEC_VPREDICATE)
> > (const_vector:VNx2BI repeat [
> > (const_int 1 [0x1])
> > ]) # all-1
> > (unspec:VNx2BI [
> > (reg:SI 0 zero)
> > ] UNSPEC_VUNDEF))) # still vundef
> > (expr_list:REG_DEAD (reg:DI 143)
> > (nil)))
> Right. My concern is that when we call relational_result it's going to
> return -1 (as a vector of bools) which bubbles up through the call
> chain. If that doesn't match the actual register state after the
> instruction (irrespective of the tail policy), then we have the potential to
> generate incorrect code.
>
> For example, if there's a subsequent instruction that tried to set a vector
> register to -1, it could just copy from the destination of the vmset to the
> new target. But if the vmset didn't set all the bits to 1, then the code is
> wrong.
>
> With all the UNSPECs in place, this may not be a problem in practice.
> Unsure. I'm willing to defer to you on this Kito.
>
> Jeff
