RFR: 8368897: RISC-V: Cleanup RV_EXT_FEATURE_FLAGS & RV_NON_EXT_FEATURE_FLAGS [v4]
Fei Yang
fyang at openjdk.org
Tue Oct 7 04:36:02 UTC 2025
On Wed, 1 Oct 2025 10:34:28 GMT, Hamlin Li <mli at openjdk.org> wrote:
>> Hi,
>> Can you help to review the patch?
>>
>> This patch cleans up RV_EXT_FEATURE_FLAGS & RV_NON_EXT_FEATURE_FLAGS, as discussed https://github.com/openjdk/jdk/pull/27152#discussion_r2367109820:
>> * reorder flags in alphabetic order for RV_EXT_FEATURE_FLAGS
>> * move comments close to feature declaration for RV_EXT_FEATURE_FLAGS & RV_NON_EXT_FEATURE_FLAGS
>>
>> We also discussed (https://github.com/openjdk/jdk/pull/27171#discussion_r2387195562) the assert introduced in https://github.com/openjdk/jdk/pull/24094, previously we think this will restrict the flags order in RV_EXT_FEATURE_FLAGS, but I found out that this assert (<del>is not necessary</del>, so we should be able to order flags in RV_EXT_FEATURE_FLAGS in any way we'd like to) does not work as expected, will fix this in another pr.
>>
>> Thanks!
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>
> order RV_NON_EXT_FEATURE_FLAGS
src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 5877:
> 5875: // you want to use it elsewhere, note that cnt must be >= zicboz_block_size.
> 5876: void MacroAssembler::zero_dcache_blocks(Register base, Register cnt, Register tmp1, Register tmp2) {
> 5877: int zicboz_block_size = VM_Version::non_ext_ZicbozBlockSize.value();
Hi, I am a bit concerned that the names might be too long. Say you have something like `VM_Version::non_ext_zicboz_block_size`.
How about introducing some extra namespaces if we want to distinguish these two groups (extensions and non-extensions)?
Then we could have something like `VM_Version::non_ext::zicboz_block_size`, `VM_Version::ext::rvi`, `VM_Version::ext::rvm`. What do you think?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27562#discussion_r2409313398
More information about the hotspot-dev
mailing list