Skip to content

Commit

Permalink
8287418: riscv: Fix correctness issue of MacroAssembler::movptr
Browse files Browse the repository at this point in the history
Reviewed-by: fjiang, yadongwang, fyang
  • Loading branch information
zhengxiaolinX authored and RealFYang committed May 30, 2022
1 parent a27ba1a commit 447ae00
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 19 deletions.
14 changes: 7 additions & 7 deletions src/hotspot/cpu/riscv/assembler_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,23 +282,23 @@ void Assembler::movptr_with_offset(Register Rd, address addr, int32_t &offset) {
}
#endif
assert(is_unsigned_imm_in_range(imm64, 47, 0) || (imm64 == (uintptr_t)-1),
"48-bit overflow in address constant");
// Load upper 32 bits
int32_t imm = imm64 >> 16;
"bit 47 overflows in address constant");
// Load upper 31 bits
int32_t imm = imm64 >> 17;
int64_t upper = imm, lower = imm;
lower = (lower << 52) >> 52;
upper -= lower;
upper = (int32_t)upper;
lui(Rd, upper);
addi(Rd, Rd, lower);

// Load the rest 16 bits.
// Load the rest 17 bits.
slli(Rd, Rd, 11);
addi(Rd, Rd, (imm64 >> 5) & 0x7ff);
slli(Rd, Rd, 5);
addi(Rd, Rd, (imm64 >> 6) & 0x7ff);
slli(Rd, Rd, 6);

// This offset will be used by following jalr/ld.
offset = imm64 & 0x1f;
offset = imm64 & 0x3f;
}

void Assembler::movptr(Register Rd, uintptr_t imm64) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ static const struct CheckInsn barrierInsn[] = {
{ 0x000fffff, 0x00028293, "addi t0, t0, imm1 "},
{ 0xffffffff, 0x00b29293, "slli t0, t0, 11 "},
{ 0x000fffff, 0x00028293, "addi t0, t0, imm2 "},
{ 0xffffffff, 0x00529293, "slli t0, t0, 5 "},
{ 0xffffffff, 0x00629293, "slli t0, t0, 6 "},
{ 0x000fffff, 0x000280e7, "jalr ra, imm3(t0) "},
{ 0x00000fff, 0x0000006f, "j skip "}
/* guard: */
Expand Down
18 changes: 9 additions & 9 deletions src/hotspot/cpu/riscv/macroAssembler_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1181,12 +1181,12 @@ static int patch_offset_in_pc_relative(address branch, int64_t offset) {

static int patch_addr_in_movptr(address branch, address target) {
const int MOVPTR_INSTRUCTIONS_NUM = 6; // lui + addi + slli + addi + slli + addi/jalr/load
int32_t lower = ((intptr_t)target << 36) >> 36;
int64_t upper = ((intptr_t)target - lower) >> 28;
Assembler::patch(branch + 0, 31, 12, upper & 0xfffff); // Lui. target[47:28] + target[27] ==> branch[31:12]
Assembler::patch(branch + 4, 31, 20, (lower >> 16) & 0xfff); // Addi. target[27:16] ==> branch[31:20]
Assembler::patch(branch + 12, 31, 20, (lower >> 5) & 0x7ff); // Addi. target[15: 5] ==> branch[31:20]
Assembler::patch(branch + 20, 31, 20, lower & 0x1f); // Addi/Jalr/Load. target[ 4: 0] ==> branch[31:20]
int32_t lower = ((intptr_t)target << 35) >> 35;
int64_t upper = ((intptr_t)target - lower) >> 29;
Assembler::patch(branch + 0, 31, 12, upper & 0xfffff); // Lui. target[48:29] + target[28] ==> branch[31:12]
Assembler::patch(branch + 4, 31, 20, (lower >> 17) & 0xfff); // Addi. target[28:17] ==> branch[31:20]
Assembler::patch(branch + 12, 31, 20, (lower >> 6) & 0x7ff); // Addi. target[16: 6] ==> branch[31:20]
Assembler::patch(branch + 20, 31, 20, lower & 0x3f); // Addi/Jalr/Load. target[ 5: 0] ==> branch[31:20]
return MOVPTR_INSTRUCTIONS_NUM * NativeInstruction::instruction_size;
}

Expand Down Expand Up @@ -1258,9 +1258,9 @@ static long get_offset_of_pc_relative(address insn_addr) {

static address get_target_of_movptr(address insn_addr) {
assert_cond(insn_addr != NULL);
intptr_t target_address = (((int64_t)Assembler::sextract(((unsigned*)insn_addr)[0], 31, 12)) & 0xfffff) << 28; // Lui.
target_address += ((int64_t)Assembler::sextract(((unsigned*)insn_addr)[1], 31, 20)) << 16; // Addi.
target_address += ((int64_t)Assembler::sextract(((unsigned*)insn_addr)[3], 31, 20)) << 5; // Addi.
intptr_t target_address = (((int64_t)Assembler::sextract(((unsigned*)insn_addr)[0], 31, 12)) & 0xfffff) << 29; // Lui.
target_address += ((int64_t)Assembler::sextract(((unsigned*)insn_addr)[1], 31, 20)) << 17; // Addi.
target_address += ((int64_t)Assembler::sextract(((unsigned*)insn_addr)[3], 31, 20)) << 6; // Addi.
target_address += ((int64_t)Assembler::sextract(((unsigned*)insn_addr)[5], 31, 20)); // Addi/Jalr/Load.
return (address) target_address;
}
Expand Down
3 changes: 2 additions & 1 deletion src/hotspot/cpu/riscv/macroAssembler_riscv.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,8 @@ class MacroAssembler: public Assembler {

// Return true if an address is within the 48-bit RISCV64 address space.
bool is_valid_riscv64_address(address addr) {
return ((uintptr_t)addr >> 48) == 0;
// sv48: must have bits 63–48 all equal to bit 47
return ((uintptr_t)addr >> 47) == 0;
}

void ld_constant(Register dest, const Address &const_addr) {
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/cpu/riscv/nativeInst_riscv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ bool NativeInstruction::is_movptr_at(address instr) {
is_addi_at(instr + instruction_size) && // Addi
is_slli_shift_at(instr + instruction_size * 2, 11) && // Slli Rd, Rs, 11
is_addi_at(instr + instruction_size * 3) && // Addi
is_slli_shift_at(instr + instruction_size * 4, 5) && // Slli Rd, Rs, 5
is_slli_shift_at(instr + instruction_size * 4, 6) && // Slli Rd, Rs, 6
(is_addi_at(instr + instruction_size * 5) ||
is_jalr_at(instr + instruction_size * 5) ||
is_load_at(instr + instruction_size * 5)) && // Addi/Jalr/Load
Expand Down

1 comment on commit 447ae00

@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.