RFR: 8286301: Port JEP 425 to RISC-V [v4]
Richard Reingruber
rrich at openjdk.org
Fri Nov 4 09:41:37 UTC 2022
On Fri, 4 Nov 2022 09:20:52 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> The value for `fsize` as it is calculated is correct on PPC64 but only because errors compensate. The calculation makes assumptions about the position of fp which is not specified.
>>
>> This is the assumed layout:
>>
>>
>> : :
>> : :
>> | |
>> |-----------------|
>> | |
>> | locals array |
>> | |<- callers_SP
>> ===================
>> | |
>> | metadata at bottom |
>> | |<- FP
>> |-----------------|
>> | |
>> | |
>> | |
>> | |
>> | |<- SP
>> ===================
>>
>>
>> Here the metadata lies outside [FP, SP] and needs to be added to FP-SP just like the size of the locals array.
>>
>> This coincidentally matches the layout on PPC64:
>>
>>
>> : :
>> : :
>> | |
>> |-----------------|
>> | |
>> | locals array |
>> | |
>> |-----------------|
>> | |
>> | metadata at top |
>> | |<- FP / callers_SP
>> ===================
>> | |
>> | |
>> | |
>> | |
>> |-----------------|
>> | |
>> | metadata at top |
>> | |<- SP
>> ===================
>>
>>
>>
>> I assume the layout on Risc-V is like this:
>>
>>
>> : :
>> : :
>> | |
>> |-----------------|
>> | |
>> | locals array |
>> | |<- FP / callers_SP
>> ===================
>> | |
>> | metadata at bottom |
>> | |
>> |-----------------|
>> | |
>> | |
>> | |
>> | |
>> | |<- SP
>> ===================
>>
>> Here the metadata is included in [FP, SP]
>>
>>
>> We could change the line to
>>
>> ```c++
>> const int fsize = ContinuationHelper::InterpretedFrame::callers_sp(f) + frame::metadata_words_at_top + locals - stack_frame_top;
>>
>>
>> with a platform dependent implementation of `callers_sp()`
>
> @reinrich : Yes, you are right about the frame layout of RISC-V. And your proposed solution looks reasonable to me. Since that will need testing accross all platforms, maybe we can do that as a separate PR later on after this is merged. The RISCV testing platform is relatively slow and this PR has been tested for a long time. Thanks.
@RealFYang I'm ok with leaving this for later. The change is actually rather small. I've added it to the PPC64 port with https://github.com/openjdk/jdk/pull/10961/commits/0d12b0577d8314ed1d571d9cac65fa8866939180
Quick tests succeeded on X86, AARCH64 and PPC64.
-------------
PR: https://git.openjdk.org/jdk/pull/10917
More information about the hotspot-dev
mailing list