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