On Fri, 20 Dec 2024 17:32:53 GMT, Robbin Ehn <r...@openjdk.org> wrote:
>> Hi please consider.
>>
>> This adds below to hs_err:
>>
>> Floating point state:
>> fcsr=1
>> Floating point registers:
>> f0=0xffffffff44a72000 | 1.84467e+19
>> f1=0xffffffff44a72000 | 1.84467e+19
>> ....
>> f31=0xffffffff44a72000 | 1.84467e+19
>>
>> Vector state:
>> vstart=0x0000000000000000
>> vl=0x0000000000000020
>> vtype=0x0000000000000000
>> vcsr=0x0000000000000000
>> vlenb=0x0000000000000020
>> Vector registers:
>> v0=0x0101010101010101010101010101010101010101010101010101010101010101
>> ....
>> v31=0x0101010101010101010101010101010101010101010101010101010101010101
>>
>>
>> To get vector the headers need to include those structures, hence build
>> files hackery.
>> This means if you compile on a kernel without RVV support the error handler
>> will lack support for it.
>> We don't care about RVV option as carshing in native may still use vector
>> even if the jit do not.
>>
>> I'm doubt full about the printing as double for fp regs, maybe that should
>> be removed.
>>
>> Local testing, running t1 over weekend.
>>
>> Thanks, Robbin
>
> Robbin Ehn has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Review comments
> - Make file fixes
src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp line 369:
> 367: // No other choice but to define it.
> 368: #ifndef RISCV_V_MAGIC
> 369: #define RISCV_V_MAGIC 0x53465457
It might help to have some extra code comment about where it is defined.
src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp line 387:
> 385: uint32_t ext_size = ext->hdr.size;
> 386:
> 387: if (ext_size < (sizeof(struct __riscv_ctx_hdr) +
> sizeof(*v_ext_state))) {
I am not sure, but can we check for equality about the size here? Personally, I
prefer `sizeof(struct __riscv_v_ext_state)` to `sizeof(*v_ext_state)`. I mean
something like this:
if (ext_size == (sizeof(struct __riscv_ctx_hdr) + sizeof(struct
__riscv_v_ext_state))) {
src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp line 392:
> 390: }
> 391:
> 392: ext_size -= (sizeof(struct __riscv_ctx_hdr) + sizeof(*v_ext_state));
Similar here. `sizeof(truct __riscv_v_ext_state)` is perfered.
src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp line 394:
> 392: ext_size -= (sizeof(struct __riscv_ctx_hdr) + sizeof(*v_ext_state));
> 393:
> 394: v_ext_state = (struct __riscv_v_ext_state *)((char *)(ext) +
> sizeof(*ext));
And here. `sizeof(__riscv_extra_ext_header)` is perfered.
src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp line 401:
> 399: st->print_cr("vtype=" INTPTR_FORMAT, v_ext_state->vtype);
> 400: st->print_cr("vcsr=" INTPTR_FORMAT, v_ext_state->vcsr);
> 401: st->print_cr("vlenb=" INTPTR_FORMAT, v_ext_state->vlenb);
Maybe add some formatting here so that the numbers printed are on the same
colume.
src/hotspot/os_cpu/linux_riscv/os_linux_riscv.cpp line 406:
> 404: uint64_t vr_size = v_ext_state->vlenb;
> 405:
> 406: if (ext_size < (32 * vr_size)) {
Similar here. Can we check for equality about the size here?
if (ext_size == (32 * vr_size)) {
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22845#discussion_r1899272570
PR Review Comment: https://git.openjdk.org/jdk/pull/22845#discussion_r1899270700
PR Review Comment: https://git.openjdk.org/jdk/pull/22845#discussion_r1899271032
PR Review Comment: https://git.openjdk.org/jdk/pull/22845#discussion_r1899271210
PR Review Comment: https://git.openjdk.org/jdk/pull/22845#discussion_r1899271884
PR Review Comment: https://git.openjdk.org/jdk/pull/22845#discussion_r1899271555
