Re: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) shortcut optimization

[Kito Cheng <kito.cheng@sifive.com>]

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

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 <pan2.li@intel.com> 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 <kito.cheng@gmail.com>
> Sent: Tuesday, April 25, 2023 9:08 PM
> To: Li, Pan2 <pan2.li@intel.com>; Jeff Law <jeffreyalaw@gmail.com>
> Cc: juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>;
> Kito.cheng <kito.cheng@sifive.com>; 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
>
> <gcc-patches@gcc.gnu.org> 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 <kito.cheng@gmail.com>
> > Sent: Friday, April 21, 2023 9:02 PM
> > To: Li, Pan2 <pan2.li@intel.com>
> > Cc: juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>;
> > Kito.cheng <kito.cheng@sifive.com>; Wang, Yanzhang
> > <yanzhang.wang@intel.com>
> > 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<mode>
> >
> > 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 <kito.cheng@gmail.com>
> > > Sent: Friday, April 21, 2023 6:17 PM
> > > To: Li, Pan2 <pan2.li@intel.com>
> > > Cc: juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>;
> > > Kito.cheng <kito.cheng@sifive.com>; Wang, Yanzhang
> > > <yanzhang.wang@intel.com>
> > > 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' <kito.cheng@gmail.com>; 'juzhe.zhong@rivai.ai'
> > > > <juzhe.zhong@rivai.ai>
> > > > Cc: 'gcc-patches' <gcc-patches@gcc.gnu.org>; 'Kito.cheng'
> > > > <kito.cheng@sifive.com>; Wang, Yanzhang <yanzhang.wang@intel.com>
> > > > 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 <kito.cheng@gmail.com>; juzhe.zhong@rivai.ai
> > > > Cc: gcc-patches <gcc-patches@gcc.gnu.org>; Kito.cheng
> > > > <kito.cheng@sifive.com>; Wang, Yanzhang <yanzhang.wang@intel.com>
> > > > 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 <kito.cheng@gmail.com>
> > > > Sent: Wednesday, April 19, 2023 5:46 PM
> > > > To: juzhe.zhong@rivai.ai
> > > > Cc: Li, Pan2 <pan2.li@intel.com>; gcc-patches
> > > > <gcc-patches@gcc.gnu.org>; Kito.cheng <kito.cheng@sifive.com>;
> > > > Wang, Yanzhang <yanzhang.wang@intel.com>
> > > > 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
>