> 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
>>
