machinst x64: fix checked div sequence

- it should mark as clobbering (def) rdx, not modifying it
- the signed-div check requires a temporary to compare against int64_min
This commit is contained in:
Benjamin Bouvier
2020-07-06 18:45:07 +02:00
parent f932bccaf8
commit d9310e8d90
4 changed files with 38 additions and 7 deletions

View File

@@ -199,7 +199,7 @@ impl RegMemImm {
match self { match self {
Self::Reg { reg } => collector.add_use(*reg), Self::Reg { reg } => collector.add_use(*reg),
Self::Mem { addr } => addr.get_regs_as_uses(collector), Self::Mem { addr } => addr.get_regs_as_uses(collector),
Self::Imm { simm32: _ } => {} Self::Imm { .. } => {}
} }
} }
} }

View File

@@ -666,6 +666,7 @@ pub(crate) fn emit(
size, size,
divisor, divisor,
loc, loc,
tmp,
} => { } => {
// Generates the following code sequence: // Generates the following code sequence:
// //
@@ -727,8 +728,18 @@ pub(crate) fn emit(
(Some(do_op), Some(done_label)) (Some(do_op), Some(done_label))
} else { } else {
// Check for integer overflow. // Check for integer overflow.
if *size == 8 {
let tmp = tmp.expect("temporary for i64 sdiv");
let inst = Inst::imm_r(true, 0x8000000000000000, tmp);
inst.emit(sink, flags, state);
let inst = Inst::cmp_rmi_r(8, RegMemImm::reg(tmp.to_reg()), regs::rax());
inst.emit(sink, flags, state);
} else {
let inst = Inst::cmp_rmi_r(*size, RegMemImm::imm(0x80000000), regs::rax()); let inst = Inst::cmp_rmi_r(*size, RegMemImm::imm(0x80000000), regs::rax());
inst.emit(sink, flags, state); inst.emit(sink, flags, state);
}
// If not equal, jump over the trap. // If not equal, jump over the trap.
let inst = Inst::trap_if(CC::Z, TrapCode::IntegerOverflow, *loc); let inst = Inst::trap_if(CC::Z, TrapCode::IntegerOverflow, *loc);

View File

@@ -69,16 +69,20 @@ pub enum Inst {
MulHi { size: u8, signed: bool, rhs: RegMem }, MulHi { size: u8, signed: bool, rhs: RegMem },
/// A synthetic sequence to implement the right inline checks for remainder and division, /// A synthetic sequence to implement the right inline checks for remainder and division,
/// assuming the dividend is in $rax. /// assuming the dividend is in %rax.
/// Puts the result back into $rax if is_div, $rdx if !is_div, to mimic what the div /// Puts the result back into %rax if is_div, %rdx if !is_div, to mimic what the div
/// instruction does. /// instruction does.
/// The generated code sequence is described in the emit's function match arm for this /// The generated code sequence is described in the emit's function match arm for this
/// instruction. /// instruction.
///
/// Note: %rdx is marked as modified by this instruction, to avoid an early clobber problem
/// with the temporary and divisor. Make sure to zero %rdx right before this instruction!
CheckedDivOrRemSeq { CheckedDivOrRemSeq {
is_div: bool, is_div: bool,
is_signed: bool, is_signed: bool,
size: u8, size: u8,
divisor: Reg, divisor: Reg,
tmp: Option<Writable<Reg>>,
loc: SourceLoc, loc: SourceLoc,
}, },
@@ -846,10 +850,16 @@ fn x64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) {
collector.add_def(Writable::from_reg(regs::rdx())); collector.add_def(Writable::from_reg(regs::rdx()));
rhs.get_regs_as_uses(collector); rhs.get_regs_as_uses(collector);
} }
Inst::CheckedDivOrRemSeq { divisor, .. } => { Inst::CheckedDivOrRemSeq { divisor, tmp, .. } => {
// Mark both fixed registers as mods, to avoid an early clobber problem in codegen
// (i.e. the temporary is allocated one of the fixed registers). This requires writing
// the rdx register *before* the instruction, which is not too bad.
collector.add_mod(Writable::from_reg(regs::rax())); collector.add_mod(Writable::from_reg(regs::rax()));
collector.add_mod(Writable::from_reg(regs::rdx())); collector.add_mod(Writable::from_reg(regs::rdx()));
collector.add_use(*divisor); collector.add_use(*divisor);
if let Some(tmp) = tmp {
collector.add_def(*tmp);
}
} }
Inst::SignExtendRaxRdx { .. } => { Inst::SignExtendRaxRdx { .. } => {
collector.add_use(regs::rax()); collector.add_use(regs::rax());
@@ -1038,8 +1048,11 @@ fn x64_map_regs<RUM: RegUsageMapper>(inst: &mut Inst, mapper: &RUM) {
} }
Inst::Div { divisor, .. } => divisor.map_uses(mapper), Inst::Div { divisor, .. } => divisor.map_uses(mapper),
Inst::MulHi { rhs, .. } => rhs.map_uses(mapper), Inst::MulHi { rhs, .. } => rhs.map_uses(mapper),
Inst::CheckedDivOrRemSeq { divisor, .. } => { Inst::CheckedDivOrRemSeq { divisor, tmp, .. } => {
map_use(mapper, divisor); map_use(mapper, divisor);
if let Some(tmp) = tmp {
map_def(mapper, tmp)
}
} }
Inst::SignExtendRaxRdx { .. } => {} Inst::SignExtendRaxRdx { .. } => {}
Inst::XMM_Mov_RM_R { Inst::XMM_Mov_RM_R {

View File

@@ -825,11 +825,18 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
// regalloc is aware of the coalescing opportunity between rax/rdx and the // regalloc is aware of the coalescing opportunity between rax/rdx and the
// destination register. // destination register.
let divisor = input_to_reg(ctx, inputs[1]); let divisor = input_to_reg(ctx, inputs[1]);
let tmp = if op == Opcode::Sdiv && size == 8 {
Some(ctx.alloc_tmp(RegClass::I64, I64))
} else {
None
};
ctx.emit(Inst::imm_r(true, 0, Writable::from_reg(regs::rdx())));
ctx.emit(Inst::CheckedDivOrRemSeq { ctx.emit(Inst::CheckedDivOrRemSeq {
is_div, is_div,
is_signed, is_signed,
size, size,
divisor, divisor,
tmp,
loc: srcloc, loc: srcloc,
}); });
} else { } else {