On Tue, 22 Apr 2025 16:46:44 GMT, Hamlin Li <m...@openjdk.org> wrote:
>> src/hotspot/share/runtime/abstract_vm_version.cpp line 349:
>>
>>> 347: assert(features_offset <= cpu_info_string_len, "");
>>> 348: if (features_offset < cpu_info_string_len) {
>>> 349: assert(cpu_info_string[features_offset + 0] == ',', "");
>>
>> This assert fails on riscv.
>
> A simple fix could be:
>
> diff --git a/src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp
> b/src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp
> index 484a2a645aa..a785dc65c9e 100644
> --- a/src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp
> +++ b/src/hotspot/os_cpu/linux_riscv/vm_version_linux_riscv.cpp
> @@ -196,25 +196,12 @@ void VM_Version::setup_cpu_available_features() {
>
> _cpu_info_string = os::strdup(buf);
>
> - _features_string = extract_features_string(_cpu_info_string,
> - strnlen(_cpu_info_string,
> sizeof(buf)),
> - features_offset);
> + _features_string = _cpu_info_string;
> }
Alternatively, it's fine for now to completely drop `extract_features_string`
call on linux-riscv (as on some other platforms) and fix it separately. Then
`VectorSupport.getCPUFeatures()` returns empty string. `VectorMathLibrary`
doesn't rely on `CPUFeatures` on RISC-V.
Let me know how you prefer to handle it.
>> src/hotspot/share/runtime/abstract_vm_version.hpp line 61:
>>
>>> 59: static const char* _features_string;
>>> 60:
>>> 61: static const char* _cpu_info_string;
>>
>> Not quite sure the reason to introduce `_cpu_info_string`.
>> Seems to me you could just use _features_string, and remove _cpu_info_string
>> and its related code, e.g. `extract_features_string`. Please check the code
>> in `test/lib/jdk/test/whitebox/cpuinfo/CPUInfo.java`
>
> Mayber in `CPUFeatures`, could use the similar code as `CPUInfo` to split the
> cpu string into cpu features?
The intention is to align `_features_string` with `_features` which enumerates
well-known CPU capabilities JVM manages. As of now, `_features_string` contains
more information, so I introduced `_cpu_info_string` to keep it.
Speaking of `test/lib/jdk/test/whitebox/cpuinfo/CPUInfo.java`, the approach
chosen there may be fine for a test library, but we need a more stable API
between JVM and JDK.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2055073587
PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2055071408
