diff mbox series

[V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]

Message ID 20230914080321.3234794-1-juzhe.zhong@rivai.ai
State New
Headers show
Series [V3] RISC-V: Expand VLS mode to scalar mode move[PR111391] | expand

Commit Message

钟居哲 Sept. 14, 2023, 8:03 a.m. UTC 
This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391

I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
This FAIL is because of the latent BUG of VSETVL PASS.

So this patch includes VSETVL PASS fix even though it's not related to the PR111391.

I have confirm the whole regression no additional FAILs are introduced.

	PR target/111391

gcc/ChangeLog:

	* config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
	(vec_extract<mode><vel>): Ditto.
	* config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
	(pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
	* config/riscv/riscv.cc (riscv_legitimize_move): Expand move.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
	* gcc.target/riscv/rvv/autovec/pr111391.c: New test.

---
 gcc/config/riscv/autovec.md                   |  2 +-
 gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
 gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
 .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
 .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
 5 files changed, 64 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c

Comments

Kito Cheng Sept. 14, 2023, 8:11 a.m. UTC | #1 
On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
>
> This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
>
> I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> This FAIL is because of the latent BUG of VSETVL PASS.
>
> So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
>
> I have confirm the whole regression no additional FAILs are introduced.
>
>         PR target/111391
>
> gcc/ChangeLog:
>
>         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
>         (vec_extract<mode><vel>): Ditto.
>         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
>         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
>         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
>         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
>
> ---
>  gcc/config/riscv/autovec.md                   |  2 +-
>  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
>  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
>  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
>  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
>  5 files changed, 64 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index e74a1695709..7121bab1716 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -1442,7 +1442,7 @@
>  ;; -------------------------------------------------------------------------
>  ;; ---- [INT,FP] Extract a vector element.
>  ;; -------------------------------------------------------------------------
> -(define_expand "@vec_extract<mode><vel>"
> +(define_expand "vec_extract<mode><vel>"

Why remove this? I saw this change was introduced in v3?


>    [(set (match_operand:<VEL>     0 "register_operand")
>       (vec_select:<VEL>
>         (match_operand:V_VLS      1 "register_operand")
钟居哲 Sept. 14, 2023, 8:15 a.m. UTC | #2 
>> Why remove this? I saw this change was introduced in v3?

The "@" was introduced by this patch: https://gcc.gnu.org/pipermail/gcc-patches/2023-September/630184.html 
At the first time, I thought I need to explicitly call emit_insn (gen_vec_extract (mode, mode, ....)
That's why I added in the last patch.

However, I found I don't need to call gen_vec_extract, so I remove "@" in this patch:
+      enum insn_code icode
+	= convert_optab_handler (vec_extract_optab, vmode, mode);
+      gcc_assert (icode != CODE_FOR_nothing);
+      class expand_operand ops[3];
+      create_output_operand (&ops[0], dest, mode);
+      ops[0].target = 1;
+      create_input_operand (&ops[1], gen_lowpart (vmode, SUBREG_REG (src)),
+			    vmode);
+      unsigned int index = SUBREG_BYTE (src).to_constant () / mode_size;
+      create_integer_operand (&ops[2], index);
+      expand_insn (icode, 3, ops);
This code is copied from optabs-query.cc



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-09-14 16:11
To: Juzhe-Zhong
CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
Subject: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
>
> This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
>
> I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> This FAIL is because of the latent BUG of VSETVL PASS.
>
> So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
>
> I have confirm the whole regression no additional FAILs are introduced.
>
>         PR target/111391
>
> gcc/ChangeLog:
>
>         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
>         (vec_extract<mode><vel>): Ditto.
>         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
>         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
>         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
>         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
>
> ---
>  gcc/config/riscv/autovec.md                   |  2 +-
>  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
>  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
>  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
>  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
>  5 files changed, 64 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index e74a1695709..7121bab1716 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -1442,7 +1442,7 @@
>  ;; -------------------------------------------------------------------------
>  ;; ---- [INT,FP] Extract a vector element.
>  ;; -------------------------------------------------------------------------
> -(define_expand "@vec_extract<mode><vel>"
> +(define_expand "vec_extract<mode><vel>"
 
Why remove this? I saw this change was introduced in v3?
 
 
>    [(set (match_operand:<VEL>     0 "register_operand")
>       (vec_select:<VEL>
>         (match_operand:V_VLS      1 "register_operand")
钟居哲 Sept. 14, 2023, 9:18 a.m. UTC | #3 
Is it Ok for trunk ? Or you want me send a separate patch to remove "@" in vec_extract optab ?



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-09-14 16:11
To: Juzhe-Zhong
CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
Subject: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
>
> This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
>
> I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> This FAIL is because of the latent BUG of VSETVL PASS.
>
> So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
>
> I have confirm the whole regression no additional FAILs are introduced.
>
>         PR target/111391
>
> gcc/ChangeLog:
>
>         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
>         (vec_extract<mode><vel>): Ditto.
>         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
>         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
>         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
>         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
>
> ---
>  gcc/config/riscv/autovec.md                   |  2 +-
>  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
>  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
>  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
>  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
>  5 files changed, 64 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
>
> diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> index e74a1695709..7121bab1716 100644
> --- a/gcc/config/riscv/autovec.md
> +++ b/gcc/config/riscv/autovec.md
> @@ -1442,7 +1442,7 @@
>  ;; -------------------------------------------------------------------------
>  ;; ---- [INT,FP] Extract a vector element.
>  ;; -------------------------------------------------------------------------
> -(define_expand "@vec_extract<mode><vel>"
> +(define_expand "vec_extract<mode><vel>"
 
Why remove this? I saw this change was introduced in v3?
 
 
>    [(set (match_operand:<VEL>     0 "register_operand")
>       (vec_select:<VEL>
>         (match_operand:V_VLS      1 "register_operand")
Kito Cheng Sept. 14, 2023, 9:20 a.m. UTC | #4 
Could you check if it work correctly for rv64gcv_zve32x? add testcase
no matter if it works or not :)

On Thu, Sep 14, 2023 at 5:19 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> Is it Ok for trunk ? Or you want me send a separate patch to remove "@" in vec_extract optab ?
>
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-09-14 16:11
> To: Juzhe-Zhong
> CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
> Subject: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
> On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
> >
> > This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
> >
> > I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> > This FAIL is because of the latent BUG of VSETVL PASS.
> >
> > So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
> >
> > I have confirm the whole regression no additional FAILs are introduced.
> >
> >         PR target/111391
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
> >         (vec_extract<mode><vel>): Ditto.
> >         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
> >         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
> >         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
> >         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
> >
> > ---
> >  gcc/config/riscv/autovec.md                   |  2 +-
> >  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
> >  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
> >  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
> >  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
> >  5 files changed, 64 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
> >
> > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> > index e74a1695709..7121bab1716 100644
> > --- a/gcc/config/riscv/autovec.md
> > +++ b/gcc/config/riscv/autovec.md
> > @@ -1442,7 +1442,7 @@
> >  ;; -------------------------------------------------------------------------
> >  ;; ---- [INT,FP] Extract a vector element.
> >  ;; -------------------------------------------------------------------------
> > -(define_expand "@vec_extract<mode><vel>"
> > +(define_expand "vec_extract<mode><vel>"
>
> Why remove this? I saw this change was introduced in v3?
>
>
> >    [(set (match_operand:<VEL>     0 "register_operand")
> >       (vec_select:<VEL>
> >         (match_operand:V_VLS      1 "register_operand")
>
钟居哲 Sept. 14, 2023, 9:23 a.m. UTC | #5 
You mean try pr111391.c 
that I added with rv64gcv_zve32x ?



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-09-14 17:20
To: juzhe.zhong@rivai.ai
CC: gcc-patches; Kito.cheng; jeffreyalaw; Robin Dapp
Subject: Re: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
Could you check if it work correctly for rv64gcv_zve32x? add testcase
no matter if it works or not :)
 
On Thu, Sep 14, 2023 at 5:19 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> Is it Ok for trunk ? Or you want me send a separate patch to remove "@" in vec_extract optab ?
>
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-09-14 16:11
> To: Juzhe-Zhong
> CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
> Subject: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
> On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
> >
> > This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
> >
> > I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> > This FAIL is because of the latent BUG of VSETVL PASS.
> >
> > So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
> >
> > I have confirm the whole regression no additional FAILs are introduced.
> >
> >         PR target/111391
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
> >         (vec_extract<mode><vel>): Ditto.
> >         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
> >         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
> >         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
> >         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
> >
> > ---
> >  gcc/config/riscv/autovec.md                   |  2 +-
> >  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
> >  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
> >  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
> >  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
> >  5 files changed, 64 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
> >
> > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> > index e74a1695709..7121bab1716 100644
> > --- a/gcc/config/riscv/autovec.md
> > +++ b/gcc/config/riscv/autovec.md
> > @@ -1442,7 +1442,7 @@
> >  ;; -------------------------------------------------------------------------
> >  ;; ---- [INT,FP] Extract a vector element.
> >  ;; -------------------------------------------------------------------------
> > -(define_expand "@vec_extract<mode><vel>"
> > +(define_expand "vec_extract<mode><vel>"
>
> Why remove this? I saw this change was introduced in v3?
>
>
> >    [(set (match_operand:<VEL>     0 "register_operand")
> >       (vec_select:<VEL>
> >         (match_operand:V_VLS      1 "register_operand")
>
钟居哲 Sept. 14, 2023, 9:26 a.m. UTC | #6 
Oh I see.
It ICE:

during RTL pass: expand
bug.c:26:9: internal compiler error: in require, at machmode.h:313
   26 |         i (a);
      |         ^~~~~
0x1032253 opt_mode<machine_mode>::require() const
        ../../../../gcc/gcc/machmode.h:313
0x1c47877 riscv_legitimize_move(machine_mode, rtx_def*, rtx_def*)
        ../../../../gcc/gcc/config/riscv/riscv.cc:2532
0x274bbe0 gen_movdi(rtx_def*, rtx_def*)
        ../../../../gcc/gcc/config/riscv/riscv.md:2024
0x102cb1c rtx_insn* insn_gen_fn::operator()<rtx_def*, rtx_def*>(rtx_def*, rtx_def*) const
        ../../../../gcc/gcc/recog.h:411
0x11fbc8e emit_move_insn_1(rtx_def*, rtx_def*)
        ../../../../gcc/gcc/expr.cc:4164
0x11fc809 emit_move_insn(rtx_def*, rtx_def*)
        ../../../../gcc/gcc/expr.cc:4334
0x1039a0b load_register_parameters
        ../../../../gcc/gcc/calls.cc:2155
0x103d865 expand_call(tree_node*, rtx_def*, int)
        ../../../../gcc/gcc/calls.cc:3626
0x121e78c expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        ../../../../gcc/gcc/expr.cc:11921
0x120ffb8 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        ../../../../gcc/gcc/expr.cc:9010
0x102c694 expand_expr(tree_node*, rtx_def*, machine_mode, expand_modifier)
        ../../../../gcc/gcc/expr.h:310
0x105ccc9 expand_call_stmt
        ../../../../gcc/gcc/cfgexpand.cc:2831
0x10608af expand_gimple_stmt_1
        ../../../../gcc/gcc/cfgexpand.cc:3880
0x1060f4d expand_gimple_stmt
        ../../../../gcc/gcc/cfgexpand.cc:4044
0x10699f3 expand_gimple_basic_block


Thanks for catching this.



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-09-14 17:20
To: juzhe.zhong@rivai.ai
CC: gcc-patches; Kito.cheng; jeffreyalaw; Robin Dapp
Subject: Re: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
Could you check if it work correctly for rv64gcv_zve32x? add testcase
no matter if it works or not :)
 
On Thu, Sep 14, 2023 at 5:19 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> Is it Ok for trunk ? Or you want me send a separate patch to remove "@" in vec_extract optab ?
>
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-09-14 16:11
> To: Juzhe-Zhong
> CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
> Subject: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
> On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
> >
> > This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
> >
> > I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> > This FAIL is because of the latent BUG of VSETVL PASS.
> >
> > So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
> >
> > I have confirm the whole regression no additional FAILs are introduced.
> >
> >         PR target/111391
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
> >         (vec_extract<mode><vel>): Ditto.
> >         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
> >         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
> >         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
> >         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
> >
> > ---
> >  gcc/config/riscv/autovec.md                   |  2 +-
> >  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
> >  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
> >  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
> >  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
> >  5 files changed, 64 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
> >
> > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> > index e74a1695709..7121bab1716 100644
> > --- a/gcc/config/riscv/autovec.md
> > +++ b/gcc/config/riscv/autovec.md
> > @@ -1442,7 +1442,7 @@
> >  ;; -------------------------------------------------------------------------
> >  ;; ---- [INT,FP] Extract a vector element.
> >  ;; -------------------------------------------------------------------------
> > -(define_expand "@vec_extract<mode><vel>"
> > +(define_expand "vec_extract<mode><vel>"
>
> Why remove this? I saw this change was introduced in v3?
>
>
> >    [(set (match_operand:<VEL>     0 "register_operand")
> >       (vec_select:<VEL>
> >         (match_operand:V_VLS      1 "register_operand")
>
Kito Cheng Sept. 14, 2023, 9:26 a.m. UTC | #7 
Yeah, try pr111391.c with rv64gc_zve32x (NO v, my mistake in last mail
:P), maybe add a testcase pr111391-zve32x.c that just include
pr111391.c and set dg option to rv64gc_zve32x


On Thu, Sep 14, 2023 at 5:24 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> You mean try pr111391.c
> that I added with rv64gcv_zve32x ?
>
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-09-14 17:20
> To: juzhe.zhong@rivai.ai
> CC: gcc-patches; Kito.cheng; jeffreyalaw; Robin Dapp
> Subject: Re: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
> Could you check if it work correctly for rv64gcv_zve32x? add testcase
> no matter if it works or not :)
>
> On Thu, Sep 14, 2023 at 5:19 PM juzhe.zhong@rivai.ai
> <juzhe.zhong@rivai.ai> wrote:
> >
> > Is it Ok for trunk ? Or you want me send a separate patch to remove "@" in vec_extract optab ?
> >
> >
> >
> > juzhe.zhong@rivai.ai
> >
> > From: Kito Cheng
> > Date: 2023-09-14 16:11
> > To: Juzhe-Zhong
> > CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
> > Subject: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
> > On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
> > >
> > > This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
> > >
> > > I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> > > This FAIL is because of the latent BUG of VSETVL PASS.
> > >
> > > So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
> > >
> > > I have confirm the whole regression no additional FAILs are introduced.
> > >
> > >         PR target/111391
> > >
> > > gcc/ChangeLog:
> > >
> > >         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
> > >         (vec_extract<mode><vel>): Ditto.
> > >         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
> > >         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
> > >         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
> > >         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
> > >
> > > ---
> > >  gcc/config/riscv/autovec.md                   |  2 +-
> > >  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
> > >  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
> > >  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
> > >  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
> > >  5 files changed, 64 insertions(+), 3 deletions(-)
> > >  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
> > >
> > > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> > > index e74a1695709..7121bab1716 100644
> > > --- a/gcc/config/riscv/autovec.md
> > > +++ b/gcc/config/riscv/autovec.md
> > > @@ -1442,7 +1442,7 @@
> > >  ;; -------------------------------------------------------------------------
> > >  ;; ---- [INT,FP] Extract a vector element.
> > >  ;; -------------------------------------------------------------------------
> > > -(define_expand "@vec_extract<mode><vel>"
> > > +(define_expand "vec_extract<mode><vel>"
> >
> > Why remove this? I saw this change was introduced in v3?
> >
> >
> > >    [(set (match_operand:<VEL>     0 "register_operand")
> > >       (vec_select:<VEL>
> > >         (match_operand:V_VLS      1 "register_operand")
> >
>
钟居哲 Sept. 14, 2023, 10:08 a.m. UTC | #8 
Hi. Kito.

Could you review this code ? Regression is running....
  /* Expand
       (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
     Expand this data movement instead of simply forbid it since
     we can improve the code generation for this following scenario
     by RVV auto-vectorization:
       (set (reg:V8QI 149) (vec_duplicate:V8QI (reg:QI))
       (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
     Since RVV mode and scalar mode are in different REG_CLASS,
     we need to explicitly move data from V_REGS to GR_REGS by scalar move.  */
  if (SUBREG_P (src) && riscv_v_ext_mode_p (GET_MODE (SUBREG_REG (src))))
    {
      machine_mode vmode = GET_MODE (SUBREG_REG (src));
      unsigned int mode_size = GET_MODE_SIZE (mode).to_constant ();
      unsigned int vmode_size = GET_MODE_SIZE (vmode).to_constant ();
      unsigned int nunits = vmode_size / mode_size;
      scalar_mode smode = as_a<scalar_mode> (mode);
      unsigned int index = SUBREG_BYTE (src).to_constant () / mode_size;
      unsigned int num = smode == DImode && !TARGET_VECTOR_ELEN_64 ? 2 : 1;

      if (num == 2)
        {
          /* If we want to extract 64bit value but ELEN < 64,
             we use RVV vector mode with EEW = 32 to extract
             the highpart and lowpart.  */
          smode = SImode;
          nunits = nunits * 2;
        }
      vmode = riscv_vector::get_vector_mode (smode, nunits).require ();
      enum insn_code icode
        = convert_optab_handler (vec_extract_optab, vmode, smode);
      gcc_assert (icode != CODE_FOR_nothing);
      rtx v = gen_lowpart (vmode, SUBREG_REG (src));

      for (unsigned int i = 0; i < num; i++)
        {
          class expand_operand ops[3];
          rtx result;
          if (num == 1)
            result = dest;
          else if (i == 0)
            result = gen_lowpart (smode, dest);
          else
            result = gen_reg_rtx (smode);
          create_output_operand (&ops[0], result, smode);
          ops[0].target = 1;
          create_input_operand (&ops[1], v, vmode);
          create_integer_operand (&ops[2], index + i);
          expand_insn (icode, 3, ops);
          if (ops[0].value != result)
            emit_move_insn (result, ops[0].value);

          if (i == 1)
            {
              rtx tmp
                = expand_binop (Pmode, ashl_optab, gen_lowpart (Pmode, result),
                                gen_int_mode (32, Pmode), NULL_RTX, 0,
                                OPTAB_DIRECT);
              rtx tmp2 = expand_binop (Pmode, ior_optab, tmp, dest, NULL_RTX, 0,
                                       OPTAB_DIRECT);
              emit_move_insn (dest, tmp2);
            }
        }
      return true;
    }


ASM:
vsetivli zero,2,e32,mf2,ta,ma
vslidedown.vi v2,v1,1
vmv.x.s a5,v2
slli a5,a5,32
vmv.x.s a0,v1
or a0,a5,a0



juzhe.zhong@rivai.ai
 
From: Kito Cheng
Date: 2023-09-14 17:26
To: juzhe.zhong@rivai.ai
CC: gcc-patches; Kito.cheng; jeffreyalaw; Robin Dapp
Subject: Re: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
Yeah, try pr111391.c with rv64gc_zve32x (NO v, my mistake in last mail
:P), maybe add a testcase pr111391-zve32x.c that just include
pr111391.c and set dg option to rv64gc_zve32x
 
 
On Thu, Sep 14, 2023 at 5:24 PM juzhe.zhong@rivai.ai
<juzhe.zhong@rivai.ai> wrote:
>
> You mean try pr111391.c
> that I added with rv64gcv_zve32x ?
>
>
>
> juzhe.zhong@rivai.ai
>
> From: Kito Cheng
> Date: 2023-09-14 17:20
> To: juzhe.zhong@rivai.ai
> CC: gcc-patches; Kito.cheng; jeffreyalaw; Robin Dapp
> Subject: Re: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
> Could you check if it work correctly for rv64gcv_zve32x? add testcase
> no matter if it works or not :)
>
> On Thu, Sep 14, 2023 at 5:19 PM juzhe.zhong@rivai.ai
> <juzhe.zhong@rivai.ai> wrote:
> >
> > Is it Ok for trunk ? Or you want me send a separate patch to remove "@" in vec_extract optab ?
> >
> >
> >
> > juzhe.zhong@rivai.ai
> >
> > From: Kito Cheng
> > Date: 2023-09-14 16:11
> > To: Juzhe-Zhong
> > CC: gcc-patches; kito.cheng; jeffreyalaw; rdapp.gcc
> > Subject: Re: [PATCH V3] RISC-V: Expand VLS mode to scalar mode move[PR111391]
> > On Thu, Sep 14, 2023 at 4:04 PM Juzhe-Zhong <juzhe.zhong@rivai.ai> wrote:
> > >
> > > This patch fixes https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111391
> > >
> > > I notice that previous patch (V2 patch) cause additional execution fail of pr69719.c
> > > This FAIL is because of the latent BUG of VSETVL PASS.
> > >
> > > So this patch includes VSETVL PASS fix even though it's not related to the PR111391.
> > >
> > > I have confirm the whole regression no additional FAILs are introduced.
> > >
> > >         PR target/111391
> > >
> > > gcc/ChangeLog:
> > >
> > >         * config/riscv/autovec.md (@vec_extract<mode><vel>): Remove @.
> > >         (vec_extract<mode><vel>): Ditto.
> > >         * config/riscv/riscv-vsetvl.cc (emit_vsetvl_insn): Fix bug.
> > >         (pass_vsetvl::local_eliminate_vsetvl_insn): Ditto.
> > >         * config/riscv/riscv.cc (riscv_legitimize_move): Expand move.
> > >
> > > gcc/testsuite/ChangeLog:
> > >
> > >         * gcc.target/riscv/rvv/autovec/partial/slp-9.c: Adapt test.
> > >         * gcc.target/riscv/rvv/autovec/pr111391.c: New test.
> > >
> > > ---
> > >  gcc/config/riscv/autovec.md                   |  2 +-
> > >  gcc/config/riscv/riscv-vsetvl.cc              |  4 ++-
> > >  gcc/config/riscv/riscv.cc                     | 32 +++++++++++++++++++
> > >  .../riscv/rvv/autovec/partial/slp-9.c         |  1 -
> > >  .../gcc.target/riscv/rvv/autovec/pr111391.c   | 28 ++++++++++++++++
> > >  5 files changed, 64 insertions(+), 3 deletions(-)
> > >  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
> > >
> > > diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
> > > index e74a1695709..7121bab1716 100644
> > > --- a/gcc/config/riscv/autovec.md
> > > +++ b/gcc/config/riscv/autovec.md
> > > @@ -1442,7 +1442,7 @@
> > >  ;; -------------------------------------------------------------------------
> > >  ;; ---- [INT,FP] Extract a vector element.
> > >  ;; -------------------------------------------------------------------------
> > > -(define_expand "@vec_extract<mode><vel>"
> > > +(define_expand "vec_extract<mode><vel>"
> >
> > Why remove this? I saw this change was introduced in v3?
> >
> >
> > >    [(set (match_operand:<VEL>     0 "register_operand")
> > >       (vec_select:<VEL>
> > >         (match_operand:V_VLS      1 "register_operand")
> >
>
diff mbox series

Patch

diff --git a/gcc/config/riscv/autovec.md b/gcc/config/riscv/autovec.md
index e74a1695709..7121bab1716 100644
--- a/gcc/config/riscv/autovec.md
+++ b/gcc/config/riscv/autovec.md
@@ -1442,7 +1442,7 @@ 
 ;; -------------------------------------------------------------------------
 ;; ---- [INT,FP] Extract a vector element.
 ;; -------------------------------------------------------------------------
-(define_expand "@vec_extract<mode><vel>"
+(define_expand "vec_extract<mode><vel>"
   [(set (match_operand:<VEL>	  0 "register_operand")
      (vec_select:<VEL>
        (match_operand:V_VLS	  1 "register_operand")
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index f81361c4ccd..7731e2a5f20 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -649,6 +649,8 @@  emit_vsetvl_insn (enum vsetvl_type insn_type, enum emit_type emit_type,
     {
       fprintf (dump_file, "\nInsert vsetvl insn PATTERN:\n");
       print_rtl_single (dump_file, pat);
+      fprintf (dump_file, "\nfor insn:\n");
+      print_rtl_single (dump_file, rinsn);
     }
 
   if (emit_type == EMIT_DIRECT)
@@ -3861,7 +3863,7 @@  pass_vsetvl::local_eliminate_vsetvl_insn (const bb_info *bb) const
 	      skip_one = true;
 	    }
 
-	  curr_avl = get_avl (rinsn);
+	  curr_avl = curr_dem.get_avl ();
 
 	  /* Some instrucion like pred_extract_first<mode> don't reqruie avl, so
 	     the avl is null, use vl_placeholder for unify the handling
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 762937b0e37..3ba6379028f 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -2513,6 +2513,38 @@  riscv_legitimize_move (machine_mode mode, rtx dest, rtx src)
 	}
       return true;
     }
+  /* Expand
+       (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
+     Expand this data movement instead of simply forbid it since
+     we can improve the code generation for this following scenario
+     by RVV auto-vectorization:
+       (set (reg:V8QI 149) (vec_duplicate:V8QI (reg:QI))
+       (set (reg:DI target) (subreg:DI (reg:V8QI reg) 0))
+     Since RVV mode and scalar mode are in different REG_CLASS,
+     we need to explicitly move data from V_REGS to GR_REGS by scalar move.  */
+  if (SUBREG_P (src) && riscv_v_ext_mode_p (GET_MODE (SUBREG_REG (src))))
+    {
+      machine_mode vmode = GET_MODE (SUBREG_REG (src));
+      unsigned int mode_size = GET_MODE_SIZE (mode).to_constant ();
+      unsigned int vmode_size = GET_MODE_SIZE (vmode).to_constant ();
+      unsigned int nunits = vmode_size / mode_size;
+      scalar_mode smode = as_a<scalar_mode> (mode);
+      vmode = riscv_vector::get_vector_mode (smode, nunits).require ();
+      enum insn_code icode
+	= convert_optab_handler (vec_extract_optab, vmode, mode);
+      gcc_assert (icode != CODE_FOR_nothing);
+      class expand_operand ops[3];
+      create_output_operand (&ops[0], dest, mode);
+      ops[0].target = 1;
+      create_input_operand (&ops[1], gen_lowpart (vmode, SUBREG_REG (src)),
+			    vmode);
+      unsigned int index = SUBREG_BYTE (src).to_constant () / mode_size;
+      create_integer_operand (&ops[2], index);
+      expand_insn (icode, 3, ops);
+      if (ops[0].value != dest)
+	emit_move_insn (dest, ops[0].value);
+      return true;
+    }
   /* Expand
        (set (reg:QI target) (mem:QI (address)))
      to
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/partial/slp-9.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/partial/slp-9.c
index 5fba27c7a35..7c42438c9d9 100644
--- a/gcc/testsuite/gcc.target/riscv/rvv/autovec/partial/slp-9.c
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/partial/slp-9.c
@@ -29,4 +29,3 @@ 
 TEST_ALL (VEC_PERM)
 
 /* { dg-final { scan-assembler-times {viota.m} 2 } } */
-/* { dg-final { scan-assembler-not {vmv\.v\.i} } } */
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
new file mode 100644
index 00000000000..a7f64c937c6
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/autovec/pr111391.c
@@ -0,0 +1,28 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -Wno-int-conversion -Wno-implicit-function -Wno-incompatible-pointer-types -Wno-implicit-function-declaration -Ofast -ftree-vectorize" } */
+
+int d ();
+typedef struct
+{
+  int b;
+} c;
+int
+e (char *f, long g)
+{
+  f += g;
+  while (g--)
+    *--f = d;
+}
+
+int
+d (c * f)
+{
+  while (h ())
+    switch (f->b)
+      case 'Q':
+      {
+	long a;
+	e (&a, sizeof (a));
+	i (a);
+      }
+}