RE: [PATCH] RISC-V: Remove incorrect earliest vsetvl post optimization[PR111313]

[Li, Pan2 via Gcc-patches]

Committed, thanks Kito.

Pan

-Original Message-
From: Gcc-patches  On Behalf 
Of Kito Cheng via Gcc-patches
Sent: Thursday, September 7, 2023 11:39 AM
To: Juzhe-Zhong 
Cc: GCC Patches ; Kito Cheng 
Subject: Re: [PATCH] RISC-V: Remove incorrect earliest vsetvl post 
optimization[PR111313]

LGTM

Juzhe-Zhong  於 2023年9月7日 週四 11:36 寫道：

> This patch removes the incorrect earliest poset vsetvl optimization,
> such bug was found in vect-double-reduc-5.c which is runtime(execution
> fail) and also in PR111313.
>
> For VLMAX intrinsics, we always emit a bogus patter which is vlmax_avl
> (see vector.md) to
> occupy a scalar register which is used by the following RVV instruction
> which is VLMAX AVL.
>
> Then for O2, O3, Ofast, earliest LCM works so well.
> However, for O1, the vlmax_avl is not well optimized in the before pass
> which confused LCM earliest
> so that we will end up with some redundant vsetvli zero,zero instructions
> in O1. (Note that O2 O3 Ofast are all good).
>
> To elide those redundant vsetvli zero,zero, I added
> cleanup_earliest_vsetvls to elide those redundant vsetvls.
>
> Now, after I review the implementation of this post optimizaiton again, I
> found it is incorrect and it is hard to
> do the post optimizations for vsetvls that earliest LCM failed to
> eliminate.
>
> Besides, such performance issues only happen in O1 or O0, such issues may
> not be serious.
> So remove it and we may will find another way (E.g. adjust vlmax_avl
> pattern COST)
> to optimize it if we really need to care about performance for O1.
>
> PR target/111313
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vsetvl.cc
> (pass_vsetvl::cleanup_earliest_vsetvls): Remove.
> (pass_vsetvl::df_post_optimization): Remove incorrect function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/vsetvl/avl_single-13.c: Adapt test.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-17.c: Skip check for
> O1.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-18.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-19.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-20.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-1.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-10.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-11.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-12.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-13.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-14.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-15.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-16.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-17.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-18.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-19.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-2.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-20.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-21.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-22.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-23.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-24.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-25.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-26.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-27.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-28.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-3.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-4.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-5.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-6.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-7.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-8.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-9.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-2.c: Ditto.
> * gcc.target/riscv/rvv/autovec/pr111313.c: New test.
>
> ---
>  gcc/config/riscv/riscv-vsetvl.cc  | 58 ---
>  .../gcc.target/riscv/rvv/autovec/pr111313.c   | 20 +++
>  .../riscv/rvv/vsetvl/avl_single-13.c  |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-17.c   |  8 +--
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-18.c   |  8 +--
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-19.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-20.c   |  4 +-
>  .../gcc.target/riscv/rvv/vsetvl/vlmax_phi-1.c |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-10.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-11.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-12.c   |  2 +-
>  .../riscv/r

Re: [PATCH] RISC-V: Remove incorrect earliest vsetvl post optimization[PR111313]

[Kito Cheng via Gcc-patches]

LGTM

Juzhe-Zhong  於 2023年9月7日 週四 11:36 寫道：

