RE: [PATCH] RISC-V: Allow RVV VMS{Compare}(V1, V1) simplify to VMSET

["Li, Pan2" <pan2.li@intel.com>]

Thanks all for comments. Summary what I have learned from the mail thread as below. Please feel free to correct me if any mistake.

1. The RVV VMSET has tail policy and the high bits of target register can be overridden to 1 or retain the value they held according to the ISA.
2. The semantics of tail policy is different with s390 according the macro comment " /* The truth element value for vector comparisons.  Our instructions always generate -1 in that case.  */ ".
3. We still have a lot of work to do for the RISC-V besides compiler.
4. The RTL pattern of PATCH v1 models tail policy and vector length as well.

Pan

-----Original Message-----
From: Kito Cheng <kito.cheng@sifive.com> 
Sent: Sunday, April 30, 2023 9:40 AM
To: Jeff Law <jeffreyalaw@gmail.com>
Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; Andrew Waterman <andrew@sifive.com>
Subject: Re: [PATCH] RISC-V: Allow RVV VMS{Compare}(V1, V1) simplify to VMSET

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)))



On Sat, Apr 29, 2023 at 11:05 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 4/28/23 20:55, Li, Pan2 wrote:
> > Thanks Jeff for comments.
> >
> > It makes sense to me. For the EQ operator we should have CONSTM1.
> That's not the way I interpret the RVV documentation.  Of course it's
> not terribly clear.    I guess one could do some experiments with qemu
> or try to dig into the sail code and figure out the intent from those.
>
>
>
> Does this mean s390 parts has similar issue here? Then for 
> instructions like VMSEQ, we need to adjust the simplify_rtx up to a point.
> You'd have to refer to the s390 instruction set reference to 
> understand precisely how the vector compares work.
>
> But as it stands this really isn't a simplify-rtx question, but a
> question of the semantics of risc-v.   What happens with the high bits
> in the destination mask register is critical -- and if risc-v doesn't 
> set them to all ones in this case, then that would mean that defining 
> that macro is simply wrong for risc-v.
>
> jeff