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 <richard.guent...@gmail.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com; 
jeffreya...@gmail.com; rdapp....@gmail.com
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 <richard.guent...@gmail.com> 
Sent: Monday, February 17, 2025 6:02 PM
To: Li, Pan2 <pan2...@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com; 
jeffreya...@gmail.com; rdapp....@gmail.com
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 <pan2...@intel.com> wrote:
>
> From: Pan Li <pan2...@intel.com>
>
> 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 <pan2...@intel.com>
> ---
>  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
>

Reply via email to