Skip to content

Commit

Permalink
8290280: riscv: Clean up stack and register handling in interpreter
Browse files Browse the repository at this point in the history
Reviewed-by: fyang
  • Loading branch information
feilongjiang authored and RealFYang committed Jul 18, 2022
1 parent 522b657 commit 4dd236b
Show file tree
Hide file tree
Showing 11 changed files with 109 additions and 78 deletions.
15 changes: 14 additions & 1 deletion src/hotspot/cpu/riscv/abstractInterpreter_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ int AbstractInterpreter::size_activation(int max_stack,
// for the callee's params we only need to account for the extra
// locals.
int size = overhead +
(callee_locals - callee_params) +
(callee_locals - callee_params) * Interpreter::stackElementWords +
monitors * frame::interpreter_frame_monitor_size() +
// On the top frame, at all times SP <= ESP, and SP is
// 16-aligned. We ensure this by adjusting SP on method
Expand Down Expand Up @@ -160,6 +160,19 @@ void AbstractInterpreter::layout_activation(Method* method,
popframe_extra_args;
interpreter_frame->interpreter_frame_set_last_sp(last_sp);

// We have to add extra reserved slots to max_stack. There are 3 users of the extra slots,
// none of which are at the same time, so we just need to make sure there is enough room
// for the biggest user:
// -reserved slot for excepton handler
// -reserved slot for JSR292. Method::extra_stack_entries() is the size.
// -reserved slot for TraceBytecodes
int max_stack = method->constMethod()->max_stack() + MAX2(3, Method::extra_stack_entries());
intptr_t* extended_sp = (intptr_t*) monbot -
(max_stack * Interpreter::stackElementWords) -
popframe_extra_args;
extended_sp = align_down(extended_sp, StackAlignmentInBytes);
interpreter_frame->interpreter_frame_set_extended_sp(extended_sp);

// All frames but the initial (oldest) interpreter frame we fill in have
// a value for sender_sp that allows walking the stack but isn't
// truly correct. Correct the value here.
Expand Down
4 changes: 3 additions & 1 deletion src/hotspot/cpu/riscv/assembler_riscv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ REGISTER_DECLARATION(Register, xthread, x23);
REGISTER_DECLARATION(Register, xbcp, x22);
// Dispatch table base
REGISTER_DECLARATION(Register, xdispatch, x21);
// Java stack pointer
// Java expression stack pointer
REGISTER_DECLARATION(Register, esp, x20);
// Sender's SP while in interpreter
REGISTER_DECLARATION(Register, x19_sender_sp, x19);

// temporary register(caller-save registers)
REGISTER_DECLARATION(Register, t0, x5);
Expand Down
5 changes: 5 additions & 0 deletions src/hotspot/cpu/riscv/frame_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,10 @@ void frame::interpreter_frame_set_last_sp(intptr_t* last_sp) {
*((intptr_t**)addr_at(interpreter_frame_last_sp_offset)) = last_sp;
}

void frame::interpreter_frame_set_extended_sp(intptr_t* sp) {
*((intptr_t**)addr_at(interpreter_frame_extended_sp_offset)) = sp;
}

frame frame::sender_for_entry_frame(RegisterMap* map) const {
assert(map != NULL, "map must be set");
// Java frame called from C; skip all C frames and return top C
Expand Down Expand Up @@ -544,6 +548,7 @@ void frame::describe_pd(FrameValues& values, int frame_no) {
DESCRIBE_FP_OFFSET(interpreter_frame_last_sp);
DESCRIBE_FP_OFFSET(interpreter_frame_method);
DESCRIBE_FP_OFFSET(interpreter_frame_mdp);
DESCRIBE_FP_OFFSET(interpreter_frame_extended_sp);
DESCRIBE_FP_OFFSET(interpreter_frame_mirror);
DESCRIBE_FP_OFFSET(interpreter_frame_cache);
DESCRIBE_FP_OFFSET(interpreter_frame_locals);
Expand Down
10 changes: 6 additions & 4 deletions src/hotspot/cpu/riscv/frame_riscv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@
// [constant pool cache ] = cache() cache_offset

// [klass of method ] = mirror() mirror_offset
// [padding ]
// [extended SP ] extended_sp offset

// [methodData ] = mdp() mdx_offset
// [Method ] = method() method_offset

// [last esp ] = last_sp() last_sp_offset
// [old stack pointer ] (sender_sp) sender_sp_offset
// [sender's SP ] (sender_sp) sender_sp_offset

// [old frame pointer ]
// [return pc ]
Expand Down Expand Up @@ -120,8 +120,8 @@
interpreter_frame_last_sp_offset = interpreter_frame_sender_sp_offset - 1,
interpreter_frame_method_offset = interpreter_frame_last_sp_offset - 1,
interpreter_frame_mdp_offset = interpreter_frame_method_offset - 1,
interpreter_frame_padding_offset = interpreter_frame_mdp_offset - 1,
interpreter_frame_mirror_offset = interpreter_frame_padding_offset - 1,
interpreter_frame_extended_sp_offset = interpreter_frame_mdp_offset - 1,
interpreter_frame_mirror_offset = interpreter_frame_extended_sp_offset - 1,
interpreter_frame_cache_offset = interpreter_frame_mirror_offset - 1,
interpreter_frame_locals_offset = interpreter_frame_cache_offset - 1,
interpreter_frame_bcp_offset = interpreter_frame_locals_offset - 1,
Expand Down Expand Up @@ -200,6 +200,8 @@
// expression stack tos if we are nested in a java call
intptr_t* interpreter_frame_last_sp() const;

void interpreter_frame_set_extended_sp(intptr_t* sp);

template <typename RegisterMapT>
static void update_map_with_saved_link(RegisterMapT* map, intptr_t** link_addr);

Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/riscv/interp_masm_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -466,7 +466,7 @@ void InterpreterMacroAssembler::load_double(Address src) {

void InterpreterMacroAssembler::prepare_to_jump_from_interpreted() {
// set sender sp
mv(x30, sp);
mv(x19_sender_sp, sp);
// record last_sp
sd(esp, Address(fp, frame::interpreter_frame_last_sp_offset * wordSize));
}
Expand Down
24 changes: 24 additions & 0 deletions src/hotspot/cpu/riscv/interp_masm_riscv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,30 @@ class InterpreterMacroAssembler: public MacroAssembler {
ld(xcpool, Address(fp, frame::interpreter_frame_cache_offset * wordSize));
}

void restore_sp_after_call() {
Label L;
ld(t0, Address(fp, frame::interpreter_frame_extended_sp_offset * wordSize));
#ifdef ASSERT
bnez(t0, L);
stop("SP is null");
#endif
bind(L);
mv(sp, t0);
}

void check_extended_sp(const char* msg = "check extended SP") {
#ifdef ASSERT
Label L;
ld(t0, Address(fp, frame::interpreter_frame_extended_sp_offset * wordSize));
beq(sp, t0, L);
stop(msg);
bind(L);
#endif
}

#define check_extended_sp() \
check_extended_sp("SP does not match extended SP in frame at " __FILE__ ":" XSTR(__LINE__))

void get_dispatch();

// Helpers for runtime call arguments/results
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/cpu/riscv/methodHandles_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ address MethodHandles::generate_method_handle_interpreter_entry(MacroAssembler*
return NULL;
}

// x30: sender SP (must preserve; see prepare_to_jump_from_interpreted)
// x19_sender_sp: sender SP (must preserve; see prepare_to_jump_from_interpreted)
// xmethod: Method*
// x13: argument locator (parameter slot count, added to sp)
// x11: used as temp to hold mh or receiver
Expand Down Expand Up @@ -274,7 +274,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
// temps used in this code are not used in *either* compiled or interpreted calling sequences
Register temp1 = x7;
Register temp2 = x28;
Register temp3 = x29; // x30 is live by this point: it contains the sender SP
Register temp3 = x29;
if (for_compiler_entry) {
assert(receiver_reg == (iid == vmIntrinsics::_linkToStatic ? noreg : j_rarg0), "only valid assignment");
assert_different_registers(temp1, j_rarg0, j_rarg1, j_rarg2, j_rarg3, j_rarg4, j_rarg5, j_rarg6, j_rarg7);
Expand Down Expand Up @@ -345,7 +345,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
// Live registers at this point:
// member_reg - MemberName that was the trailing argument
// temp1_recv_klass - klass of stacked receiver, if needed
// x30 - interpreter linkage (if interpreted)
// x19 - interpreter linkage (if interpreted)
// x11 ... x10 - compiler arguments (if compiled)

Label L_incompatible_class_change_error;
Expand Down Expand Up @@ -430,7 +430,7 @@ void MethodHandles::generate_method_handle_dispatch(MacroAssembler* _masm,
break;
}

// live at this point: xmethod, x30 (if interpreted)
// live at this point: xmethod, x19_sender_sp (if interpreted)

// After figuring out which concrete method to call, jump into it.
// Note that this works in the interpreter with no data motion.
Expand Down
7 changes: 6 additions & 1 deletion src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ static void gen_c2i_adapter(MacroAssembler *masm,

int extraspace = total_args_passed * Interpreter::stackElementSize;

__ mv(x30, sp);
__ mv(x19_sender_sp, sp);

// stack is aligned, keep it that way
extraspace = align_up(extraspace, 2 * wordSize);
Expand Down Expand Up @@ -498,6 +498,11 @@ void SharedRuntime::gen_i2c_adapter(MacroAssembler *masm,
int comp_args_on_stack,
const BasicType *sig_bt,
const VMRegPair *regs) {
// Note: x19_sender_sp contains the senderSP on entry. We must
// preserve it since we may do a i2c -> c2i transition if we lose a
// race where compiled code goes non-entrant while we get args
// ready.

// Cut-out for having no stack args.
int comp_words_on_stack = align_up(comp_args_on_stack * VMRegImpl::stack_slot_size, wordSize) >> LogBytesPerWord;
if (comp_args_on_stack != 0) {
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/cpu/riscv/stubGenerator_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,9 @@ class StubGenerator: public StubCodeGenerator {

// call Java entry -- passing methdoOop, and current sp
// xmethod: Method*
// x30: sender sp
// x19_sender_sp: sender sp
BLOCK_COMMENT("call Java function");
__ mv(x30, sp);
__ mv(x19_sender_sp, sp);
__ jalr(c_rarg4);

// save current address for use by exception handling code
Expand Down

1 comment on commit 4dd236b

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.