RFR: 8322174: RISC-V: C2 VectorizedHashCode RVV Version [v25]
Fei Yang
fyang at openjdk.org
Thu Aug 14 13:26:29 UTC 2025
On Wed, 13 Aug 2025 12:02:58 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:
>
> - addressed reviewer's comments/suggestions.
Thanks for the quick update. Some minor comments remain.
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1920:
> 1918: BasicType eltype)
> 1919: {
> 1920: assert(!UseRVV, "sanity");
Although not dierectly related, can you fix indentation issue of switch-case in this function, `C2_MacroAssembler::arrays_hashcode_elsize` and `C2_MacroAssembler::arrays_hashcode_elload`?
We need add two spaces on the left of each case.
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1998:
> 1996: {
> 1997: assert(UseRVV, "sanity");
> 1998: assert(MaxVectorSize >= 16, "sanity");
`MaxVectorSize >= 16` condition has already been ensured on JVM startup in `VM_Version::c2_initialize()`.
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2005:
> 2003: // size when check UseRVV (i.e. MaxVectorSize == VM_Version::_initial_vector_length).
> 2004: // Let's use T_INT as all hashCode calculations eventually deal with ints.
> 2005: const int ints_in_vec_reg = MaxVectorSize/sizeof(jint);
Please leave a space around the `/` operator.
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2010:
> 2008: const int elsize_bytes = arrays_hashcode_elsize(eltype);
> 2009: const int elsize_shift = exact_log2(elsize_bytes);
> 2010: const int MAX_VEC_MASK = ~(ints_in_vec_reg*lmul - 1);
Please leave a space around the `*` operator.
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2024:
> 2022: const Register pow31_highest = tmp1;
> 2023: const Register ary_end = tmp2;
> 2024: const Register consumed = tmp3;
Suggestion:
const Register pow31_highest = tmp1;
const Register ary_end = tmp2;
const Register consumed = tmp3;
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2056:
> 2054: shadd(ary, consumed, ary, t0, elsize_shift);
> 2055: subw(cnt, cnt, consumed);
> 2056: andi(t1, cnt, MAX_VEC_MASK);
Can you move `subw + andi` to immediately before `bnez`?
src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2111:
> 2109: Register src,
> 2110: BasicType eltype) {
> 2111: assert((T_INT == eltype) || (vdst != vtmp), "should be");
Or simply: `assert_different_registers(vdst, vtmp).` ?
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 6586:
> 6584: address generate_arrays_hashcode_powers_of_31() {
> 6585: assert(UseRVV, "sanity");
> 6586: const int ints_in_vec_reg = MaxVectorSize/sizeof(jint);
Please leave a space around the `/` operator.
src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 6591:
> 6589: StubCodeMark mark(this, "StubRoutines", "arrays_hashcode_powers_of_31");
> 6590: address start = __ pc();
> 6591: for (int i = ints_in_vec_reg*lmul; i >= 0; i--) {
Please leave a space around the `*` operator.
-------------
PR Review: https://git.openjdk.org/jdk/pull/17413#pullrequestreview-3115969220
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2275637206
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2275645194
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2276613026
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2276615983
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2276622884
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2276591563
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2273430390
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2276613462
PR Review Comment: https://git.openjdk.org/jdk/pull/17413#discussion_r2276614023
More information about the hotspot-compiler-dev
mailing list