Skip to content

Commit da0de3c

Browse files
committed
ZJIT: A64: Fix splitting for large memory displacements
On the ruby side, this fixes a crash for methods with 39 or more parameters. We used to miscomp those entry points due to Insn::Lea picking ADDS which cannot reference SP: # set method params: 40 mov x0, #0xfee8 movk x0, #0xffff, lsl #16 movk x0, #0xffff, lsl #32 movk x0, #0xffff, lsl #48 adds x0, xzr, x0 Have Lea work for all i32 displacements and avoid involving the split pass. Previously, direct use of Insn::Lea directly from the user (as opposed to generated by the split pass for some memory operations) wasn't split, so being able to handle the whole range in arm64_emit() was implicitly required. Also, not going through split reduces register pressure.
1 parent 0aabbbe commit da0de3c

File tree

2 files changed

+68
-39
lines changed

2 files changed

+68
-39
lines changed

test/ruby/test_zjit.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,17 @@ def a(n1,n2,n3,n4,n5,n6,n7,n8) = [n8]
826826
}
827827
end
828828

829+
def test_forty_param_method
830+
# This used to a trigger a miscomp on A64 due
831+
# to a memory displacement larger than 9 bits.
832+
assert_compiles '1', %Q{
833+
def foo(#{'_,' * 39} n40) = n40
834+
835+
foo(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1)
836+
}
837+
end
838+
839+
829840
def test_opt_aref_with
830841
assert_compiles ':ok', %q{
831842
def aref_with(hash) = hash["key"]

zjit/src/backend/arm64/mod.rs

Lines changed: 57 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -219,29 +219,6 @@ impl Assembler
219219
/// have no memory operands.
220220
fn arm64_split(mut self) -> Assembler
221221
{
222-
/// When we're attempting to load a memory address into a register, the
223-
/// displacement must fit into the maximum number of bits for an Op::Add
224-
/// immediate. If it doesn't, we have to load the displacement into a
225-
/// register first.
226-
fn split_lea_operand(asm: &mut Assembler, opnd: Opnd) -> Opnd {
227-
match opnd {
228-
Opnd::Mem(Mem { base, disp, num_bits }) => {
229-
if disp >= 0 && ShiftedImmediate::try_from(disp as u64).is_ok() {
230-
asm.lea(opnd)
231-
} else {
232-
let disp = asm.load(Opnd::Imm(disp.into()));
233-
let reg = match base {
234-
MemBase::Reg(reg_no) => Opnd::Reg(Reg { reg_no, num_bits }),
235-
MemBase::VReg(idx) => Opnd::VReg { idx, num_bits }
236-
};
237-
238-
asm.add(reg, disp)
239-
}
240-
},
241-
_ => unreachable!("Op::Lea only accepts Opnd::Mem operands.")
242-
}
243-
}
244-
245222
/// When you're storing a register into a memory location or loading a
246223
/// memory location into a register, the displacement from the base
247224
/// register of the memory location must fit into 9 bits. If it doesn't,
@@ -252,7 +229,7 @@ impl Assembler
252229
if mem_disp_fits_bits(mem.disp) {
253230
opnd
254231
} else {
255-
let base = split_lea_operand(asm, opnd);
232+
let base = asm.lea(opnd);
256233
Opnd::mem(64, base, 0)
257234
}
258235
},
@@ -575,7 +552,7 @@ impl Assembler
575552
},
576553
Insn::IncrCounter { mem, value } => {
577554
let counter_addr = match mem {
578-
Opnd::Mem(_) => split_lea_operand(asm, *mem),
555+
Opnd::Mem(_) => asm.lea(*mem),
579556
_ => *mem
580557
};
581558

@@ -1113,22 +1090,24 @@ impl Assembler
11131090
}
11141091
},
11151092
Insn::Lea { opnd, out } => {
1116-
let opnd: A64Opnd = opnd.into();
1093+
let &Opnd::Mem(Mem { num_bits: _, base: MemBase::Reg(base_reg_no), disp }) = opnd else {
1094+
panic!("Unexpected Insn::Lea operand in arm64_emit: {opnd:?}");
1095+
};
1096+
let out: A64Opnd = out.into();
1097+
let base_reg = A64Opnd::Reg(A64Reg { num_bits: 64, reg_no: base_reg_no });
1098+
assert_ne!(31, out.unwrap_reg().reg_no, "Insn::Lea sp, [sp, #imm] not always encodable. Use add/sub instead.");
11171099

1118-
match opnd {
1119-
A64Opnd::Mem(mem) => {
1120-
add(
1121-
cb,
1122-
out.into(),
1123-
A64Opnd::Reg(A64Reg { reg_no: mem.base_reg_no, num_bits: 64 }),
1124-
A64Opnd::new_imm(mem.disp.into())
1125-
);
1126-
},
1127-
_ => {
1128-
panic!("Op::Lea only accepts Opnd::Mem operands.");
1129-
}
1100+
if ShiftedImmediate::try_from(disp.unsigned_abs() as u64).is_ok() {
1101+
// Use ADD/SUB if the displacement fits
1102+
add(cb, out, base_reg, A64Opnd::new_imm(disp.into()));
1103+
} else {
1104+
// Use add_extended() to interpret reg_no=31 as sp
1105+
// since the base register is never the zero register.
1106+
// Careful! Only the first two operands can refer to sp.
1107+
emit_load_value(cb, out, disp as u64);
1108+
add_extended(cb, out, base_reg, out);
11301109
};
1131-
},
1110+
}
11321111
Insn::LeaJumpTarget { out, target, .. } => {
11331112
if let Target::Label(label_idx) = target {
11341113
// Set output to the raw address of the label
@@ -1607,6 +1586,45 @@ mod tests {
16071586
asm.compile_with_num_regs(&mut cb, 0);
16081587
}
16091588

1589+
#[test]
1590+
fn test_emit_lea() {
1591+
let (mut asm, mut cb) = setup_asm();
1592+
1593+
// Test values that exercise various types of immediates.
1594+
// - 9 bit displacement for Load/Store
1595+
// - 12 bit shifted immediate
1596+
// - bit mask immediates
1597+
for displacement in [i32::MAX, 0x10008, 0x1800, 0x208, -0x208, -0x1800, -0x1008, i32::MIN] {
1598+
let mem = Opnd::mem(64, NATIVE_STACK_PTR, displacement);
1599+
asm.lea_into(Opnd::Reg(X0_REG), mem);
1600+
}
1601+
1602+
asm.compile_with_num_regs(&mut cb, 0);
1603+
assert_disasm!(cb, "e07b40b2e063208b000180d22000a0f2e063208b000083d2e063208be0230891e02308d100009dd2e0ffbff2e0ffdff2e0fffff2e063208b00ff9dd2e0ffbff2e0ffdff2e0fffff2e063208be08361b2e063208b", "
1604+
0x0: orr x0, xzr, #0x7fffffff
1605+
0x4: add x0, sp, x0
1606+
0x8: mov x0, #8
1607+
0xc: movk x0, #1, lsl #16
1608+
0x10: add x0, sp, x0
1609+
0x14: mov x0, #0x1800
1610+
0x18: add x0, sp, x0
1611+
0x1c: add x0, sp, #0x208
1612+
0x20: sub x0, sp, #0x208
1613+
0x24: mov x0, #0xe800
1614+
0x28: movk x0, #0xffff, lsl #16
1615+
0x2c: movk x0, #0xffff, lsl #32
1616+
0x30: movk x0, #0xffff, lsl #48
1617+
0x34: add x0, sp, x0
1618+
0x38: mov x0, #0xeff8
1619+
0x3c: movk x0, #0xffff, lsl #16
1620+
0x40: movk x0, #0xffff, lsl #32
1621+
0x44: movk x0, #0xffff, lsl #48
1622+
0x48: add x0, sp, x0
1623+
0x4c: orr x0, xzr, #0xffffffff80000000
1624+
0x50: add x0, sp, x0
1625+
");
1626+
}
1627+
16101628
/*
16111629
#[test]
16121630
fn test_emit_lea_label() {

0 commit comments

Comments
 (0)