Skip to content

Commit

Permalink
8300197: Freeze/thaw an interpreter frame using a single copy_to_chun…
Browse files Browse the repository at this point in the history 
…k() call

Reviewed-by: rrich, pchilanomate, fyang
  • Loading branch information
@fbredber @reinrich
fbredber authored and reinrich committed May 2, 2023
1 parent 1532a1b commit a8d16de
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 96 deletions.
27 changes: 8 additions & 19 deletions src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp
View file
Expand Up @@ -84,11 +84,11 @@ frame FreezeBase::new_heap_frame(frame& f, frame& caller) {
if (FKind::interpreted) {
assert((intptr_t*)f.at(frame::interpreter_frame_last_sp_offset) == nullptr
|| f.unextended_sp() == (intptr_t*)f.at(frame::interpreter_frame_last_sp_offset), "");
int locals = f.interpreter_frame_method()->max_locals();
intptr_t locals_offset = *f.addr_at(frame::interpreter_frame_locals_offset);
// If the caller.is_empty(), i.e. we're freezing into an empty chunk, then we set
// the chunk's argsize in finalize_freeze and make room for it above the unextended_sp
bool overlap_caller = caller.is_interpreted_frame() || caller.is_empty();
fp = caller.unextended_sp() - (locals + frame::sender_sp_offset) + (overlap_caller ? ContinuationHelper::InterpretedFrame::stack_argsize(f) : 0);
fp = caller.unextended_sp() - 1 - locals_offset + (overlap_caller ? ContinuationHelper::InterpretedFrame::stack_argsize(f) : 0);
sp = fp - (f.fp() - f.unextended_sp());
assert(sp <= fp, "");
assert(fp <= caller.unextended_sp(), "");
Expand All @@ -97,7 +97,8 @@ frame FreezeBase::new_heap_frame(frame& f, frame& caller) {
assert(_cont.tail()->is_in_chunk(sp), "");

frame hf(sp, sp, fp, f.pc(), nullptr, nullptr, true /* on_heap */);
*hf.addr_at(frame::interpreter_frame_locals_offset) = frame::sender_sp_offset + locals - 1;
// copy relativized locals from the stack frame
*hf.addr_at(frame::interpreter_frame_locals_offset) = locals_offset;
return hf;
} else {
// We need to re-read fp out of the frame because it may be an oop and we might have
Expand Down Expand Up @@ -145,13 +146,11 @@ inline void FreezeBase::relativize_interpreted_frame_metadata(const frame& f, co

// on AARCH64, we may insert padding between the locals and the rest of the frame
// (see TemplateInterpreterGenerator::generate_normal_entry, and AbstractInterpreter::layout_activation)
// so we compute locals "from scratch" rather than relativizing the value in the stack frame, which might include padding,
// since we don't freeze the padding word (see recurse_freeze_interpreted_frame).
// because we freeze the padding word (see recurse_freeze_interpreted_frame) in order to keep the same relativized
// locals value, we don't need to change the locals value here.

// at(frame::interpreter_frame_last_sp_offset) can be NULL at safepoint preempts
*hf.addr_at(frame::interpreter_frame_last_sp_offset) = hf.unextended_sp() - hf.fp();
// This line can be changed into an assert when we have fixed the "frame padding problem", see JDK-8300197
*hf.addr_at(frame::interpreter_frame_locals_offset) = frame::sender_sp_offset + f.interpreter_frame_method()->max_locals() - 1;

relativize_one(vfp, hfp, frame::interpreter_frame_initial_sp_offset); // == block_top == block_bottom
relativize_one(vfp, hfp, frame::interpreter_frame_extended_sp_offset);
Expand Down Expand Up @@ -222,11 +221,9 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
const int locals = hf.interpreter_frame_method()->max_locals();
intptr_t* frame_sp = caller.unextended_sp() - fsize;
intptr_t* fp = frame_sp + (hf.fp() - heap_sp);
int padding = 0;
if ((intptr_t)fp % frame::frame_alignment != 0) {
fp--;
frame_sp--;
padding++;
log_develop_trace(continuations)("Adding internal interpreted frame alignment");
}
DEBUG_ONLY(intptr_t* unextended_sp = fp + *hf.addr_at(frame::interpreter_frame_last_sp_offset);)
Expand All @@ -235,10 +232,8 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
frame f(frame_sp, frame_sp, fp, hf.pc());
// we need to set the locals so that the caller of new_stack_frame() can call
// ContinuationHelper::InterpretedFrame::frame_bottom
intptr_t offset = *hf.addr_at(frame::interpreter_frame_locals_offset);
assert((int)offset == frame::sender_sp_offset + locals - 1, "");
// set relativized locals
*f.addr_at(frame::interpreter_frame_locals_offset) = padding + offset;
// copy relativized locals from the heap frame
*f.addr_at(frame::interpreter_frame_locals_offset) = *hf.addr_at(frame::interpreter_frame_locals_offset);
assert((intptr_t)f.fp() % frame::frame_alignment == 0, "");
return f;
} else {
Expand Down Expand Up @@ -300,10 +295,4 @@ inline void ThawBase::derelativize_interpreted_frame_metadata(const frame& hf, c
derelativize_one(vfp, frame::interpreter_frame_extended_sp_offset);
}

inline void ThawBase::set_interpreter_frame_bottom(const frame& f, intptr_t* bottom) {
// set relativized locals
// this line can be changed into an assert when we have fixed the "frame padding problem", see JDK-8300197
*f.addr_at(frame::interpreter_frame_locals_offset) = (bottom - 1) - f.fp();
}

#endif // CPU_AARCH64_CONTINUATIONFREEZETHAW_AARCH64_INLINE_HPP
4 changes: 0 additions & 4 deletions src/hotspot/cpu/arm/continuationFreezeThaw_arm.inline.hpp
View file
Expand Up @@ -70,10 +70,6 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
return frame();
}

inline void ThawBase::set_interpreter_frame_bottom(const frame& f, intptr_t* bottom) {
Unimplemented();
}

inline void ThawBase::derelativize_interpreted_frame_metadata(const frame& hf, const frame& f) {
Unimplemented();
}
Expand Down
24 changes: 8 additions & 16 deletions src/hotspot/cpu/ppc/continuationFreezeThaw_ppc.inline.hpp
View file
Expand Up @@ -84,9 +84,9 @@ inline void FreezeBase::relativize_interpreted_frame_metadata(const frame& f, co
assert(f.fp() > (intptr_t*)f.interpreter_frame_esp(), "");

// There is alignment padding between vfp and f's locals array in the original
// frame, therefore we cannot use it to relativize the locals pointer.
// This line can be changed into an assert when we have fixed the "frame padding problem", see JDK-8300197
*hf.addr_at(ijava_idx(locals)) = frame::metadata_words + f.interpreter_frame_method()->max_locals() - 1;
// frame, because we freeze the padding (see recurse_freeze_interpreted_frame)
// in order to keep the same relativized locals pointer, we don't need to change it here.

relativize_one(vfp, hfp, ijava_idx(monitors));
relativize_one(vfp, hfp, ijava_idx(esp));
relativize_one(vfp, hfp, ijava_idx(top_frame_sp));
Expand Down Expand Up @@ -264,15 +264,15 @@ frame FreezeBase::new_heap_frame(frame& f, frame& caller) {

intptr_t *sp, *fp;
if (FKind::interpreted) {
int locals = f.interpreter_frame_method()->max_locals();
intptr_t locals_offset = *f.addr_at(ijava_idx(locals));
// If the caller.is_empty(), i.e. we're freezing into an empty chunk, then we set
// the chunk's argsize in finalize_freeze and make room for it above the unextended_sp
// See also comment on StackChunkFrameStream<frame_kind>::interpreter_frame_size()
int overlap =
(caller.is_interpreted_frame() || caller.is_empty())
? ContinuationHelper::InterpretedFrame::stack_argsize(f) + frame::metadata_words_at_top
: 0;
fp = caller.unextended_sp() + overlap - locals - frame::metadata_words_at_top;
fp = caller.unextended_sp() - 1 - locals_offset + overlap;
// esp points one slot below the last argument
intptr_t* x86_64_like_unextended_sp = f.interpreter_frame_esp() + 1 - frame::metadata_words_at_top;
sp = fp - (f.fp() - x86_64_like_unextended_sp);
Expand All @@ -286,7 +286,7 @@ frame FreezeBase::new_heap_frame(frame& f, frame& caller) {

frame hf(sp, sp, fp, f.pc(), nullptr, nullptr, true /* on_heap */);
// frame_top() and frame_bottom() read these before relativize_interpreted_frame_metadata() is called
*hf.addr_at(ijava_idx(locals)) = frame::metadata_words + locals - 1;
*hf.addr_at(ijava_idx(locals)) = locals_offset;
*hf.addr_at(ijava_idx(esp)) = f.interpreter_frame_esp() - f.fp();
return hf;
} else {
Expand Down Expand Up @@ -507,10 +507,8 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
frame f(frame_sp, hf.pc(), frame_sp, fp);
// we need to set the locals so that the caller of new_stack_frame() can call
// ContinuationHelper::InterpretedFrame::frame_bottom
intptr_t offset = *hf.addr_at(ijava_idx(locals)) + padding;
assert((int)offset == hf.interpreter_frame_method()->max_locals() + frame::metadata_words_at_top + padding - 1, "");
// set relativized locals
*f.addr_at(ijava_idx(locals)) = offset;
// copy relativized locals from the heap frame
*f.addr_at(ijava_idx(locals)) = *hf.addr_at(ijava_idx(locals));

return f;
} else {
Expand Down Expand Up @@ -549,12 +547,6 @@ inline void ThawBase::derelativize_interpreted_frame_metadata(const frame& hf, c
derelativize_one(vfp, ijava_idx(top_frame_sp));
}

inline void ThawBase::set_interpreter_frame_bottom(const frame& f, intptr_t* bottom) {
// set relativized locals
// This line can be changed into an assert when we have fixed the "frame padding problem", see JDK-8300197
*f.addr_at(ijava_idx(locals)) = (bottom - 1) - f.fp();
}

inline void ThawBase::patch_pd(frame& f, const frame& caller) {
patch_callee_link(caller, caller.fp());
// Prevent assertion if f gets deoptimized right away before it's fully initialized
Expand Down
26 changes: 7 additions & 19 deletions src/hotspot/cpu/riscv/continuationFreezeThaw_riscv.inline.hpp
View file
Expand Up @@ -83,11 +83,11 @@ template<typename FKind> frame FreezeBase::new_heap_frame(frame& f, frame& calle
if (FKind::interpreted) {
assert((intptr_t*)f.at(frame::interpreter_frame_last_sp_offset) == nullptr
|| f.unextended_sp() == (intptr_t*)f.at(frame::interpreter_frame_last_sp_offset), "");
int locals = f.interpreter_frame_method()->max_locals();
intptr_t locals_offset = *f.addr_at(frame::interpreter_frame_locals_offset);
// If the caller.is_empty(), i.e. we're freezing into an empty chunk, then we set
// the chunk's argsize in finalize_freeze and make room for it above the unextended_sp
bool overlap_caller = caller.is_interpreted_frame() || caller.is_empty();
fp = caller.unextended_sp() - (locals + frame::sender_sp_offset) + (overlap_caller ? ContinuationHelper::InterpretedFrame::stack_argsize(f) : 0);
fp = caller.unextended_sp() - 1 - locals_offset + (overlap_caller ? ContinuationHelper::InterpretedFrame::stack_argsize(f) : 0);
sp = fp - (f.fp() - f.unextended_sp());
assert(sp <= fp, "");
assert(fp <= caller.unextended_sp(), "");
Expand All @@ -96,7 +96,7 @@ template<typename FKind> frame FreezeBase::new_heap_frame(frame& f, frame& calle
assert(_cont.tail()->is_in_chunk(sp), "");

frame hf(sp, sp, fp, f.pc(), nullptr, nullptr, true /* on_heap */);
*hf.addr_at(frame::interpreter_frame_locals_offset) = frame::sender_sp_offset + locals - 1;
*hf.addr_at(frame::interpreter_frame_locals_offset) = locals_offset;
return hf;
} else {
// We need to re-read fp out of the frame because it may be an oop and we might have
Expand Down Expand Up @@ -144,13 +144,11 @@ inline void FreezeBase::relativize_interpreted_frame_metadata(const frame& f, co

// On RISCV, we may insert padding between the locals and the rest of the frame
// (see TemplateInterpreterGenerator::generate_normal_entry, and AbstractInterpreter::layout_activation)
// so we compute locals "from scratch" rather than relativizing the value in the stack frame, which might include padding,
// since we don't freeze the padding word (see recurse_freeze_interpreted_frame).
// because we freeze the padding word (see recurse_freeze_interpreted_frame) in order to keep the same relativized
// locals value, we don't need to change the locals value here.

// at(frame::interpreter_frame_last_sp_offset) can be null at safepoint preempts
*hf.addr_at(frame::interpreter_frame_last_sp_offset) = hf.unextended_sp() - hf.fp();
// this line can be changed into an assert when we have fixed the "frame padding problem", see JDK-8300197
*hf.addr_at(frame::interpreter_frame_locals_offset) = frame::sender_sp_offset + f.interpreter_frame_method()->max_locals() - 1;

relativize_one(vfp, hfp, frame::interpreter_frame_initial_sp_offset); // == block_top == block_bottom
relativize_one(vfp, hfp, frame::interpreter_frame_extended_sp_offset);
Expand Down Expand Up @@ -225,11 +223,9 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
const int locals = hf.interpreter_frame_method()->max_locals();
intptr_t* frame_sp = caller.unextended_sp() - fsize;
intptr_t* fp = frame_sp + (hf.fp() - heap_sp);
int padding = 0;
if ((intptr_t)fp % frame::frame_alignment != 0) {
fp--;
frame_sp--;
padding++;
log_develop_trace(continuations)("Adding internal interpreted frame alignment");
}
DEBUG_ONLY(intptr_t* unextended_sp = fp + *hf.addr_at(frame::interpreter_frame_last_sp_offset);)
Expand All @@ -238,10 +234,8 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
frame f(frame_sp, frame_sp, fp, hf.pc());
// we need to set the locals so that the caller of new_stack_frame() can call
// ContinuationHelper::InterpretedFrame::frame_bottom
intptr_t offset = *hf.addr_at(frame::interpreter_frame_locals_offset);
assert((int)offset == frame::sender_sp_offset + locals - 1, "");
// set relativized locals
*f.addr_at(frame::interpreter_frame_locals_offset) = padding + offset;
// copy relativized locals from the heap frame
*f.addr_at(frame::interpreter_frame_locals_offset) = *hf.addr_at(frame::interpreter_frame_locals_offset);
assert((intptr_t)f.fp() % frame::frame_alignment == 0, "");
return f;
} else {
Expand Down Expand Up @@ -303,10 +297,4 @@ inline void ThawBase::derelativize_interpreted_frame_metadata(const frame& hf, c
derelativize_one(vfp, frame::interpreter_frame_extended_sp_offset);
}

inline void ThawBase::set_interpreter_frame_bottom(const frame& f, intptr_t* bottom) {
// set relativized locals
// This line can be changed into an assert when we have fixed the "frame padding problem", see JDK-8300197
*f.addr_at(frame::interpreter_frame_locals_offset) = (bottom - 1) - f.fp();
}

#endif // CPU_RISCV_CONTINUATIONFREEZETHAW_RISCV_INLINE_HPP
4 changes: 0 additions & 4 deletions src/hotspot/cpu/s390/continuationFreezeThaw_s390.inline.hpp
View file
Expand Up @@ -70,10 +70,6 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
return frame();
}

inline void ThawBase::set_interpreter_frame_bottom(const frame& f, intptr_t* bottom) {
Unimplemented();
}

inline void ThawBase::derelativize_interpreted_frame_metadata(const frame& hf, const frame& f) {
Unimplemented();
}
Expand Down
19 changes: 8 additions & 11 deletions src/hotspot/cpu/x86/continuationFreezeThaw_x86.inline.hpp
View file
Expand Up @@ -81,11 +81,11 @@ frame FreezeBase::new_heap_frame(frame& f, frame& caller) {
if (FKind::interpreted) {
assert((intptr_t*)f.at(frame::interpreter_frame_last_sp_offset) == nullptr
|| f.unextended_sp() == (intptr_t*)f.at(frame::interpreter_frame_last_sp_offset), "");
int locals = f.interpreter_frame_method()->max_locals();
intptr_t locals_offset = *f.addr_at(frame::interpreter_frame_locals_offset);
// If the caller.is_empty(), i.e. we're freezing into an empty chunk, then we set
// the chunk's argsize in finalize_freeze and make room for it above the unextended_sp
bool overlap_caller = caller.is_interpreted_frame() || caller.is_empty();
fp = caller.unextended_sp() - (locals + frame::sender_sp_offset) + (overlap_caller ? ContinuationHelper::InterpretedFrame::stack_argsize(f) : 0);
fp = caller.unextended_sp() - 1 - locals_offset + (overlap_caller ? ContinuationHelper::InterpretedFrame::stack_argsize(f) : 0);
sp = fp - (f.fp() - f.unextended_sp());
assert(sp <= fp, "");
assert(fp <= caller.unextended_sp(), "");
Expand All @@ -94,7 +94,8 @@ frame FreezeBase::new_heap_frame(frame& f, frame& caller) {
assert(_cont.tail()->is_in_chunk(sp), "");

frame hf(sp, sp, fp, f.pc(), nullptr, nullptr, true /* on_heap */);
*hf.addr_at(frame::interpreter_frame_locals_offset) = frame::sender_sp_offset + locals - 1;
// copy relativized locals from the stack frame
*hf.addr_at(frame::interpreter_frame_locals_offset) = locals_offset;
return hf;
} else {
// We need to re-read fp out of the frame because it may be an oop and we might have
Expand Down Expand Up @@ -223,10 +224,10 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
frame f(frame_sp, frame_sp, fp, hf.pc());
// we need to set the locals so that the caller of new_stack_frame() can call
// ContinuationHelper::InterpretedFrame::frame_bottom
intptr_t offset = *hf.addr_at(frame::interpreter_frame_locals_offset);
assert((int)offset == frame::sender_sp_offset + locals - 1, "");
// set relativized locals
*f.addr_at(frame::interpreter_frame_locals_offset) = offset;
intptr_t locals_offset = *hf.addr_at(frame::interpreter_frame_locals_offset);
assert((int)locals_offset == frame::sender_sp_offset + locals - 1, "");
// copy relativized locals from the heap frame
*f.addr_at(frame::interpreter_frame_locals_offset) = locals_offset;
return f;
} else {
int fsize = FKind::size(hf);
Expand Down Expand Up @@ -285,8 +286,4 @@ inline void ThawBase::derelativize_interpreted_frame_metadata(const frame& hf, c
derelativize_one(vfp, frame::interpreter_frame_initial_sp_offset);
}

inline void ThawBase::set_interpreter_frame_bottom(const frame& f, intptr_t* bottom) {
// Nothing to do. Just make sure the relativized locals is already set.
assert((*f.addr_at(frame::interpreter_frame_locals_offset) == (bottom - 1) - f.fp()), "");
}
#endif // CPU_X86_CONTINUATIONFREEZE_THAW_X86_INLINE_HPP
4 changes: 0 additions & 4 deletions src/hotspot/cpu/zero/continuationFreezeThaw_zero.inline.hpp
View file
Expand Up @@ -70,10 +70,6 @@ template<typename FKind> frame ThawBase::new_stack_frame(const frame& hf, frame&
return frame();
}

inline void ThawBase::set_interpreter_frame_bottom(const frame& f, intptr_t* bottom) {
Unimplemented();
}

inline void ThawBase::derelativize_interpreted_frame_metadata(const frame& hf, const frame& f) {
Unimplemented();
}
Expand Down

1 comment on commit a8d16de

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.