Skip to content

Commit

Permalink
8320280: RISC-V: Avoid passing t0 as temp register to MacroAssembler:…
Browse files Browse the repository at this point in the history 
…:lightweight_lock/unlock

Reviewed-by: fyang, rehn
  • Loading branch information
@zifeihan @RealFYang
zifeihan authored and RealFYang committed Nov 20, 2023
1 parent 60c8d9c commit a6098e4
Show file tree
Hide file tree
Showing 10 changed files with 52 additions and 43 deletions.
7 changes: 4 additions & 3 deletions src/hotspot/cpu/riscv/c1_LIRAssembler_riscv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ int LIR_Assembler::emit_unwind_handler() {
if (LockingMode == LM_MONITOR) {
__ j(*stub->entry());
} else {
__ unlock_object(x15, x14, x10, *stub->entry());
__ unlock_object(x15, x14, x10, x16, *stub->entry());
}
__ bind(*stub->continuation());
}
Expand Down Expand Up @@ -1512,6 +1512,7 @@ void LIR_Assembler::emit_lock(LIR_OpLock* op) {
Register obj = op->obj_opr()->as_register(); // may not be an oop
Register hdr = op->hdr_opr()->as_register();
Register lock = op->lock_opr()->as_register();
Register temp = op->scratch_opr()->as_register();
if (LockingMode == LM_MONITOR) {
if (op->info() != nullptr) {
add_debug_info_for_null_check_here(op->info());
Expand All @@ -1521,13 +1522,13 @@ void LIR_Assembler::emit_lock(LIR_OpLock* op) {
} else if (op->code() == lir_lock) {
assert(BasicLock::displaced_header_offset_in_bytes() == 0, "lock_reg must point to the displaced header");
// add debug info for NullPointerException only if one is possible
int null_check_offset = __ lock_object(hdr, obj, lock, *op->stub()->entry());
int null_check_offset = __ lock_object(hdr, obj, lock, temp, *op->stub()->entry());
if (op->info() != nullptr) {
add_debug_info_for_null_check(null_check_offset, op->info());
}
} else if (op->code() == lir_unlock) {
assert(BasicLock::displaced_header_offset_in_bytes() == 0, "lock_reg must point to the displaced header");
__ unlock_object(hdr, obj, lock, *op->stub()->entry());
__ unlock_object(hdr, obj, lock, temp, *op->stub()->entry());
} else {
Unimplemented();
}
Expand Down
6 changes: 4 additions & 2 deletions src/hotspot/cpu/riscv/c1_LIRGenerator_riscv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ void LIRGenerator::do_MonitorEnter(MonitorEnter* x) {

// "lock" stores the address of the monitor stack slot, so this is not an oop
LIR_Opr lock = new_register(T_INT);
LIR_Opr scratch = new_register(T_INT);

CodeEmitInfo* info_for_exception = nullptr;
if (x->needs_null_check()) {
Expand All @@ -282,7 +283,7 @@ void LIRGenerator::do_MonitorEnter(MonitorEnter* x) {
// this CodeEmitInfo must not have the xhandlers because here the
// object is already locked (xhandlers expect object to be unlocked)
CodeEmitInfo* info = state_for(x, x->state(), true);
monitor_enter(obj.result(), lock, syncTempOpr(), LIR_OprFact::illegalOpr,
monitor_enter(obj.result(), lock, syncTempOpr(), scratch,
x->monitor_no(), info_for_exception, info);
}

Expand All @@ -294,8 +295,9 @@ void LIRGenerator::do_MonitorExit(MonitorExit* x) {

LIR_Opr lock = new_register(T_INT);
LIR_Opr obj_temp = new_register(T_INT);
LIR_Opr scratch = new_register(T_INT);
set_no_result(x);
monitor_exit(obj_temp, lock, syncTempOpr(), LIR_OprFact::illegalOpr, x->monitor_no());
monitor_exit(obj_temp, lock, syncTempOpr(), scratch, x->monitor_no());
}

// neg
Expand Down
32 changes: 16 additions & 16 deletions src/hotspot/cpu/riscv/c1_MacroAssembler_riscv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ void C1_MacroAssembler::float_cmp(bool is_float, int unordered_result,
}
}

int C1_MacroAssembler::lock_object(Register hdr, Register obj, Register disp_hdr, Label& slow_case) {
int C1_MacroAssembler::lock_object(Register hdr, Register obj, Register disp_hdr, Register temp, Label& slow_case) {
const int aligned_mask = BytesPerWord - 1;
const int hdr_offset = oopDesc::mark_offset_in_bytes();
assert_different_registers(hdr, obj, disp_hdr);
assert_different_registers(hdr, obj, disp_hdr, temp, t0, t1);
int null_check_offset = -1;

verify_oop(obj);
Expand All @@ -65,15 +65,15 @@ int C1_MacroAssembler::lock_object(Register hdr, Register obj, Register disp_hdr
if (DiagnoseSyncOnValueBasedClasses != 0) {
load_klass(hdr, obj);
lwu(hdr, Address(hdr, Klass::access_flags_offset()));
test_bit(t0, hdr, exact_log2(JVM_ACC_IS_VALUE_BASED_CLASS));
bnez(t0, slow_case, true /* is_far */);
test_bit(temp, hdr, exact_log2(JVM_ACC_IS_VALUE_BASED_CLASS));
bnez(temp, slow_case, true /* is_far */);
}

// Load object header
ld(hdr, Address(obj, hdr_offset));

if (LockingMode == LM_LIGHTWEIGHT) {
lightweight_lock(obj, hdr, t0, t1, slow_case);
lightweight_lock(obj, hdr, temp, t1, slow_case);
} else if (LockingMode == LM_LEGACY) {
Label done;
// and mark it as unlocked
Expand All @@ -83,8 +83,8 @@ int C1_MacroAssembler::lock_object(Register hdr, Register obj, Register disp_hdr
// test if object header is still the same (i.e. unlocked), and if so, store the
// displaced header address in the object header - if it is not the same, get the
// object header instead
la(t1, Address(obj, hdr_offset));
cmpxchgptr(hdr, disp_hdr, t1, t0, done, /*fallthough*/nullptr);
la(temp, Address(obj, hdr_offset));
cmpxchgptr(hdr, disp_hdr, temp, t1, done, /*fallthough*/nullptr);
// if the object header was the same, we're done
// if the object header was not the same, it is now in the hdr register
// => test if it is a stack pointer into the same stack (recursive locking), i.e.:
Expand All @@ -100,8 +100,8 @@ int C1_MacroAssembler::lock_object(Register hdr, Register obj, Register disp_hdr
// assuming both the stack pointer and page_size have their least
// significant 2 bits cleared and page_size is a power of 2
sub(hdr, hdr, sp);
mv(t0, aligned_mask - (int)os::vm_page_size());
andr(hdr, hdr, t0);
mv(temp, aligned_mask - (int)os::vm_page_size());
andr(hdr, hdr, temp);
// for recursive locking, the result is zero => save it in the displaced header
// location (null in the displaced hdr location indicates recursive locking)
sd(hdr, Address(disp_hdr, 0));
Expand All @@ -115,10 +115,10 @@ int C1_MacroAssembler::lock_object(Register hdr, Register obj, Register disp_hdr
return null_check_offset;
}

void C1_MacroAssembler::unlock_object(Register hdr, Register obj, Register disp_hdr, Label& slow_case) {
void C1_MacroAssembler::unlock_object(Register hdr, Register obj, Register disp_hdr, Register temp, Label& slow_case) {
const int aligned_mask = BytesPerWord - 1;
const int hdr_offset = oopDesc::mark_offset_in_bytes();
assert(hdr != obj && hdr != disp_hdr && obj != disp_hdr, "registers must be different");
assert_different_registers(hdr, obj, disp_hdr, temp, t0, t1);
Label done;

if (LockingMode != LM_LIGHTWEIGHT) {
Expand All @@ -135,18 +135,18 @@ void C1_MacroAssembler::unlock_object(Register hdr, Register obj, Register disp_

if (LockingMode == LM_LIGHTWEIGHT) {
ld(hdr, Address(obj, oopDesc::mark_offset_in_bytes()));
test_bit(t0, hdr, exact_log2(markWord::monitor_value));
bnez(t0, slow_case, /* is_far */ true);
lightweight_unlock(obj, hdr, t0, t1, slow_case);
test_bit(temp, hdr, exact_log2(markWord::monitor_value));
bnez(temp, slow_case, /* is_far */ true);
lightweight_unlock(obj, hdr, temp, t1, slow_case);
} else if (LockingMode == LM_LEGACY) {
// test if object header is pointing to the displaced header, and if so, restore
// the displaced header in the object - if the object header is not pointing to
// the displaced header, get the object header instead
// if the object header was not pointing to the displaced header,
// we do unlocking via runtime call
if (hdr_offset) {
la(t0, Address(obj, hdr_offset));
cmpxchgptr(disp_hdr, hdr, t0, t1, done, &slow_case);
la(temp, Address(obj, hdr_offset));
cmpxchgptr(disp_hdr, hdr, temp, t1, done, &slow_case);
} else {
cmpxchgptr(disp_hdr, hdr, obj, t1, done, &slow_case);
}
Expand Down
6 changes: 4 additions & 2 deletions src/hotspot/cpu/riscv/c1_MacroAssembler_riscv.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,16 @@ using MacroAssembler::null_check;
// hdr : must be x10, contents destroyed
// obj : must point to the object to lock, contents preserved
// disp_hdr: must point to the displaced header location, contents preserved
// temp : temporary register, must not be scratch register t0 or t1
// returns code offset at which to add null check debug information
int lock_object (Register swap, Register obj, Register disp_hdr, Label& slow_case);
int lock_object(Register swap, Register obj, Register disp_hdr, Register temp, Label& slow_case);

// unlocking
// hdr : contents destroyed
// obj : must point to the object to lock, contents preserved
// disp_hdr: must be x10 & must point to the displaced header location, contents destroyed
void unlock_object(Register swap, Register obj, Register lock, Label& slow_case);
// temp : temporary register, must not be scratch register t0 or t1
void unlock_object(Register swap, Register obj, Register lock, Register temp, Label& slow_case);

void initialize_object(
Register obj, // result: pointer to object after successful allocation
Expand Down
14 changes: 7 additions & 7 deletions src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@

#define BIND(label) bind(label); BLOCK_COMMENT(#label ":")

void C2_MacroAssembler::fast_lock(Register objectReg, Register boxReg, Register tmp1Reg,
Register tmp2Reg) {
void C2_MacroAssembler::fast_lock(Register objectReg, Register boxReg,
Register tmp1Reg, Register tmp2Reg, Register tmp3Reg) {
// Use cr register to indicate the fast_lock result: zero for success; non-zero for failure.
Register flag = t1;
Register oop = objectReg;
Expand All @@ -55,7 +55,7 @@ void C2_MacroAssembler::fast_lock(Register objectReg, Register boxReg, Register
Label object_has_monitor;
Label count, no_count;

assert_different_registers(oop, box, tmp, disp_hdr, t0);
assert_different_registers(oop, box, tmp, disp_hdr, flag, tmp3Reg, t0);

// Load markWord from object into displaced_header.
ld(disp_hdr, Address(oop, oopDesc::mark_offset_in_bytes()));
Expand Down Expand Up @@ -109,7 +109,7 @@ void C2_MacroAssembler::fast_lock(Register objectReg, Register boxReg, Register
} else {
assert(LockingMode == LM_LIGHTWEIGHT, "");
Label slow;
lightweight_lock(oop, disp_hdr, tmp, t0, slow);
lightweight_lock(oop, disp_hdr, tmp, tmp3Reg, slow);

// Indicate success on completion.
mv(flag, zr);
Expand Down Expand Up @@ -157,8 +157,8 @@ void C2_MacroAssembler::fast_lock(Register objectReg, Register boxReg, Register
bind(no_count);
}

void C2_MacroAssembler::fast_unlock(Register objectReg, Register boxReg, Register tmp1Reg,
Register tmp2Reg) {
void C2_MacroAssembler::fast_unlock(Register objectReg, Register boxReg,
Register tmp1Reg, Register tmp2Reg) {
// Use cr register to indicate the fast_unlock result: zero for success; non-zero for failure.
Register flag = t1;
Register oop = objectReg;
Expand All @@ -169,7 +169,7 @@ void C2_MacroAssembler::fast_unlock(Register objectReg, Register boxReg, Registe
Label object_has_monitor;
Label count, no_count;

assert_different_registers(oop, box, tmp, disp_hdr, flag);
assert_different_registers(oop, box, tmp, disp_hdr, flag, t0);

if (LockingMode == LM_LEGACY) {
// Find the lock address and load the displaced header from the stack.
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
public:
// Code used by cmpFastLock and cmpFastUnlock mach instructions in .ad file.
// See full description in macroAssembler_riscv.cpp.
void fast_lock(Register object, Register box, Register tmp1, Register tmp2);
void fast_lock(Register object, Register box, Register tmp1, Register tmp2, Register tmp3);
void fast_unlock(Register object, Register box, Register tmp1, Register tmp2);

void string_compare(Register str1, Register str2,
Expand Down
11 changes: 7 additions & 4 deletions src/hotspot/cpu/riscv/interp_masm_riscv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ void InterpreterMacroAssembler::remove_activation(
//
// Kills:
// x10
// c_rarg0, c_rarg1, c_rarg2, c_rarg3, .. (param regs)
// c_rarg0, c_rarg1, c_rarg2, c_rarg3, c_rarg4, c_rarg5, .. (param regs)
// t0, t1 (temp regs)
void InterpreterMacroAssembler::lock_object(Register lock_reg)
{
Expand All @@ -742,6 +742,8 @@ void InterpreterMacroAssembler::lock_object(Register lock_reg)
const Register swap_reg = x10;
const Register tmp = c_rarg2;
const Register obj_reg = c_rarg3; // Will contain the oop
const Register tmp2 = c_rarg4;
const Register tmp3 = c_rarg5;

const int obj_offset = in_bytes(BasicObjectLock::obj_offset());
const int lock_offset = in_bytes(BasicObjectLock::lock_offset());
Expand All @@ -762,7 +764,7 @@ void InterpreterMacroAssembler::lock_object(Register lock_reg)

if (LockingMode == LM_LIGHTWEIGHT) {
ld(tmp, Address(obj_reg, oopDesc::mark_offset_in_bytes()));
lightweight_lock(obj_reg, tmp, t0, t1, slow_case);
lightweight_lock(obj_reg, tmp, tmp2, tmp3, slow_case);
j(count);
} else if (LockingMode == LM_LEGACY) {
// Load (object->mark() | 1) into swap_reg
Expand Down Expand Up @@ -826,7 +828,7 @@ void InterpreterMacroAssembler::lock_object(Register lock_reg)
//
// Kills:
// x10
// c_rarg0, c_rarg1, c_rarg2, c_rarg3, ... (param regs)
// c_rarg0, c_rarg1, c_rarg2, c_rarg3, c_rarg4, ... (param regs)
// t0, t1 (temp regs)
void InterpreterMacroAssembler::unlock_object(Register lock_reg)
{
Expand All @@ -840,6 +842,7 @@ void InterpreterMacroAssembler::unlock_object(Register lock_reg)
const Register swap_reg = x10;
const Register header_reg = c_rarg2; // Will contain the old oopMark
const Register obj_reg = c_rarg3; // Will contain the oop
const Register tmp_reg = c_rarg4; // Temporary used by lightweight_unlock

save_bcp(); // Save in case of exception

Expand Down Expand Up @@ -875,7 +878,7 @@ void InterpreterMacroAssembler::unlock_object(Register lock_reg)
ld(header_reg, Address(obj_reg, oopDesc::mark_offset_in_bytes()));
test_bit(t0, header_reg, exact_log2(markWord::monitor_value));
bnez(t0, slow_case);
lightweight_unlock(obj_reg, header_reg, swap_reg, t0, slow_case);
lightweight_unlock(obj_reg, header_reg, swap_reg, tmp_reg, slow_case);
j(count);

bind(slow_case);
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4701,7 +4701,7 @@ void MacroAssembler::test_bit(Register Rd, Register Rs, uint32_t bit_pos) {
// - tmp1, tmp2: temporary registers, will be destroyed
void MacroAssembler::lightweight_lock(Register obj, Register hdr, Register tmp1, Register tmp2, Label& slow) {
assert(LockingMode == LM_LIGHTWEIGHT, "only used with new lightweight locking");
assert_different_registers(obj, hdr, tmp1, tmp2);
assert_different_registers(obj, hdr, tmp1, tmp2, t0);

// Check if we would have space on lock-stack for the object.
lwu(tmp1, Address(xthread, JavaThread::lock_stack_top_offset()));
Expand Down Expand Up @@ -4735,7 +4735,7 @@ void MacroAssembler::lightweight_lock(Register obj, Register hdr, Register tmp1,
// - tmp1, tmp2: temporary registers
void MacroAssembler::lightweight_unlock(Register obj, Register hdr, Register tmp1, Register tmp2, Label& slow) {
assert(LockingMode == LM_LIGHTWEIGHT, "only used with new lightweight locking");
assert_different_registers(obj, hdr, tmp1, tmp2);
assert_different_registers(obj, hdr, tmp1, tmp2, t0);

#ifdef ASSERT
{
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/cpu/riscv/riscv.ad
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10401,16 +10401,16 @@ instruct tlsLoadP(javaThread_RegP dst)

// inlined locking and unlocking
// using t1 as the 'flag' register to bridge the BoolNode producers and consumers
instruct cmpFastLock(rFlagsReg cr, iRegP object, iRegP box, iRegPNoSp tmp1, iRegPNoSp tmp2)
instruct cmpFastLock(rFlagsReg cr, iRegP object, iRegP box, iRegPNoSp tmp1, iRegPNoSp tmp2, iRegPNoSp tmp3)
%{
match(Set cr (FastLock object box));
effect(TEMP tmp1, TEMP tmp2);
effect(TEMP tmp1, TEMP tmp2, TEMP tmp3);

ins_cost(LOAD_COST * 2 + STORE_COST * 3 + ALU_COST * 6 + BRANCH_COST * 3);
format %{ "fastlock $object,$box\t! kills $tmp1,$tmp2, #@cmpFastLock" %}
format %{ "fastlock $object,$box\t! kills $tmp1,$tmp2,$tmp3, #@cmpFastLock" %}

ins_encode %{
__ fast_lock($object$$Register, $box$$Register, $tmp1$$Register, $tmp2$$Register);
__ fast_lock($object$$Register, $box$$Register, $tmp1$$Register, $tmp2$$Register, $tmp3$$Register);
%}

ins_pipe(pipe_serial);
Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/cpu/riscv/sharedRuntime_riscv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1648,6 +1648,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
const Register obj_reg = x9; // Will contain the oop
const Register lock_reg = x30; // Address of compiler lock object (BasicLock)
const Register old_hdr = x30; // value of old header at unlock time
const Register lock_tmp = x31; // Temporary used by lightweight_lock/unlock
const Register tmp = ra;

Label slow_path_lock;
Expand Down Expand Up @@ -1699,7 +1700,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
} else {
assert(LockingMode == LM_LIGHTWEIGHT, "");
__ ld(swap_reg, Address(obj_reg, oopDesc::mark_offset_in_bytes()));
__ lightweight_lock(obj_reg, swap_reg, tmp, t0, slow_path_lock);
__ lightweight_lock(obj_reg, swap_reg, tmp, lock_tmp, slow_path_lock);
}

__ bind(count);
Expand Down Expand Up @@ -1827,7 +1828,7 @@ nmethod* SharedRuntime::generate_native_wrapper(MacroAssembler* masm,
__ ld(old_hdr, Address(obj_reg, oopDesc::mark_offset_in_bytes()));
__ test_bit(t0, old_hdr, exact_log2(markWord::monitor_value));
__ bnez(t0, slow_path_unlock);
__ lightweight_unlock(obj_reg, old_hdr, swap_reg, t0, slow_path_unlock);
__ lightweight_unlock(obj_reg, old_hdr, swap_reg, lock_tmp, slow_path_unlock);
__ decrement(Address(xthread, JavaThread::held_monitor_count_offset()));
}

Expand Down

3 comments on commit a6098e4

@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

@zifeihan
Copy link
Member Author

Choose a reason for hiding this comment

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

/backport jdk21u

@openjdk
Copy link

@openjdk openjdk bot commented on a6098e4 Nov 24, 2023

Choose a reason for hiding this comment

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

@zifeihan the backport was successfully created on the branch zifeihan-backport-a6098e43 in my personal fork of openjdk/jdk21u. To create a pull request with this backport targeting openjdk/jdk21u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit a6098e43 from the openjdk/jdk repository.

The commit being backported was authored by Gui Cao on 20 Nov 2023 and was reviewed by Fei Yang and Robbin Ehn.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21u:

$ git fetch https://github.com/openjdk-bots/jdk21u.git zifeihan-backport-a6098e43:zifeihan-backport-a6098e43
$ git checkout zifeihan-backport-a6098e43
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk21u.git zifeihan-backport-a6098e43

Please sign in to comment.