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

[Hamlin Li]

On Thu, 25 Sep 2025 01:22:17 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   undef macros
>
> 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.

make sense, will fix.

> 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.

I think it's better to be element_index, please check usage of it,`int idx = element_index(f);`, we are using it to find the index in the _features_bitmap, and there is another method called `element_count`, their name should be related to each other.

> 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;`

You're right, seems there is a bug here.

> 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));`

yes, it's better.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2378243624
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2378244299
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2378242349
PR Review Comment: https://git.openjdk.org/jdk/pull/27152#discussion_r2378243210