RFR: 8339910: RISC-V: crc32 intrinsic with carry-less multiplication [v3]

[Hamlin Li]

On Tue, 3 Dec 2024 12:34:00 GMT, Ludovic Henry <luhenry at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add some background description
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1795:
> 
>> 1793:   vle64_v(v7, buf); addi(buf, buf, STEP);
>> 1794: 
>> 1795:   vmv_v_x(v31, zr);
> 
> Why the `vmv_v_x`? Where is `v31` used here? Same for https://github.com/openjdk/jdk/pull/22475/files#diff-7a5c3ed05b6f3f06ed1c59f5fc2a14ec566a6a5bd1d09606115767daa99115bdR1797

Thanks for have a look!

This is because we only want to put the initial `crc` value into the lowest part of the vector register, so explicitly reset the other parts to zero. Hope this answer your question.

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1805:
> 
>> 1803: 
>> 1804:   Label L_16_bytes_loop;
>> 1805:   j(L_16_bytes_loop);
> 
> Why this unconditional jump which will go to the next instruction?

Because we have a `align(OptoLoopAlignment);` below just before the loop.

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 2048:
> 
>> 2046:   const int64_t single_table_size = 256;
>> 2047:   const int64_t table_num = 8;   // 4 for scalar, 4 for plain vector
>> 2048:   const ExternalAddress table_addr = StubRoutines::crc_table_addr();
> 
> Wouldn't it be easier and clearer to have a dedicated table?

Make sense to me, I'll fix it later.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22475#discussion_r1867736242
PR Review Comment: https://git.openjdk.org/jdk/pull/22475#discussion_r1867736334
PR Review Comment: https://git.openjdk.org/jdk/pull/22475#discussion_r1867736533