RFR: 8346706: RISC-V: Add available registers to hs_err [v2]

[Fei Yang]

On Fri, 20 Dec 2024 17:32:53 GMT, Robbin Ehn <rehn at 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