Re: Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization

["juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

Ok. Let's wait for Kito's more comments.
Thanks.


juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-05-24 05:07
To: 钟居哲; gcc-patches
CC: rdapp.gcc; kito.cheng; kito.cheng; palmer; palmer; Jeff Law; richard.sandiford
Subject: Re: [PATCH V2] RISC-V: Add RVV comparison autovectorization
>>> Don't you want to use your shiny new operand passing style here as
>>> with the other expanders?
> Hmmmm, I do this just following ARM code style.
> You can see I do pass rtx[] for expand_vcond and pass rtx,rtx,rtx for expand_vec_cmp.
> Well, I just follow ARM SVE implementation (You can check aarch64-sve.md, we are the same)  :)
> If don't like it, could give me more information then I change it for you.
 
It doesn't matter that much in the end.  I just wondered that we just introduced
a new style of passing operands to the insn_expander and then immediately not
use it in the first follow up :)
 
Nit:
+  e.set_policy (op_num == RVV_CMP_OP ? MASK_UNDISTURBED : MASK_ANY);
 
This looks weird in an emit__cmp_insn.  Without a comment it's unclear
why anything else but a CMP_OP would ever be used here.  The double meaning
of the enum (that I wanted to be an instruction type rather than a "number
of operands") doesn't help.  But well, fixable in the future.  We just
need to make sure not to accumulate too many of these warts.
 
From the expander side V3 looks clean now.  The integer parts look OK to me
but I haven't checked the FP side at all.
 
Regards
Robin