RFR: 8295282: Use Zicboz/cbo.zero to zero-out memory on RISC-V [v11]

Fei Yang fyang at openjdk.org
Mon Oct 24 07:46:42 UTC 2022


On Fri, 21 Oct 2022 15:32:02 GMT, Ludovic Henry <luhenry at openjdk.org> wrote:

>> Similarly to AArch64 DC.ZVA, the RISC-V Zicboz [1] extension provides the cbo.zero [2] instruction that allows to zero out memory a cache-line at a time. This should be faster than storing zeroes 64bits at a time.
>> 
>> [1] https://github.com/riscv/riscv-CMOs
>> [2] https://github.com/riscv/riscv-CMOs/blob/master/cmobase/Zicboz.adoc#insns-cbo_zero
>
> Ludovic Henry has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Disable block zeroing in case CacheLineSize isn't the default value
>  - Disable UseZicboz if CacheLineSize is set by user

Still several more comments. What kind of tests has been carried out for this change?

src/hotspot/cpu/riscv/globals_riscv.hpp line 90:

> 88:           "Minimum size in bytes when block zeroing will be used")               \
> 89:           range(1, max_jint)                                                     \
> 90:   product(intx, CacheLineSize, DEFAULT_CACHE_LINE_SIZE,                          \

Given that 64 bytes is effectively an industry standard, I don't think we need to add "CacheLineSize" as an option at this stage. It will be safer if everything here only depend on "UseZic64b" option which only implies cache block size of 64 bytes. This would also simplify the changes in file: src/hotspot/cpu/riscv/vm_version_riscv.cpp.

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 3985:

> 3983:       sd(zr, Address(ptr, j*8));
> 3984:     }
> 3985:     addi(ptr, ptr, i*8);

Missing space before and after '*' operator here and other places.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 671:

> 669:   //      x29 < MacroAssembler::zero_words_block_size.
> 670: 
> 671:   address generate_zero_blocks() {

Could you please correct the comment at line #669 please? It looks to me that x29 is bigger than MacroAssembler::zero_words_block_size.

src/hotspot/cpu/riscv/stubGenerator_riscv.cpp line 696:

> 694:       __ mv(tmp1, MacroAssembler::zero_words_block_size);
> 695:       __ bind(loop);
> 696:       __ blt(cnt, tmp1, done);

Can we avoid adding this one-extra "blt" instruction into the loop here?

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

Changes requested by fyang (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10718


More information about the hotspot-dev mailing list