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

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

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

I see. If there is no question from Juzhe, I will try to implement it follow the option 2.

Pan

From: Kito Cheng <kito.cheng@sifive.com>
Sent: Tuesday, April 25, 2023 9:57 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: Kito Cheng <kito.cheng@gmail.com>; Jeff Law <jeffreyalaw@gmail.com>; juzhe.zhong@rivai.ai; gcc-patches <gcc-patches@gcc.gnu.org>; Wang, Yanzhang <yanzhang.wang@intel.com>
Subject: Re: Re: [PATCH] RISC-V: Allow VMS{Compare} (V1, V1) shortcut optimization

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