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