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

Fei Yang fyang at openjdk.org
Sat Nov 16 03:00:46 UTC 2024


On Fri, 15 Nov 2024 14:47:14 GMT, Hamlin Li <mli 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 need to use 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)
>
> 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.

Thanks for having a look. Yeah, I also witnessed that when making this change and that's also partially the reason why I used constant number 2 instead of `frame::metadata_words` before. But I didn't touch this `-1` in this PR as similar issue is also there for aarch64 and x86 platforms and there doesn't seem to be a proper enum for this purpose. Maybe a separate PR if we want to fix this `-1` for all?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22096#discussion_r1844888709


More information about the hotspot-dev mailing list