Re: Re: [PATCH] RISC-V: Remove phase 6 of vsetvl pass in GCC13[PR111412]
commited, thanks kito and juzhe. -- Li Xu >I think it's not make too much sense to back port GCC14's change to >GCC 13, removing phase 6 optimization is reasonable to me, so LGTM :) > >On Mon, Sep 18, 2023 at 2:44 PM juzhe.zh...@rivai.ai > wrote: >> >> Thanks for fixing it. >> I am ok remove phase 6 optimization which has many latent bugs (in GCC 14 >> kito has refactored it) there. >> But I think we need kito's more comments about that. >> >> >> >> juzhe.zh...@rivai.ai >> >> From: Li Xu >> Date: 2023-09-18 12:19 >> To: gcc-patches >> CC: kito.cheng; palmer; juzhe.zhong; xuli >> Subject: [PATCH] RISC-V: Remove phase 6 of vsetvl pass in GCC13[PR111412] >> From: xuli >> >> vsetvl pass has been refactored in gcc14, and the optimization >> is more reasonable than releases/gcc-13. This problem does not >> exist in gcc14. >> >> Phase 6 of gcc13 is an optimization patch. Due to lack of consideration, >> there will be some hidden bugs, so we decided to remove phase 6. >> Although the generated code will be redundant, the program is correct. >> >> PR target/111412 >> >> gcc/ChangeLog: >> >> * config/riscv/riscv-vsetvl.cc (vector_infos_manager::release): >>Remove. >> (pass_vsetvl::refine_vsetvls): Ditto. >> (pass_vsetvl::cleanup_vsetvls): Ditto. >> (pass_vsetvl::propagate_avl): Ditto. >> (pass_vsetvl::lazy_vsetvl): Ditto. >> * config/riscv/riscv-vsetvl.h: Ditto. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/rvv/vsetvl/avl_single-79.c: Adjust case. >> * gcc.target/riscv/rvv/vsetvl/avl_single-80.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/avl_single-86.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/avl_single-87.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/avl_single-88.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/avl_single-89.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/avl_single-90.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vlmax_back_prop-25.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vlmax_back_prop-26.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-14.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-15.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vsetvl-1.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vsetvl-5.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vsetvl-6.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vsetvl-7.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vsetvl-8.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vsetvlmax-2.c: Ditto. >> * gcc.target/riscv/rvv/vsetvl/vsetvlmax-4.c: Ditto. >> * gcc.target/riscv/rvv/base/pr111412.c: New test. >> --- >> gcc/config/riscv/riscv-vsetvl.cc | 153 +- >> gcc/config/riscv/riscv-vsetvl.h | 2 - >> .../gcc.target/riscv/rvv/base/pr111412.c | 41 + >> .../riscv/rvv/vsetvl/avl_single-79.c | 4 +- >> .../riscv/rvv/vsetvl/avl_single-80.c | 4 +- >> .../riscv/rvv/vsetvl/avl_single-86.c | 4 +- >> .../riscv/rvv/vsetvl/avl_single-87.c | 4 +- >> .../riscv/rvv/vsetvl/avl_single-88.c | 4 +- >> .../riscv/rvv/vsetvl/avl_single-89.c | 4 +- >> .../riscv/rvv/vsetvl/avl_single-90.c | 4 +- >> .../riscv/rvv/vsetvl/vlmax_back_prop-25.c | 10 +- >> .../riscv/rvv/vsetvl/vlmax_back_prop-26.c | 10 +- >> .../riscv/rvv/vsetvl/vlmax_switch_vtype-14.c | 6 +- >> .../riscv/rvv/vsetvl/vlmax_switch_vtype-15.c | 2 +- >> .../gcc.target/riscv/rvv/vsetvl/vsetvl-1.c | 2 +- >> .../gcc.target/riscv/rvv/vsetvl/vsetvl-5.c | 2 +- >> .../gcc.target/riscv/rvv/vsetvl/vsetvl-6.c | 2 +- >> .../gcc.target/riscv/rvv/vsetvl/vsetvl-7.c | 2 +- >> .../gcc.target/riscv/rvv/vsetvl/vsetvl-8.c | 2 +- >> .../gcc.target/riscv/rvv/vsetvl/vsetvlmax-2.c | 4 +- >> .../gcc.target/riscv/rvv/vsetvl/vsetvlmax-4.c | 4 +- >> 21 files changed, 80 insertions(+), 190 deletions(-) >> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr111412.c >> >> diff --git a/gcc/config/riscv/riscv-vsetvl.cc >> b/gcc/config/riscv/riscv-vsetvl.cc >> index 0cf4bc818e2..9dca2ce709d 100644 >> --- a/gcc/config/riscv/riscv-vsetvl.cc >> +++ b/gcc/config/riscv/riscv-vsetvl.cc >> @@ -2494,8 +2494,6 @@ vector_infos_manager::release (void) >> if (!vector_exprs.is_empty ()) >> vector_exprs.release (); >> - gcc_assert (to_refine_vsetvls.is_empty ()); >> - gcc_assert (to_delete_vsetvls.is_empty ()); >> if (optimize > 0) >> free_bitmap_vectors (); >> } >> @@ -2702,9 +2700,6 @@ private: >> /* Phase 5. */ >> void cleanup_insns (void) const; >> - /* Phase 6. */ >> - void propagate_avl (void) const; >> - >> void init (void); >> void done (void); >> void compute_probabilities (void); >> @@ -3823,10 +3818,8 @@ pass_vsetvl::refine_vsetvls (void) const >> /* We can't refine user vsetvl into vsetvl zero,zero since the dest >>
Re: [PATCH] RISC-V: Remove phase 6 of vsetvl pass in GCC13[PR111412]
I think it's not make too much sense to back port GCC14's change to
GCC 13, removing phase 6 optimization is reasonable to me, so LGTM :)
On Mon, Sep 18, 2023 at 2:44 PM juzhe.zh...@rivai.ai
wrote:
>
> Thanks for fixing it.
> I am ok remove phase 6 optimization which has many latent bugs (in GCC 14
> kito has refactored it) there.
> But I think we need kito's more comments about that.
>
>
>
> juzhe.zh...@rivai.ai
>
> From: Li Xu
> Date: 2023-09-18 12:19
> To: gcc-patches
> CC: kito.cheng; palmer; juzhe.zhong; xuli
> Subject: [PATCH] RISC-V: Remove phase 6 of vsetvl pass in GCC13[PR111412]
> From: xuli
>
> vsetvl pass has been refactored in gcc14, and the optimization
> is more reasonable than releases/gcc-13. This problem does not
> exist in gcc14.
>
> Phase 6 of gcc13 is an optimization patch. Due to lack of consideration,
> there will be some hidden bugs, so we decided to remove phase 6.
> Although the generated code will be redundant, the program is correct.
>
> PR target/111412
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vsetvl.cc (vector_infos_manager::release):
> Remove.
> (pass_vsetvl::refine_vsetvls): Ditto.
> (pass_vsetvl::cleanup_vsetvls): Ditto.
> (pass_vsetvl::propagate_avl): Ditto.
> (pass_vsetvl::lazy_vsetvl): Ditto.
> * config/riscv/riscv-vsetvl.h: Ditto.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/vsetvl/avl_single-79.c: Adjust case.
> * gcc.target/riscv/rvv/vsetvl/avl_single-80.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/avl_single-86.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/avl_single-87.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/avl_single-88.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/avl_single-89.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/avl_single-90.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_back_prop-25.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_back_prop-26.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-14.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-15.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vsetvl-1.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vsetvl-5.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vsetvl-6.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vsetvl-7.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vsetvl-8.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vsetvlmax-2.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vsetvlmax-4.c: Ditto.
> * gcc.target/riscv/rvv/base/pr111412.c: New test.
> ---
> gcc/config/riscv/riscv-vsetvl.cc | 153 +-
> gcc/config/riscv/riscv-vsetvl.h | 2 -
> .../gcc.target/riscv/rvv/base/pr111412.c | 41 +
> .../riscv/rvv/vsetvl/avl_single-79.c | 4 +-
> .../riscv/rvv/vsetvl/avl_single-80.c | 4 +-
> .../riscv/rvv/vsetvl/avl_single-86.c | 4 +-
> .../riscv/rvv/vsetvl/avl_single-87.c | 4 +-
> .../riscv/rvv/vsetvl/avl_single-88.c | 4 +-
> .../riscv/rvv/vsetvl/avl_single-89.c | 4 +-
> .../riscv/rvv/vsetvl/avl_single-90.c | 4 +-
> .../riscv/rvv/vsetvl/vlmax_back_prop-25.c | 10 +-
> .../riscv/rvv/vsetvl/vlmax_back_prop-26.c | 10 +-
> .../riscv/rvv/vsetvl/vlmax_switch_vtype-14.c | 6 +-
> .../riscv/rvv/vsetvl/vlmax_switch_vtype-15.c | 2 +-
> .../gcc.target/riscv/rvv/vsetvl/vsetvl-1.c| 2 +-
> .../gcc.target/riscv/rvv/vsetvl/vsetvl-5.c| 2 +-
> .../gcc.target/riscv/rvv/vsetvl/vsetvl-6.c| 2 +-
> .../gcc.target/riscv/rvv/vsetvl/vsetvl-7.c| 2 +-
> .../gcc.target/riscv/rvv/vsetvl/vsetvl-8.c| 2 +-
> .../gcc.target/riscv/rvv/vsetvl/vsetvlmax-2.c | 4 +-
> .../gcc.target/riscv/rvv/vsetvl/vsetvlmax-4.c | 4 +-
> 21 files changed, 80 insertions(+), 190 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr111412.c
>
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc
> b/gcc/config/riscv/riscv-vsetvl.cc
> index 0cf4bc818e2..9dca2ce709d 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -2494,8 +2494,6 @@ vector_infos_manager::release (void)
>if (!vector_exprs.is_empty ())
> vector_exprs.release ();
> - gcc_assert (to_refine_vsetvls.is_empty ());
> - gcc_assert (to_delete_vsetvls.is_empty ());
>if (optimize > 0)
> free_bitmap_vectors ();
> }
> @@ -2702,9 +2700,6 @@ private:
>/* Phase 5. */
>void cleanup_insns (void) const;
> - /* Phase 6. */
> - void propagate_avl (void) const;
> -
>void init (void);
>void done (void);
>void compute_probabilities (void);
> @@ -3823,10 +3818,8 @@ pass_vsetvl::refine_vsetvls (void) const
>/* We can't refine user vsetvl into vsetvl zero,zero since the dest
> will be used by the following instructions. */
>if (vector_config_insn_p (rinsn))
> - {
> - m_vector_manager->to_refine_vsetvls.add (rinsn);
> continue;
> -
Re: [PATCH] RISC-V: Remove phase 6 of vsetvl pass in GCC13[PR111412]
Thanks for fixing it.
I am ok remove phase 6 optimization which has many latent bugs (in GCC 14 kito
has refactored it) there.
But I think we need kito's more comments about that.
juzhe.zh...@rivai.ai
From: Li Xu
Date: 2023-09-18 12:19
To: gcc-patches
CC: kito.cheng; palmer; juzhe.zhong; xuli
Subject: [PATCH] RISC-V: Remove phase 6 of vsetvl pass in GCC13[PR111412]
From: xuli
vsetvl pass has been refactored in gcc14, and the optimization
is more reasonable than releases/gcc-13. This problem does not
exist in gcc14.
Phase 6 of gcc13 is an optimization patch. Due to lack of consideration,
there will be some hidden bugs, so we decided to remove phase 6.
Although the generated code will be redundant, the program is correct.
PR target/111412
gcc/ChangeLog:
* config/riscv/riscv-vsetvl.cc (vector_infos_manager::release): Remove.
(pass_vsetvl::refine_vsetvls): Ditto.
(pass_vsetvl::cleanup_vsetvls): Ditto.
(pass_vsetvl::propagate_avl): Ditto.
(pass_vsetvl::lazy_vsetvl): Ditto.
* config/riscv/riscv-vsetvl.h: Ditto.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/rvv/vsetvl/avl_single-79.c: Adjust case.
* gcc.target/riscv/rvv/vsetvl/avl_single-80.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/avl_single-86.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/avl_single-87.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/avl_single-88.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/avl_single-89.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/avl_single-90.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_back_prop-25.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_back_prop-26.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-14.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-15.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vsetvl-1.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vsetvl-5.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vsetvl-6.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vsetvl-7.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vsetvl-8.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vsetvlmax-2.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vsetvlmax-4.c: Ditto.
* gcc.target/riscv/rvv/base/pr111412.c: New test.
---
gcc/config/riscv/riscv-vsetvl.cc | 153 +-
gcc/config/riscv/riscv-vsetvl.h | 2 -
.../gcc.target/riscv/rvv/base/pr111412.c | 41 +
.../riscv/rvv/vsetvl/avl_single-79.c | 4 +-
.../riscv/rvv/vsetvl/avl_single-80.c | 4 +-
.../riscv/rvv/vsetvl/avl_single-86.c | 4 +-
.../riscv/rvv/vsetvl/avl_single-87.c | 4 +-
.../riscv/rvv/vsetvl/avl_single-88.c | 4 +-
.../riscv/rvv/vsetvl/avl_single-89.c | 4 +-
.../riscv/rvv/vsetvl/avl_single-90.c | 4 +-
.../riscv/rvv/vsetvl/vlmax_back_prop-25.c | 10 +-
.../riscv/rvv/vsetvl/vlmax_back_prop-26.c | 10 +-
.../riscv/rvv/vsetvl/vlmax_switch_vtype-14.c | 6 +-
.../riscv/rvv/vsetvl/vlmax_switch_vtype-15.c | 2 +-
.../gcc.target/riscv/rvv/vsetvl/vsetvl-1.c| 2 +-
.../gcc.target/riscv/rvv/vsetvl/vsetvl-5.c| 2 +-
.../gcc.target/riscv/rvv/vsetvl/vsetvl-6.c| 2 +-
.../gcc.target/riscv/rvv/vsetvl/vsetvl-7.c| 2 +-
.../gcc.target/riscv/rvv/vsetvl/vsetvl-8.c| 2 +-
.../gcc.target/riscv/rvv/vsetvl/vsetvlmax-2.c | 4 +-
.../gcc.target/riscv/rvv/vsetvl/vsetvlmax-4.c | 4 +-
21 files changed, 80 insertions(+), 190 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr111412.c
diff --git a/gcc/config/riscv/riscv-vsetvl.cc b/gcc/config/riscv/riscv-vsetvl.cc
index 0cf4bc818e2..9dca2ce709d 100644
--- a/gcc/config/riscv/riscv-vsetvl.cc
+++ b/gcc/config/riscv/riscv-vsetvl.cc
@@ -2494,8 +2494,6 @@ vector_infos_manager::release (void)
if (!vector_exprs.is_empty ())
vector_exprs.release ();
- gcc_assert (to_refine_vsetvls.is_empty ());
- gcc_assert (to_delete_vsetvls.is_empty ());
if (optimize > 0)
free_bitmap_vectors ();
}
@@ -2702,9 +2700,6 @@ private:
/* Phase 5. */
void cleanup_insns (void) const;
- /* Phase 6. */
- void propagate_avl (void) const;
-
void init (void);
void done (void);
void compute_probabilities (void);
@@ -3823,10 +3818,8 @@ pass_vsetvl::refine_vsetvls (void) const
/* We can't refine user vsetvl into vsetvl zero,zero since the dest
will be used by the following instructions. */
if (vector_config_insn_p (rinsn))
- {
- m_vector_manager->to_refine_vsetvls.add (rinsn);
continue;
- }
+
rinsn = PREV_INSN (rinsn);
rtx new_pat = gen_vsetvl_pat (VSETVL_VTYPE_CHANGE_ONLY, info, NULL_RTX);
change_insn (rinsn, new_pat);
@@ -3862,10 +3855,7 @@ pass_vsetvl::cleanup_vsetvls ()
/* We can't eliminate user vsetvl since the dest will be used
* by the following instructions. */
if (vector_config_insn_p (insn->rtl ()))
- {
-
