RFR: 8369685: RISC-V: refactor code related to RVFeatureValue::enabled [v2]

[Hamlin Li]

On Wed, 15 Oct 2025 03:07:50 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   review
>
> src/hotspot/cpu/riscv/vm_version_riscv.hpp line 160:
> 
>> 158:       RVExtFeatures::current()->clear_feature(_cpu_feature_index);
>> 159:     }
>> 160:     int64_t value() { return enabled(); }
> 
> Seems redundant in functionality with `enabled()` for extension features. I think this `value()` method should only apply to non-extension features.

Same as the above one. In `VM_Version::setup_cpu_available_features()`, we still need it.

> src/hotspot/cpu/riscv/vm_version_riscv.hpp line 165:
> 
>> 163:   class RVNonExtFeatureValue : public RVFeatureValue {
>> 164:     int64_t _value;
>> 165:     static const int64_t DISABLED_VALUE = -1;
> 
> I am not sure but is it better to rename it as `DEFAULT_VALUE`?

will change it.

> src/hotspot/cpu/riscv/vm_version_riscv.hpp line 171:
> 
>> 169:       _value(DISABLED_VALUE) {
>> 170:     }
>> 171:     bool enabled() { return _value != DISABLED_VALUE; }
> 
> Do we really need this `enabled()` method for non-extension features? I think this `enabled()` method should only apply to extension features.

Currently, yes.
In `VM_Version::setup_cpu_available_features()`, we still need it.

> src/hotspot/cpu/riscv/vm_version_riscv.hpp line 172:
> 
>> 170:     }
>> 171:     bool enabled() { return _value != DISABLED_VALUE; }
>> 172:     void enable_feature(int64_t value) { _value = value; }
> 
> Can you add an assertion to make sure that the passed `value` won't equal to the default value -1?

sure.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27771#discussion_r2431641458
PR Review Comment: https://git.openjdk.org/jdk/pull/27771#discussion_r2431639422
PR Review Comment: https://git.openjdk.org/jdk/pull/27771#discussion_r2431640732
PR Review Comment: https://git.openjdk.org/jdk/pull/27771#discussion_r2431639676