Hi Richard,
After some more investigation, the sample code never hit one vectorizable_*
routines which may check the loop_vinfo->vector_mode,
and then the loop_vinfo->vector_mode == DImode will hit the
vect_verify_loop_lens and trigger the assert VECTOR_MODE_P, detail
flow as below.
vect_analyze_loop_2
|- vect_pattern_recog // Hit over-widening pattern and set
loop_vinfo->vector_mode to DImode
|- ...
|- vect_analyze_loop_operations
|- (gdb) p stmt_info->def_type
|- $1 = vect_reduction_def
|- (gdb) p stmt_info->slp_type
|- $2 = pure_slp
|- vectorizable_lc_phi // Not Hit
|- vectorizable_induction // Not Hit
|- vectorizable_reduction // Not Hit
|- vectorizable_recurr // Not Hit
|- vectorizable_live_operation // Not Hit
|- vect_analyze_stmt
|- (gdb) p stmt_info->relevant
|- $3 = vect_unused_in_scope
|- (gdb) p stmt_info->live
|- $4 = false
|- (gdb) p pattern_stmt_info
|- $5 = (stmt_vec_info) 0x0
|- return opt_result::success ();
OR
|- PURE_SLP_STMT (stmt_info) && !node then dump "handled only by SLP
analysis\n"
|- Early return opt_result::success ();
|- vectorizable_load/store/call_convert/... // Not Hit
|- LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P && !LOOP_VINFO_MASKS
(loop_vinfo).is_empty ()
|- vect_verify_loop_lens (loop_vinfo)
|- assert (VECTOR_MODE_P (loop_vinfo->vector_mode); // Hit assert result
in ICE
I am a little hesitant by two options here.
1. shall we add some condition and dump log here to make the
vect_analyze_loop_2 failure when loop_vinfo->vector_mode is not supported
vector mode by target.
2. it should not be LOOP_VINFO_CAN_USE_PARTIAL_VECTORS_P here? Then we need to
find out where set the partial vector to true.
Is there any suggestion here?
Pan
-----Original Message-----
From: Li, Pan2
Sent: Monday, February 17, 2025 6:08 PM
To: Richard Biener <[email protected]>
Cc: [email protected]; [email protected]; [email protected];
[email protected]; [email protected]
Subject: RE: [PATCH v1] Vect: Fix ICE when get DImode from
get_related_vectype_for_scalar_type [PR116351]
> But that's wrong - read the comment before the code. We do support integer
> mode
> "generic" vectorization just fine. Iff there's anything to plug then
> it's how we end
> up thinking there's with_len support for DImode vectors.
I see, then we need another place to fix this, let me have a try.
Pan
-----Original Message-----
From: Richard Biener <[email protected]>
Sent: Monday, February 17, 2025 6:02 PM
To: Li, Pan2 <[email protected]>
Cc: [email protected]; [email protected]; [email protected];
[email protected]; [email protected]
Subject: Re: [PATCH v1] Vect: Fix ICE when get DImode from
get_related_vectype_for_scalar_type [PR116351]
On Mon, Feb 17, 2025 at 10:38 AM <[email protected]> wrote:
>
> From: Pan Li <[email protected]>
>
> This patch would like to fix the ICE similar as below, assump we have
> sample code:
>
> 1 │ int a, b, c;
> 2 │ short d, e, f;
> 3 │ long g (long h) { return h; }
> 4 │
> 5 │ void i () {
> 6 │ for (; b; ++b) {
> 7 │ f = 5 >> a ? d : d << a;
> 8 │ e &= c | g(f);
> 9 │ }
> 10 │ }
>
> It will ice when compile with -O3 -march=rv64gc_zve64f -mrvv-vector-bits=zvl
>
> during GIMPLE pass: vect
> pr116351-1.c: In function ‘i’:
> pr116351-1.c:8:6: internal compiler error: in get_len_load_store_mode,
> at optabs-tree.cc:655
> 8 | void i () {
> | ^
> 0x44d6b9d internal_error(char const*, ...)
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/diagnostic-global-context.cc:517
> 0x44a26a6 fancy_abort(char const*, int, char const*)
>
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/diagnostic.cc:1722
> 0x19e4309 get_len_load_store_mode(machine_mode, bool, internal_fn*,
> vec<int, va_heap, vl_ptr>*)
>
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/optabs-tree.cc:655
> 0x1fada40 vect_verify_loop_lens
>
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:1566
> 0x1fb2b07 vect_analyze_loop_2
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3037
> 0x1fb4302 vect_analyze_loop_1
>
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3478
> 0x1fb4e9a vect_analyze_loop(loop*, gimple*, vec_info_shared*)
>
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/tree-vect-loop.cc:3638
> 0x203c2dc try_vectorize_loop_1
>
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1095
> 0x203c839 try_vectorize_loop
>
> /home/pli/gcc/111/riscv-gnu-toolchain/gcc/__RISC-V_BUILD__/../gcc/tree-vectorizer.cc:1212
> 0x203cb2c execute
>
> The zve32x cannot have 64 elen, and then the
> get_related_vectype_for_scalar_type
> will get DImode as vector_mode in loop_info. After that the underlying
> vect_analyze_xx will assert the mode is VECTOR and then ICE at the assert.
>
> The fix contains 2 part, aka let the get_related_vectype_for_scalar_type
> return NULL_TREE if mode_for_vector is not VECTOR mode in the middle-end,
> and then mark the innermode of RVV is DImode is not support when the
> TARGET_VECTOR_ELEN_64 is false.
>
> The below test suites are passed for this patch.
> * The rv64gcv fully regression test.
> * The x86 bootstrap test.
> * The x86 fully regression test.
>
> PR target/116351
>
> gcc/ChangeLog:
>
> * config/riscv/riscv.cc (riscv_vector_mode_supported_any_target_p):
> Mark
> innnermode of RVV is DImode unsupported when zve32*.
> * tree-vect-stmts.cc (get_related_vectype_for_scalar_type): Return
> the NULL_TREE if mode_for_vector is not a VECTOR mode.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/riscv/rvv/base/pr116351-1.c: New test.
> * gcc.target/riscv/rvv/base/pr116351-2.c: New test.
> * gcc.target/riscv/rvv/base/pr116351.h: New test.
>
> Signed-off-by: Pan Li <[email protected]>
> ---
> gcc/config/riscv/riscv.cc | 6 +++++-
> .../gcc.target/riscv/rvv/base/pr116351-1.c | 5 +++++
> .../gcc.target/riscv/rvv/base/pr116351-2.c | 5 +++++
> .../gcc.target/riscv/rvv/base/pr116351.h | 18 ++++++++++++++++++
> gcc/tree-vect-stmts.cc | 9 +++++++--
> 5 files changed, 40 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-1.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-2.c
> create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr116351.h
>
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 9bf7713139f..89b534ac88f 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -12613,10 +12613,14 @@ extract_base_offset_in_addr (rtx mem, rtx *base,
> rtx *offset)
> /* Implements target hook vector_mode_supported_any_target_p. */
>
> static bool
> -riscv_vector_mode_supported_any_target_p (machine_mode)
> +riscv_vector_mode_supported_any_target_p (machine_mode mode)
> {
> if (TARGET_XTHEADVECTOR)
> return false;
> +
> + if (GET_MODE_INNER (mode) == DImode && !TARGET_VECTOR_ELEN_64)
> + return false;
> +
> return true;
> }
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-1.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-1.c
> new file mode 100644
> index 00000000000..f58fedfeaf1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-1.c
> @@ -0,0 +1,5 @@
> +/* Test that we do not have ice when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_zve32x -mabi=lp64d -O3 -ftree-vectorize" } */
> +
> +#include "pr116351.h"
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-2.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-2.c
> new file mode 100644
> index 00000000000..e1f46b745e2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351-2.c
> @@ -0,0 +1,5 @@
> +/* Test that we do not have ice when compile */
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gc_zve32f -mabi=lp64d -O3 -ftree-vectorize" } */
> +
> +#include "pr116351.h"
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351.h
> b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351.h
> new file mode 100644
> index 00000000000..25bd0ec65e0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116351.h
> @@ -0,0 +1,18 @@
> +#ifndef PR116351_H
> +#define PR116351_H
> +
> +#define T long
> +
> +int a, b, c;
> +short d, e, f;
> +
> +T g (T h) { return h; }
> +
> +void i () {
> + for (; b; ++b) {
> + f = 5 >> a ? d : d << a;
> + e &= c | g(f);
> + }
> +}
> +
> +#endif
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 6bbb16beff2..a4a959185da 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -14228,8 +14228,13 @@ get_related_vectype_for_scalar_type (machine_mode
> prevailing_mode,
>
> Note that nunits == 1 is allowed in order to support single
> element vector types. */
> - if (!multiple_p (GET_MODE_SIZE (simd_mode), nbytes, &nunits)
> - || !mode_for_vector (inner_mode, nunits).exists (&simd_mode))
> + if (!multiple_p (GET_MODE_SIZE (simd_mode), nbytes, &nunits))
> + return NULL_TREE;
> +
> + bool exist_p = mode_for_vector (inner_mode,
> + nunits).exists (&simd_mode);
> +
> + if (!exist_p || (exist_p && !VECTOR_MODE_P (simd_mode)))
But that's wrong - read the comment before the code. We do support integer mode
"generic" vectorization just fine. Iff there's anything to plug then
it's how we end
up thinking there's with_len support for DImode vectors.
Richard.
> return NULL_TREE;
> }
> }
> --
> 2.43.0
>
