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

Fei Yang fyang at openjdk.org
Thu Sep 25 01:30:24 UTC 2025


On Wed, 24 Sep 2025 15:54:59 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:
> 
>   undef macros

A few minor cleanup suggestions. Looks good otherwise.

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

> 291: 
> 292:     constexpr static int element_shift_count() {
> 293:       return LogBitsPerLong;

This `element_shift_count` doesn't seem intuitive from its name and could be factored out. See my comments about its callers.

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

> 294:     }
> 295: 
> 296:     static int element_index(RVFeatureIndex feature) {

Can we rename this as `feature_element`? This together with `feature_bit` tell us where the bit lies.

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

> 295: 
> 296:     static int element_index(RVFeatureIndex feature) {
> 297:       int idx = feature >> element_shift_count();

Suggestion: `int idx = feature / BitsPerLong;`

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

> 302:     static uint64_t feature_bit(RVFeatureIndex feature) {
> 303:       constexpr static uint64_t m = (1ULL << element_shift_count()) - 1;
> 304:       return (1ULL << (feature & m));

Suggestion: `return (1ULL << (feature % BitsPerLong));`

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

PR Review: https://git.openjdk.org/jdk/pull/27152#pullrequestreview-3265195893
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2377419105
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2377422343
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2377415221
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2377416959


More information about the hotspot-dev mailing list