Re: Re: [PATCH] middle-end: skipp stepped vector test of poly_int (1, 1) and allow the machine_mode definition with poly_uint16 (1, 1)

[juzhe.zh...@rivai.ai]

Hi, Richard. I tried the codes:
if (!nunits.is_constant () && maybe_gt (nunits, 1)) 
test_vector_subregs_modes (x, nunits - min_nunits, count);

It still failed. For nunits = (1,1) , maybe_gt (nunits, 1) return true value.
But I tried:
if (!nunits.is_constant () && known_gt (nunits, 1)) 
test_vector_subregs_modes (x, nunits - min_nunits, count);
It pass. But it report a warning: "warning: comparison between signed and 
unsigned integer expressions [-Wsign-compare]" during the compilation.

Finally, I tried:
if (!nunits.is_constant () && known_gt (GET_MODE_NUNITS (inner_mode), 1)) 
test_vector_subregs_modes (x, nunits - min_nunits, count);
It passed with no warning.

Is 'known_gt (GET_MODE_NUNITS (inner_mode), 1)' a good solution for this?
Thanks!


juzhe.zh...@rivai.ai
 
From: Richard Sandiford
Date: 2022-08-19 16:03
To: juzhe.zhong
CC: gcc-patches; rguenther; kito.cheng
Subject: Re: [PATCH] middle-end: skipp stepped vector test of poly_int (1, 1) 
and allow the machine_mode definition with poly_uint16 (1, 1)
juzhe.zh...@rivai.ai writes:
> From: zhongjuzhe <juzhe.zh...@rivai.ai>
>
> Hello. This patch is preparing for following RVV support.
>
> Both ARM SVE and RVV (RISC-V 'V' Extension) support length-agnostic vector.
> The minimum vector length of ARM SVE is 128-bit and the runtime invariant of 
> ARM SVE is always 128-bit blocks.
> However, the minimum vector length of RVV can be 32bit in 'Zve32*' 
> sub-extension and 64bit in 'Zve*' sub-extension.
>
> So I define the machine_mode as follows:
> VECTOR_MODE_WITH_PREFIX (VNx, INT, DI, 1, 0);
> ADJUST_NUNITS (MODE, riscv_vector_chunks);
> The riscv_vector_chunks = poly_uint16 (1, 1)
>
> The compilation is failed for the stepped vector test:
> (const_vector:VNx1DI repeat [
>         (const_int 8 [0x8])
>         (const_int 7 [0x7])
>     ])
>
> I understand for stepped vector should always have aleast 2 elements and 
> stepped vector initialization is common
> for VLA (vector-lengthe agnostic) auto-vectorization. It makes sense that 
> report fail for stepped vector of poly_uint16 (1, 1).
>
> machine mode with nunits = poly_uint16 (1, 1) needs to implemented in 
> intrinsics. And I would like to enable RVV auto-vectorization
> with vector mode only nunits is larger than poly_uint16 (2, 2) in RISC-V 
> backend. I think it will not create issue if we define
> vector mode with nunits = poly_uint16 (1, 1). Feel free to correct me or 
> offer me some other better solutions. Thanks!
>
>   
>
> gcc/ChangeLog:
>
>         * simplify-rtx.cc (test_vector_subregs_fore_back): skip test for 
> poly_uint16 (1, 1).
>
> ---
>  gcc/simplify-rtx.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> index 7d09bf7103d..61e0dfa00d0 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -8438,7 +8438,7 @@ test_vector_subregs_fore_back (machine_mode inner_mode)
>    rtx x = builder.build ();
>  
>    test_vector_subregs_modes (x);
> -  if (!nunits.is_constant ())
> +  if (!nunits.is_constant () && known_ne (nunits, poly_uint16 (1, 1)))
>      test_vector_subregs_modes (x, nunits - min_nunits, count);
 
I think instead we should use maybe_gt (nunits, 1), on the basis that
the fore_back tests require vectors that have a minimum of 2 elements.
Something like poly_uint16 (1, 2) would have the same problem as
poly_uint16 (1, 1).  ({1, 2} is an unlikely value, but it's OK in
principle.)
 
This corresponds to the minimum of 3 elements for the stepped tests:
 
  if (GET_MODE_CLASS (mode) == MODE_VECTOR_INT
      && maybe_gt (GET_MODE_NUNITS (mode), 2))
    {
      test_vector_ops_series (mode, scalar_reg);
      test_vector_subregs (mode);
    }
 
Thanks,
Richard