> You might want to investigate why you get mask and not Len for a particular > stmt. mixing will cause variable length vectorization to fail.
Yes, the new added gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.c cannot vectorize, will try to investigate why. Pan -----Original Message----- From: Richard Biener <richard.guent...@gmail.com> Sent: Monday, March 11, 2024 1:05 AM To: Li, Pan2 <pan2...@intel.com> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com; Wang, Yanzhang <yanzhang.w...@intel.com>; rdapp....@gmail.com; jeffreya...@gmail.com Subject: Re: [PATCH v2] VECT: Fix ICE for vectorizable LD/ST when both len and store are enabled > Am 10.03.2024 um 11:02 schrieb Li, Pan2 <pan2...@intel.com>: > > Committed, thanks Richard. You might want to investigate why you get mask and not Len for a particular stmt. mixing will cause variable length vectorization to fail. > Pan > > -----Original Message----- > From: Richard Biener <richard.guent...@gmail.com> > Sent: Sunday, March 10, 2024 2:53 PM > To: Li, Pan2 <pan2...@intel.com> > Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com; > Wang, Yanzhang <yanzhang.w...@intel.com>; rdapp....@gmail.com; > jeffreya...@gmail.com > Subject: Re: [PATCH v2] VECT: Fix ICE for vectorizable LD/ST when both len > and store are enabled > > > >> Am 10.03.2024 um 04:14 schrieb pan2...@intel.com: >> >> From: Pan Li <pan2...@intel.com> >> >> This patch would like to fix one ICE in vectorizable_store when both the >> loop_masks and loop_lens are enabled. The ICE looks like below when build >> with "-march=rv64gcv -O3". >> >> during GIMPLE pass: vect >> test.c: In function ‘d’: >> test.c:6:6: internal compiler error: in vectorizable_store, at >> tree-vect-stmts.cc:8691 >> 6 | void d() { >> | ^ >> 0x37a6f2f vectorizable_store >> .../__RISC-V_BUILD__/../gcc/tree-vect-stmts.cc:8691 >> 0x37b861c vect_analyze_stmt(vec_info*, _stmt_vec_info*, bool*, >> _slp_tree*, _slp_instance*, vec<stmt_info_for_cost, va_heap, vl_ptr>*) >> .../__RISC-V_BUILD__/../gcc/tree-vect-stmts.cc:13242 >> 0x1db5dca vect_analyze_loop_operations >> .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:2208 >> 0x1db885b vect_analyze_loop_2 >> .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3041 >> 0x1dba029 vect_analyze_loop_1 >> .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3481 >> 0x1dbabad vect_analyze_loop(loop*, vec_info_shared*) >> .../__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3639 >> 0x1e389d1 try_vectorize_loop_1 >> .../__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1066 >> 0x1e38f3d try_vectorize_loop >> .../__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1182 >> 0x1e39230 execute >> .../__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1298 >> >> There are two ways to reach vectorizer LD/ST, one is the analysis and >> the other is transform. We cannot have both the lens and the masks >> enabled during transform but it is valid during analysis. Given the >> transform doesn't required cost_vec, we can only enable the assert >> based on cost_vec is NULL or not. >> >> Below testsuites are passed for this patch: >> * The x86 bootstrap tests. >> * The x86 fully regression tests. >> * The aarch64 fully regression tests. >> * The riscv fully regressison tests. > > Ok > > Thanks, > Richard > >> gcc/ChangeLog: >> >> * tree-vect-stmts.cc (vectorizable_store): Enable the assert >> during transform process. >> (vectorizable_load): Ditto. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/riscv/rvv/base/pr114195-1.c: New test. >> >> Signed-off-by: Pan Li <pan2...@intel.com> >> --- >> .../gcc.target/riscv/rvv/base/pr114195-1.c | 15 +++++++++++++++ >> gcc/tree-vect-stmts.cc | 18 ++++++++++++++---- >> 2 files changed, 29 insertions(+), 4 deletions(-) >> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.c >> >> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.c >> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.c >> new file mode 100644 >> index 00000000000..a67b847112b >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr114195-1.c >> @@ -0,0 +1,15 @@ >> +/* Test that we do not have ice when compile */ >> +/* { dg-do compile } */ >> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -ftree-vectorize" } */ >> + >> +long a, b; >> +extern short c[]; >> + >> +void d() { >> + for (int e = 0; e < 35; e = 2) { >> + a = ({ a < 0 ? a : 0; }); >> + b = ({ b < 0 ? b : 0; }); >> + >> + c[e] = 0; >> + } >> +} >> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc >> index 14a3ffb5f02..e8617439a48 100644 >> --- a/gcc/tree-vect-stmts.cc >> +++ b/gcc/tree-vect-stmts.cc >> @@ -8697,8 +8697,13 @@ vectorizable_store (vec_info *vinfo, >> ? &LOOP_VINFO_LENS (loop_vinfo) >> : NULL); >> >> - /* Shouldn't go with length-based approach if fully masked. */ >> - gcc_assert (!loop_lens || !loop_masks); >> + /* The vect_transform_stmt and vect_analyze_stmt will go here but there >> + are some difference here. We cannot enable both the lens and masks >> + during transform but it is allowed during analysis. >> + Shouldn't go with length-based approach if fully masked. */ >> + if (cost_vec == NULL) >> + /* The cost_vec is NULL during transfrom. */ >> + gcc_assert ((!loop_lens || !loop_masks)); >> >> /* Targets with store-lane instructions must not require explicit >> realignment. vect_supportable_dr_alignment always returns either >> @@ -10577,8 +10582,13 @@ vectorizable_load (vec_info *vinfo, >> ? &LOOP_VINFO_LENS (loop_vinfo) >> : NULL); >> >> - /* Shouldn't go with length-based approach if fully masked. */ >> - gcc_assert (!loop_lens || !loop_masks); >> + /* The vect_transform_stmt and vect_analyze_stmt will go here but there >> + are some difference here. We cannot enable both the lens and masks >> + during transform but it is allowed during analysis. >> + Shouldn't go with length-based approach if fully masked. */ >> + if (cost_vec == NULL) >> + /* The cost_vec is NULL during transfrom. */ >> + gcc_assert ((!loop_lens || !loop_masks)); >> >> /* Targets with store-lane instructions must not require explicit >> realignment. vect_supportable_dr_alignment always returns either >> -- >> 2.34.1 >>