RFR: 8300197: Freeze/thaw an interpreter frame using a single copy_to_chunk() call

[Richard Reingruber]

On Fri, 14 Apr 2023 13:45:12 GMT, Fredrik Bredberg <duke at openjdk.org> wrote:

> On certain architectures (like AARCH64) padding may be inserted between the locals and the rest of the stack frame in order to keep the frame pointer 16-byte-aligned.
> 
> This padding is currently not freezed, instead freezing of a single interpreter stack frame is done using two separate copy_to_chunk() calls (see recurse_freeze_interpreted_frame). Likewise, thawing is done using two separate copy_from_chunk() calls.
> 
> This poses a bit of a problem when trying to relativize stack addresses in interpreter frames ([JDK-8289296](https://bugs.openjdk.org/browse/JDK-8289296)). Since relative offsets may need to be changed during freezing and thawing.
> 
> By both freezing and thawing the padding we remove the need to change any relative offsets in runtime.
> 
> Tested tier1-tier8 on supported platforms, found no new issues. PowerPC and RISC-V was sanity tested using Qemu.

I've tested jdk_loom on ppc64le successfully.
Thanks, Richard.

src/hotspot/cpu/ppc/continuationFreezeThaw_ppc.inline.hpp line 267:

> 265:   intptr_t *sp, *fp;
> 266:   if (FKind::interpreted) {
> 267:     intptr_t offset = *f.addr_at(ijava_idx(locals));

I'd prefer a more specific name. Maybe `locals_offset` or `local0_offset`?

src/hotspot/cpu/ppc/continuationFreezeThaw_ppc.inline.hpp line 510:

> 508:     // we need to set the locals so that the caller of new_stack_frame() can call
> 509:     // ContinuationHelper::InterpretedFrame::frame_bottom
> 510:     // copy relativized locals from the heap frame

Maybe reduce the comment?

    // we need to copy the locals so that the caller of new_stack_frame() can call
    // ContinuationHelper::InterpretedFrame::frame_bottom

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1092:

> 1090:   assert(heap_frame_bottom == heap_frame_top + fsize, "");
> 1091: 
> 1092:   // Some architectures (like AArch64/PPC64/RISC-V) adds padding between the locals and the fixed_frame to keep the fp 16-byte-aligned.

Typo
Suggestion:

  // Some architectures (like AArch64/PPC64/RISC-V) add padding between the locals and the fixed_frame to keep the fp 16-byte-aligned.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 1093:

> 1091: 
> 1092:   // Some architectures (like AArch64/PPC64/RISC-V) adds padding between the locals and the fixed_frame to keep the fp 16-byte-aligned.
> 1093:   // On those architectures we freeze the padding in order to keep the same localized pointer values.

Suggestion:

  // On those architectures we freeze the padding in order to keep the same relative references.

src/hotspot/share/runtime/continuationFreezeThaw.cpp line 2156:

> 2154:   assert(!f.is_heap_frame(), "should not be");
> 2155: 
> 2156:   // Some architectures (like AArch64/PPC64/RISC-V) adds padding between the locals and the fixed_frame to keep the fp 16-byte-aligned.

Suggestion:

  // Some architectures (like AArch64/PPC64/RISC-V) add padding between the locals and the fixed_frame to keep the fp 16-byte-aligned.

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

PR Review: https://git.openjdk.org/jdk/pull/13477#pullrequestreview-1387695274
PR Review Comment: https://git.openjdk.org/jdk/pull/13477#discussion_r1168464062
PR Review Comment: https://git.openjdk.org/jdk/pull/13477#discussion_r1168459316
PR Review Comment: https://git.openjdk.org/jdk/pull/13477#discussion_r1168473037
PR Review Comment: https://git.openjdk.org/jdk/pull/13477#discussion_r1168479719
PR Review Comment: https://git.openjdk.org/jdk/pull/13477#discussion_r1168480534