Skip to content

Commit 40c5448

Browse files
committed
ZJIT: A64: Fix bad operand swapping in asm.sub(imm, reg)
Previously, my buggy optimization would turn `asm.sub(imm, reg)` into `subs out, reg, imm` since it runs through the addition path which relies on the commutative property. Don't do that because subtraction does not commute. Good thing no one seems to use this form. Also, delete the 2 regs match arm for Add because it's already covered by the fallback arm -- both split_load_operand() and split_shifted_immediate() are no-op when the input is a register. Fixes: 1317377 ("ZJIT: A64: Have add/sub to SP be single-instruction")
1 parent 6bdd996 commit 40c5448

1 file changed

Lines changed: 28 additions & 6 deletions

File tree

zjit/src/backend/arm64/mod.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -415,13 +415,11 @@ impl Assembler
415415
// being used. It is okay not to use their output here.
416416
#[allow(unused_must_use)]
417417
match &mut insn {
418-
Insn::Sub { left, right, out } |
419418
Insn::Add { left, right, out } => {
420419
match (*left, *right) {
421-
(Opnd::Reg(_) | Opnd::VReg { .. }, Opnd::Reg(_) | Opnd::VReg { .. }) => {
422-
merge_three_reg_mov(&live_ranges, &mut iterator, left, right, out);
423-
asm.push_insn(insn);
424-
}
420+
// When one operand is a register, legalize the other operand
421+
// into possibly an immdiate and swap the order if necessary.
422+
// Only rhs can be immediate in A64, but addition is commutative.
425423
(reg_opnd @ (Opnd::Reg(_) | Opnd::VReg { .. }), other_opnd) |
426424
(other_opnd, reg_opnd @ (Opnd::Reg(_) | Opnd::VReg { .. })) => {
427425
*left = reg_opnd;
@@ -434,10 +432,19 @@ impl Assembler
434432
_ => {
435433
*left = split_load_operand(asm, *left);
436434
*right = split_shifted_immediate(asm, *right);
435+
merge_three_reg_mov(&live_ranges, &mut iterator, left, right, out);
437436
asm.push_insn(insn);
438437
}
439438
}
440-
},
439+
}
440+
Insn::Sub { left, right, out } => {
441+
*left = split_load_operand(asm, *left);
442+
*right = split_shifted_immediate(asm, *right);
443+
// Now `right` is either a register or an immediate,
444+
// both can try to merge with a subsequent mov.
445+
merge_three_reg_mov(&live_ranges, &mut iterator, left, left, out);
446+
asm.push_insn(insn);
447+
}
441448
Insn::And { left, right, out } |
442449
Insn::Or { left, right, out } |
443450
Insn::Xor { left, right, out } => {
@@ -1407,6 +1414,21 @@ mod tests {
14071414
");
14081415
}
14091416

1417+
#[test]
1418+
fn sub_imm_reg() {
1419+
let (mut asm, mut cb) = setup_asm();
1420+
1421+
let difference = asm.sub(0x8.into(), Opnd::Reg(X5_REG));
1422+
asm.load_into(Opnd::Reg(X1_REG), difference);
1423+
1424+
asm.compile_with_num_regs(&mut cb, 1);
1425+
assert_disasm!(cb, "ff230091948200b1", "
1426+
0x0: mov x0, #8
1427+
0x4: subs x0, x0, x5
1428+
0x8: mov x1, x0
1429+
");
1430+
}
1431+
14101432
#[test]
14111433
fn test_emit_add() {
14121434
let (mut asm, mut cb) = setup_asm();

0 commit comments

Comments
 (0)