RFR: 8344169: RISC-V: Use more meaningful frame::metadata_words where possible

Hamlin Li mli at openjdk.org
Fri Nov 15 14:52:18 UTC 2024


On Thu, 14 Nov 2024 07:00:55 GMT, Fei Yang <fyang at openjdk.org> wrote:

> Hello, please review this RISC-V specific change which improves code readability.
> 
> Some background to help understand. We have following frame enumerations in file frame_riscv.hpp:
> 
> enum {
>   link_offset           = -2,
>   return_addr_offset    = -1,
>   sender_sp_offset      =  0
> };
> 
> The values are compatible with the platform ABI and are different from other platforms like x64 and aarch64. Especially, `sender_sp_offset` is 0 for RISC-V compared to 2 for x64 and aarch64. As a result, there exists some differences in places where code calculates fp through offseting pointer sp by value `sender_sp_offset`. For RISC-V, we used constant number 2 instead of `sender_sp_offset` as the pointer offset. But the code will be more readable if we use `frame::metadata_words` which has the same value. This change would not affect correctness or functionality in theory.
> 
> Testing on linux-riscv64:
> - [x] hotspot:tier1 (release)
> - [x] hotspot_loom & jdk_loom (release & fastdebug)

Thanks for trying to make it more readable! And sorry for the late review.
Some minor comments below.

src/hotspot/cpu/riscv/frame_riscv.cpp line 156:

> 154: 
> 155:       sender_unextended_sp = sender_sp;
> 156:       sender_pc = (address) *(sender_sp - 1);

We have `-1` here, and below `-2` is replaced as frame::metadata_words, seems it's still bit confusing to me. Similar comments to several other places.
But I'm not quite sure what should be the better way to fix it.

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

PR Review: https://git.openjdk.org/jdk/pull/22096#pullrequestreview-2438809876
PR Review Comment: https://git.openjdk.org/jdk/pull/22096#discussion_r1843940515


More information about the hotspot-dev mailing list