RFR: 8322174: RISC-V: C2 VectorizedHashCode RVV Version [v26]
Fei Yang
fyang at openjdk.org
Fri Aug 15 02:23:17 UTC 2025
On Thu, 14 Aug 2025 18:39:53 GMT, Yuri Gaevsky <duke at openjdk.org> wrote:
>> The patch adds possibility to use RVV instructions for faster vectorizedHashCode calculations on RVV v1.0.0 capable hardware.
>>
>> Testing: hotspot/jtreg/compiler/ under QEMU-8.1 with RVV v1.0.0.
>
> Yuri Gaevsky has updated the pull request incrementally with one additional commit since the last revision:
>
> -more updates per reviewer's suggestions.
Seems fine to me modulo some minor coding style issues. Thanks.
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2009:
> 2007: const int elsize_bytes = arrays_hashcode_elsize(eltype);
> 2008: const int elsize_shift = exact_log2(elsize_bytes);
> 2009: const int MAX_VEC_MASK = ~(ints_in_vec_reg * lmul - 1);
`MAX_VEC_MASK` and `ints_in_vec_reg` looks a bit confusing to me considering vector register grouping. I prefer to rename `ints_in_vec_reg` to `stride`, just like you do for the scalar version. Then we can remove this `MAX_VEC_MASK` and replace it with `~(stride - 1)` where it is used.
const int lmul = 2;
const int stride = MaxVectorSize / sizeof(jint) * lmul;
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2053:
> 2051: vmul_vv(v_src, v_src, v_coeffs);
> 2052: vmadd_vx(v_sum, pow31_highest, v_src);
> 2053: shadd(ary, consumed, ary, t0, elsize_shift);
Can you further move this `shadd` to immediately after `mulw(result, result, pow31_highest);`?
I would like to group updating of both `ary` and `cnt` together.
src/hotspot/cpu/riscv/riscv_v.ad line 4095:
> 4093: TEMP tmp1, TEMP tmp2, TEMP tmp3, KILL cr);
> 4094:
> 4095: format %{ "Array HashCode array[] $ary,$cnt,$result,$basic_type -> $result\t#varrays_hashcode" %}
Suggestion:
format %{ "Array HashCode array[] $ary,$cnt,$result,$basic_type -> $result // KILL all" %}
src/hotspot/cpu/riscv/stubDeclarations_riscv.hpp line 78:
> 76: do_stub(compiler, arrays_hashcode_powers_of_31) \
> 77: do_arch_entry(riscv, compiler, arrays_hashcode_powers_of_31, \
> 78: arrays_hashcode_powers_of_31, arrays_hashcode_powers_of_31) \
Let's keep the trailing `` aligned:
do_stub(compiler, arrays_hashcode_powers_of_31) \
do_arch_entry(riscv, compiler, arrays_hashcode_powers_of_31, \
arrays_hashcode_powers_of_31, arrays_hashcode_powers_of_31) \
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 6587:
> 6585: assert(UseRVV, "sanity");
> 6586: const int ints_in_vec_reg = MaxVectorSize / sizeof(jint);
> 6587: const int lmul = 2;
Suggestion:
const int lmul = 2;
const int stride = MaxVectorSize / sizeof(jint) * lmul;
......
for (int i = stride; i >= 0; i--) {
-------------
PR Review: https://git.openjdk.org/jdk/pull/17413#pullrequestreview-3122586012
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2278068359
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2278065225
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2278061270
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2278050015
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2278077761
More information about the hotspot-compiler-dev
mailing list