RFR: 8291550: RISC-V: jdk uses misaligned memory access when AvoidUnalignedAccess enabled [v7]
Fei Yang
fyang at openjdk.org
Fri May 5 01:25:23 UTC 2023
On Thu, 4 May 2023 16:46:36 GMT, Vladimir Kempik <vkempik at openjdk.org> wrote:
>> Please review this attempt to remove misaligned loads and stores in risc-v specific part of jdk.
>>
>> The patch has two main parts:
>> - opcodes loads/stores is now using put_native_uX/get_native_uX
>> - some code in template interp got changed to prevent misaligned loads
>>
>> perf stat numbers for trp_lam ( misaligned loads) and trp_sam ( misaligned stores) before the patch:
>>
>> 169598 trp_lam
>> 13562 trp_sam
>>
>>
>> after the patch both numbers are zeroes.
>> I can see template interpreter to be ~40 % faster on hifive unmatched ( 1 repetition of renaissance philosophers in -Xint mode), and the same performance ( before and after the patch) on thead rvb-ice ( which supports misaligned stores/loads in hw)
>>
>> tier testing on hw is in progress
>
> Vladimir Kempik has updated the pull request incrementally with one additional commit since the last revision:
>
> Move misaligned lwu into macroAssembler_riscv.cpp
src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1692:
> 1690: }
> 1691:
> 1692: void MacroAssembler::load_word_misaligned(Register dst, Address src, Register tmp, bool is_signed) {
I am afraid that the function name is a bit confusing considering that the well-known global 'wordSize' is 8 on linux-riscv64. But we are actually loading 4 bytes here. How about renaming it into something like "load_int_unaligned"? This will be more consistent in naming convention with existing function like 'load_unsigned_byte' and 'load_unsigned_short'. Also, it's safer to add an assertion to make sure that 'dst' and 'tmp' are different registers.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13645#discussion_r1185635904
More information about the hotspot-dev
mailing list