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