pushed v1 to trunk
On Fri, May 5, 2023 at 8:46 PM Li, Pan2 via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> 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
