RFR: 8286301: Port JEP 425 to RISC-V [v4]

Fei Yang fyang at openjdk.org
Fri Nov 4 09:24:31 UTC 2022


On Fri, 4 Nov 2022 08:53:35 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> Hi, I went through the PPC64 changes to shared code and I think for RISC-V we should define metadata_words_at_top and metadata_words_at_bottom to 0 and 2 (which equals metadata_words as defined by this PR) respectively. The PPC64 changes will help eliminate the RISC-V-specific change made in stackChunkOopDesc::is_usable_in_chunk in file src/hotspot/share/oops/stackChunkOop.inline.hpp. But we might still need a RISC-V platform-dependent change here due to the particularity of the RISC-V frame structure. Thanks.
>
> 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.

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

PR: https://git.openjdk.org/jdk/pull/10917


More information about the hotspot-dev mailing list