RE: [PATCH] RISC-V: Remove incorrect earliest vsetvl post optimization[PR111313]
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]
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 +- >
