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

Reviewed-by: rrich, pchilanomate, fyang

Author: fbredber

src/hotspot/cpu/aarch64/continuationFreezeThaw_aarch64.inline.hpp

@@ -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(), "");
@@ -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
@@ -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);
@@ -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);)
@@ -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 {
@@ -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

src/hotspot/cpu/arm/continuationFreezeThaw_arm.inline.hpp

@@ -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();
 }

src/hotspot/cpu/ppc/continuationFreezeThaw_ppc.inline.hpp

@@ -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));
@@ -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);
@@ -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 {
@@ -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 {
@@ -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

src/hotspot/cpu/riscv/continuationFreezeThaw_riscv.inline.hpp

@@ -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(), "");
@@ -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
@@ -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);
@@ -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);)
@@ -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 {
@@ -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

src/hotspot/cpu/s390/continuationFreezeThaw_s390.inline.hpp

@@ -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();
 }

src/hotspot/cpu/x86/continuationFreezeThaw_x86.inline.hpp

@@ -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(), "");
@@ -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
@@ -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);
@@ -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

src/hotspot/cpu/zero/continuationFreezeThaw_zero.inline.hpp

@@ -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();
 }