> This patch removes the incorrect earliest poset vsetvl optimization,
> such bug was found in vect-double-reduc-5.c which is runtime(execution
> fail) and also in PR111313.
>
> For VLMAX intrinsics, we always emit a bogus patter which is vlmax_avl
> (see vector.md) to
> occupy a scalar register which is used by the following RVV instruction
> which is VLMAX AVL.
>
> Then for O2, O3, Ofast, earliest LCM works so well.
> However, for O1, the vlmax_avl is not well optimized in the before pass
> which confused LCM earliest
> so that we will end up with some redundant vsetvli zero,zero instructions
> in O1. (Note that O2 O3 Ofast are all good).
>
> To elide those redundant vsetvli zero,zero, I added
> cleanup_earliest_vsetvls to elide those redundant vsetvls.
>
> Now, after I review the implementation of this post optimizaiton again, I
> found it is incorrect and it is hard to
> do the post optimizations for vsetvls that earliest LCM failed to
> eliminate.
>
> Besides, such performance issues only happen in O1 or O0, such issues may
> not be serious.
> So remove it and we may will find another way (E.g. adjust vlmax_avl
> pattern COST)
> to optimize it if we really need to care about performance for O1.
>
> PR target/111313
>
> gcc/ChangeLog:
>
> * config/riscv/riscv-vsetvl.cc
> (pass_vsetvl::cleanup_earliest_vsetvls): Remove.
> (pass_vsetvl::df_post_optimization): Remove incorrect function.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/vsetvl/avl_single-13.c: Adapt test.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-17.c: Skip check for
> O1.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-18.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-19.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-20.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-1.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-10.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-11.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-12.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-13.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-14.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-15.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-16.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-17.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-18.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-19.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-2.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-20.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-21.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-22.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-23.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-24.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-25.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-26.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-27.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-28.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-3.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-4.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-5.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-6.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-7.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-8.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_phi-9.c: Ditto.
> * gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-2.c: Ditto.
> * gcc.target/riscv/rvv/autovec/pr111313.c: New test.
>
> ---
>  gcc/config/riscv/riscv-vsetvl.cc  | 58 ---
>  .../gcc.target/riscv/rvv/autovec/pr111313.c   | 20 +++
>  .../riscv/rvv/vsetvl/avl_single-13.c  |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-17.c   |  8 +--
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-18.c   |  8 +--
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-19.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_bb_prop-20.c   |  4 +-
>  .../gcc.target/riscv/rvv/vsetvl/vlmax_phi-1.c |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-10.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-11.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-12.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-13.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-14.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-15.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-16.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-17.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-18.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-19.c   |  2 +-
>  .../gcc.target/riscv/rvv/vsetvl/vlmax_phi-2.c |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-20.c   |  2 +-
>  .../riscv/rvv/vsetvl/vlmax_phi-21.c   |  2 +-
>  

[PATCH] RISC-V: Remove incorrect earliest vsetvl post optimization[PR111313]

[Juzhe-Zhong]

This patch removes the incorrect earliest poset vsetvl optimization,
such bug was found in vect-double-reduc-5.c which is runtime(execution fail) 
and also in PR111313.

For VLMAX intrinsics, we always emit a bogus patter which is vlmax_avl (see 
vector.md) to
occupy a scalar register which is used by the following RVV instruction which 
is VLMAX AVL.

Then for O2, O3, Ofast, earliest LCM works so well.
However, for O1, the vlmax_avl is not well optimized in the before pass which 
confused LCM earliest
so that we will end up with some redundant vsetvli zero,zero instructions in 
O1. (Note that O2 O3 Ofast are all good).

To elide those redundant vsetvli zero,zero, I added cleanup_earliest_vsetvls to 
elide those redundant vsetvls.

Now, after I review the implementation of this post optimizaiton again, I found 
it is incorrect and it is hard to
do the post optimizations for vsetvls that earliest LCM failed to eliminate.

Besides, such performance issues only happen in O1 or O0, such issues may not 
be serious.
So remove it and we may will find another way (E.g. adjust vlmax_avl pattern 
COST) 
to optimize it if we really need to care about performance for O1.

PR target/111313

gcc/ChangeLog:

* config/riscv/riscv-vsetvl.cc (pass_vsetvl::cleanup_earliest_vsetvls): 
Remove.
(pass_vsetvl::df_post_optimization): Remove incorrect function.

gcc/testsuite/ChangeLog:

* gcc.target/riscv/rvv/vsetvl/avl_single-13.c: Adapt test.
* gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-17.c: Skip check for O1.
* gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-18.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-19.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_bb_prop-20.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-1.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-10.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-11.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-12.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-13.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-14.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-15.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-16.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-17.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-18.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-19.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-2.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-20.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-21.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-22.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-23.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-24.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-25.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-26.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-27.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-28.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-3.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-4.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-5.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-6.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-7.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-8.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_phi-9.c: Ditto.
* gcc.target/riscv/rvv/vsetvl/vlmax_switch_vtype-2.c: Ditto.
* gcc.target/riscv/rvv/autovec/pr111313.c: New test.

---
 gcc/config/riscv/riscv-vsetvl.cc  | 58 ---
 .../gcc.target/riscv/rvv/autovec/pr111313.c   | 20 +++
 .../riscv/rvv/vsetvl/avl_single-13.c  |  2 +-
 .../riscv/rvv/vsetvl/vlmax_bb_prop-17.c   |  8 +--
 .../riscv/rvv/vsetvl/vlmax_bb_prop-18.c   |  8 +--
 .../riscv/rvv/vsetvl/vlmax_bb_prop-19.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_bb_prop-20.c   |  4 +-
 .../gcc.target/riscv/rvv/vsetvl/vlmax_phi-1.c |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-10.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-11.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-12.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-13.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-14.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-15.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-16.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-17.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-18.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-19.c   |  2 +-
 .../gcc.target/riscv/rvv/vsetvl/vlmax_phi-2.c |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-20.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-21.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-22.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-23.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-24.c   |  2 +-
 .../riscv/rvv/vsetvl/vlmax_phi-25.c   |  2 +-