Re: Re: [PATCH 2/3 V2] RISC-V: Enable basic auto-vectorization for RVV

["juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>]

[-- Attachment #1: Type: text/plain, Size: 4678 bytes --]

>> With --param=riscv-autovec-preference=fixed-vlmax, however, the output is
>> reasonable.  BTW please use --param instead of -param in the description to
>> avoid confusion.
>>Now the patches don't explicitly note that they only work for certain marchs,
>>configurations or so but they certainly shouldn't introduce ICEs for
>>unsupported configurations.

Address comments.  And fix that soon. Thank you so much.

>>Are the "fixed-vlmax" vs "scalable" names based on ARM's SVE?  I haven't thought
>>this through but I think I'd prefer "fixed" vs "varying" or more explicitly
>>"fixed vector size" vs "dynamic vector size".  Certainly room for discussion here.
>>What about the -mriscv-vector-bits=... (which would be vlen in v-spec parlance)
>>from your "rvv-next" branch?  Is this orthogonal to the new parameter here? Are you
>>thinking of introducing this as well?

The current compile options are suggested by Kito. They are internal GCC compile option.
I was trying to add -mriscv-vector-bits-...., However, it was objected by LLVM community.
https://github.com/riscv-non-isa/riscv-toolchain-conventions/issues/33 

I think in case of compile options, Kito may give more comments since he is the RISC-V ABI and convention maintainer.
I develop this patch following his order.

Thanks.


juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-04-20 16:58
To: Kito Cheng; juzhe.zhong
CC: gcc-patches; palmer; jeffreyalaw
Subject: Re: [PATCH 2/3 V2] RISC-V: Enable basic auto-vectorization for RVV
> $ riscv64-unknown-linux-gnu-gcc
> --param=riscv-autovec-preference=fixed-vlmax
> gcc/testsuite/gcc.target/riscv/rvv/base/spill-10.c -O2 -march=rv64gcv
> -S
> ../riscv-gnu-toolchain-trunk/riscv-gcc/gcc/testsuite/gcc.target/riscv/rvv/base/spill-10.c:
> In function 'stach_check_alloca_1':
> ../riscv-gnu-toolchain-trunk/riscv-gcc/gcc/testsuite/gcc.target/riscv/rvv/base/spill-10.c:41:1:
> error: insn does not satisfy its constraints:
>    41 | }
>       | ^
> (insn 37 26 40 2 (set (reg:VNx8QI 120 v24 [orig:158 data ] [158])
>         (reg:VNx8QI 10 a0 [ data ]))
> "../riscv-gnu-toolchain-trunk/riscv-gcc/gcc/testsuite/gcc.target/riscv/rvv/base/spill-10.c":28:1
> 727 {*movvnx8qi_whole}
>      (nil))
> during RTL pass: reload
> ../riscv-gnu-toolchain-trunk/riscv-gcc/gcc/testsuite/gcc.target/riscv/rvv/base/spill-10.c:41:1:
> internal compiler error: in extract_constrain_insn, at recog.cc:2692
 
For a slightly adjusted testcase
 
void
foo0 (int32_t *__restrict f, int32_t *__restrict d, int n)
{
  for (int i = 0; i < n; ++i)
    {
      f[i * 2 + 0] = 1;
      f[i * 2 + 1] = 2;
      d[i] = 3;
    }
}
 
compiled with -fno-vect-cost-model --param=riscv-autovec-preference=scalable
I see an ICE:
 
during GIMPLE pass: vect
dump file: foo3.c.172t.vect
foo3.c: In function 'foo0':
foo3.c:4:1: internal compiler error: in exact_div, at poly-int.h:2232
    4 | foo0 (int32_t *__restrict f, int32_t *__restrict d, int n)
      | ^~~~
0x7bb237 poly_int<2u, poly_result<unsigned long, if_nonpoly<int, int, poly_int_traits<int>::is_poly>::type, poly_coeff_pair_traits<unsigned long, if_nonpoly<int, int, poly_int_traits<int>::is_poly>::type>::result_kind>::type> exact_div<2u, unsigned long, int>(poly_int_pod<2u, unsigned long> const&, int)
        ../../gcc/poly-int.h:2232
0x7bbf91 poly_int<2u, poly_result<unsigned long, if_nonpoly<int, int, poly_int_traits<int>::is_poly>::type, poly_coeff_pair_traits<unsigned long, if_nonpoly<int, int, poly_int_traits<int>::is_poly>::type>::result_kind>::type> exact_div<2u, unsigned long, int>(poly_int_pod<2u, unsigned long> const&, int)
        ../../gcc/tree.h:3663
0x7bbf91 can_duplicate_and_interleave_p(vec_info*, unsigned int, tree_node*, unsigned int*, tree_node**, tree_node**)
        ../../gcc/tree-vect-slp.cc:437
[..]
 
With --param=riscv-autovec-preference=fixed-vlmax, however, the output is
reasonable.  BTW please use --param instead of -param in the description to
avoid confusion.
 
Now the patches don't explicitly note that they only work for certain marchs,
configurations or so but they certainly shouldn't introduce ICEs for
unsupported configurations.
 
Are the "fixed-vlmax" vs "scalable" names based on ARM's SVE?  I haven't thought
this through but I think I'd prefer "fixed" vs "varying" or more explicitly
"fixed vector size" vs "dynamic vector size".  Certainly room for discussion here.
What about the -mriscv-vector-bits=... (which would be vlen in v-spec parlance)
from your "rvv-next" branch?  Is this orthogonal to the new parameter here? Are you
thinking of introducing this as well?
 
Regards
Robin