RFR: 8367103: RISC-V: store cpu features in a bitmap [v4]

[Fei Yang]

On Wed, 17 Sep 2025 08:18:24 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you help to review this patch?
>> 
>> Currently, the cpu features of riscv are stored in separate RVFeature subclasses.
>> But to support the store/restore CPU features for aot in the future, we need to store the cpu features in a continuous memory.
>> As the riscv extensions are introduced continuously, I think it's better to do it via an simple bitmap at the beginning.
>> Currently, just suppose the non-extension features will not be stored in aot image, so I also split the extension and non-extenion features. When we implement the related aot feature in the short future, we can revisit the way of splitting the features. Currently, just change the storage way of cpu features, lay the foundation for future aot.
>> 
>> Thanks
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix typo

Thanks for the update. Several minor comments remain.

src/hotspot/cpu/riscv/vm_version_riscv.hpp line 238:

> 236:   decl(ext_Zvfh         ,  Zvfh        ,  RV_NO_FLAG_BIT,  30,  true ,  UPDATE_DEFAULT_DEP(UseZvfh, ext_V)) \
> 237:   decl(ext_Zvkn         ,  Zvkn        ,  RV_NO_FLAG_BIT,  31,  true ,  UPDATE_DEFAULT_DEP(UseZvkn, ext_V)) \
> 238:   decl(ext_Zicond       ,  Zicond      ,  RV_NO_FLAG_BIT,  32,  true ,  UPDATE_DEFAULT(UseZicond))          \

I assume the `cpu feature index` field is not subject to change in the future when we have AOT support, right? Then I think the code will be cleaner if we group these features in aphabetic order before that.

src/hotspot/cpu/riscv/vm_version_riscv.hpp line 298:

> 296:     }
> 297: 
> 298:     static int element_index(RVFeatureIndex feature) {

For consistency in naming, should we further rename this into something like `features_bitmap_element_index`?

src/hotspot/cpu/riscv/vm_version_riscv.hpp line 305:

> 303: 
> 304:     static uint64_t bit_mask(RVFeatureIndex feature) {
> 305:       return (1ULL << (feature & features_bitmap_element_mask()));

The two functions `features_bitmap_element_mask` and `bit_mask` look very confusing to me. Is it better to factor out `features_bitmap_element_mask` and rename this `bit_mask` into something like `features_bitmap_element_bit_mask`?

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

PR Review: https://git.openjdk.org/jdk/pull/27152#pullrequestreview-3250899495
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2366944935
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2366936409
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2366934178