I would strongly prefer 2 since I believe this won't be the last optimization we did for this kind of thing, and I don't want to see we need to fix or worry about vsetvli stuff every time if possible. And the current pattern design is more reasonable to me - only defining those fields is really useful. On Tue, Apr 25, 2023 at 9:51 PM Li, Pan2 wrote: > Thanks Kito. > > Actually I fixed the below ICE with all riscv tests passed, but hold the > PATCH v3 as may conflict with one of Juzhe's PATCH. > > Thus, there will be 2 options for the shortcut optimization. > > 1. Adjust existing define and let the underlying pass to perform the > optimization. > 2. Add new define_split(s) for each of the shortcut optimization. > > Personally I may prefer the option 1. But here we would like the figure > out the one and the only one right way for the implementation. Thus, it is > OK if we think option 2 is a better way for this. > > Kito and Juzhe, any idea for making the decision? Thanks in advance! > > Pan > > -----Original Message----- > From: Kito Cheng > Sent: Tuesday, April 25, 2023 9:08 PM > To: Li, Pan2 ; Jeff Law > Cc: juzhe.zhong@rivai.ai; gcc-patches ; > Kito.cheng ; Wang, Yanzhang < > yanzhang.wang@intel.com> > Subject: Re: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) shortcut > optimization > > Second thought on this, we should just add define_split rather than > define_insn_and_split, otherwise we might hit the same issue again, and I > expect the split pattern will only used in combine pass. > > On Sat, Apr 22, 2023 at 1:34 PM Li, Pan2 via Gcc-patches > > wrote: > > > > Hi Kito > > > > Thanks for the suggestion. Sorry for late response due to stuck in the > rest rvv test files auto generation. > > > > I have similar discuss with juzhe for this approach, and take Patch v2's > way due to the below concern. > > > > 1. The vector.md Is quite complicated already, the maintenance may be > out of control if we will add many new define_insn_and_split for the > shortcut. > > 2. The new added pattern may not friendly for the underlying > auto-vectorization. > > > > Juzhe can help to correct me if any misleading. > > > > Pan > > > > -----Original Message----- > > From: Kito Cheng > > Sent: Friday, April 21, 2023 9:02 PM > > To: Li, Pan2 > > Cc: juzhe.zhong@rivai.ai; gcc-patches ; > > Kito.cheng ; Wang, Yanzhang > > > > Subject: Re: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) shortcut > > optimization > > > > Hi Pan: > > > > One idea come to my mind, maybe we should add a new > > define_insn_and_split pattern instead of change @pred_mov > > > > On Fri, Apr 21, 2023 at 7:17 PM Li, Pan2 via Gcc-patches < > gcc-patches@gcc.gnu.org> wrote: > > > > > > Thanks kito, will try to reproduce this issue and keep you posted. > > > > > > Pan > > > > > > -----Original Message----- > > > From: Kito Cheng > > > Sent: Friday, April 21, 2023 6:17 PM > > > To: Li, Pan2 > > > Cc: juzhe.zhong@rivai.ai; gcc-patches ; > > > Kito.cheng ; Wang, Yanzhang > > > > > > Subject: Re: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) > > > shortcut optimization > > > > > > I got a bunch of new fails including ICE for gcc testsuite, and some > cases are hanging there, could you take a look? > > > > > > $ riscv64-unknown-linux-gnu-gcc > > > gcc.target/riscv/rvv/vsetvl/avl_single-92.c -O2 -march=rv32gcv > > > -mabi=ilp32 > > > during RTL pass: expand > > > > /scratch1/kitoc/riscv-gnu-workspace/riscv-gnu-toolchain-trunk/gcc/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/avl_single-92.c: > > > In function 'f': > > > > /scratch1/kitoc/riscv-gnu-workspace/riscv-gnu-toolchain-trunk/gcc/gcc/testsuite/gcc.target/riscv/rvv/vsetvl/avl_single-92.c:8:13: > > > internal compiler error: in maybe_gen_insn, at optabs.cc:8102 > > > 8 | vbool64_t mask = *(vbool64_t*) (in + 1000000); > > > | ^~~~ > > > 0x130d278 maybe_gen_insn(insn_code, unsigned int, expand_operand*) > > > ../../../../riscv-gnu-toolchain-trunk/gcc/gcc/optabs.cc:8102 > > > > > > > > > On Fri, Apr 21, 2023 at 5:47 PM Li, Pan2 via Gcc-patches < > gcc-patches@gcc.gnu.org> wrote: > > > > > > > > Kindly ping for the PATCH v2. Just FYI there will be some underlying > investigation based on this PATCH like VMSEQ. > > > > > > > > Pan > > > > > > > > -----Original Message----- > > > > From: Li, Pan2 > > > > Sent: Wednesday, April 19, 2023 7:27 PM > > > > To: 'Kito Cheng' ; 'juzhe.zhong@rivai.ai' > > > > > > > > Cc: 'gcc-patches' ; 'Kito.cheng' > > > > ; Wang, Yanzhang > > > > Subject: RE: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) > > > > shortcut optimization > > > > > > > > Update the Patch v2 for more detail information for clarification. > Please help to review continuously. > > > > > > > > https://gcc.gnu.org/pipermail/gcc-patches/2023-April/616175.html > > > > > > > > Pan > > > > > > > > -----Original Message----- > > > > From: Li, Pan2 > > > > Sent: Wednesday, April 19, 2023 6:33 PM > > > > To: Kito Cheng ; juzhe.zhong@rivai.ai > > > > Cc: gcc-patches ; Kito.cheng > > > > ; Wang, Yanzhang > > > > Subject: RE: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) > > > > shortcut optimization > > > > > > > > Sure thing. > > > > > > > > For Changlog, I consider it was generated automatically in previous. > LOL. > > > > > > > > Pan > > > > > > > > -----Original Message----- > > > > From: Kito Cheng > > > > Sent: Wednesday, April 19, 2023 5:46 PM > > > > To: juzhe.zhong@rivai.ai > > > > Cc: Li, Pan2 ; gcc-patches > > > > ; Kito.cheng ; > > > > Wang, Yanzhang > > > > Subject: Re: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) > > > > shortcut optimization > > > > > > > > HI JuZhe: > > > > > > > > Thanks for explaining! > > > > > > > > > > > > Hi Pan: > > > > > > > > I think that would be helpful if JuZhe's explaining that could be > written into the commit log. > > > > > > > > > > > > > gcc/ChangeLog: > > > > > > > > > > * config/riscv/riscv-v.cc (emit_pred_op): > > > > > * config/riscv/riscv-vector-builtins-bases.cc: > > > > > * config/riscv/vector.md: > > > > > > > > And don't forgot write some thing in ChangeLog...:P >