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