From ce033f2a0ce291cd47dd716e4eaf14d853510d4a Mon Sep 17 00:00:00 2001 From: bjorn3 Date: Sat, 22 Aug 2020 10:44:12 +0200 Subject: [PATCH] x64: Fix udiv and sdiv for 8bit integers --- cranelift/codegen/src/isa/x64/inst/emit.rs | 20 ++++++++---- .../codegen/src/isa/x64/inst/emit_tests.rs | 4 +++ cranelift/codegen/src/isa/x64/inst/mod.rs | 22 +++++++++++-- cranelift/codegen/src/isa/x64/lower.rs | 32 +++++++++++++------ 4 files changed, 59 insertions(+), 19 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/inst/emit.rs b/cranelift/codegen/src/isa/x64/inst/emit.rs index 002a97337d..fb68a8b63c 100644 --- a/cranelift/codegen/src/isa/x64/inst/emit.rs +++ b/cranelift/codegen/src/isa/x64/inst/emit.rs @@ -662,11 +662,12 @@ pub(crate) fn emit( divisor, loc, } => { - let (prefix, rex_flags) = match size { - 2 => (LegacyPrefixes::_66, RexFlags::clear_w()), - 4 => (LegacyPrefixes::None, RexFlags::clear_w()), - 8 => (LegacyPrefixes::None, RexFlags::set_w()), - _ => unreachable!(), + let (opcode, prefix, rex_flags) = match size { + 1 => (0xF6, LegacyPrefixes::None, RexFlags::clear_w()), + 2 => (0xF7, LegacyPrefixes::_66, RexFlags::clear_w()), + 4 => (0xF7, LegacyPrefixes::None, RexFlags::clear_w()), + 8 => (0xF7, LegacyPrefixes::None, RexFlags::set_w()), + _ => unreachable!("{}", size), }; sink.add_trap(*loc, TrapCode::IntegerDivisionByZero); @@ -675,12 +676,12 @@ pub(crate) fn emit( match divisor { RegMem::Reg { reg } => { let src = int_reg_enc(*reg); - emit_std_enc_enc(sink, prefix, 0xF7, 1, subopcode, src, rex_flags) + emit_std_enc_enc(sink, prefix, opcode, 1, subopcode, src, rex_flags) } RegMem::Mem { addr: src } => emit_std_enc_mem( sink, prefix, - 0xF7, + opcode, 1, subopcode, &src.finalize(state), @@ -715,6 +716,11 @@ pub(crate) fn emit( } } + Inst::SignExtendAlAh => { + sink.put1(0x66); + sink.put1(0x98); + } + Inst::SignExtendRaxRdx { size } => { match size { 2 => sink.put1(0x66), diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs index 32bf521203..d4d187765b 100644 --- a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs @@ -1354,6 +1354,10 @@ fn test_x64_emit() { "mul %rdi", )); + // ======================================================== + // cbw + insns.push((Inst::sign_extend_al_to_ah(), "6698", "cbw")); + // ======================================================== // cdq family: SignExtendRaxRdx insns.push((Inst::sign_extend_rax_to_rdx(2), "6699", "cwd")); diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs index 268149a238..682f6e1876 100644 --- a/cranelift/codegen/src/isa/x64/inst/mod.rs +++ b/cranelift/codegen/src/isa/x64/inst/mod.rs @@ -100,6 +100,9 @@ pub enum Inst { loc: SourceLoc, }, + /// Do a sign-extend based on the sign of the value in al into ah: (cbw) + SignExtendAlAh, + /// Do a sign-extend based on the sign of the value in rax into rdx: (cwd cdq cqo) SignExtendRaxRdx { size: u8, // 1, 2, 4 or 8 @@ -574,6 +577,10 @@ impl Inst { } } + pub(crate) fn sign_extend_al_to_ah() -> Inst { + Inst::SignExtendAlAh + } + pub(crate) fn sign_extend_rax_to_rdx(size: u8) -> Inst { debug_assert!(size == 8 || size == 4 || size == 2); Inst::SignExtendRaxRdx { size } @@ -1259,6 +1266,8 @@ impl ShowWithRRU for Inst { show_ireg_sized(divisor.to_reg(), mb_rru, *size), ), + Inst::SignExtendAlAh => "cbw".into(), + Inst::SignExtendRaxRdx { size } => match size { 2 => "cwd", 4 => "cdq", @@ -1687,9 +1696,13 @@ fn x64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) { Inst::Neg { src, .. } => { collector.add_mod(*src); } - Inst::Div { divisor, .. } => { + Inst::Div { size, divisor, .. } => { collector.add_mod(Writable::from_reg(regs::rax())); - collector.add_mod(Writable::from_reg(regs::rdx())); + if *size == 1 { + collector.add_def(Writable::from_reg(regs::rdx())); + } else { + collector.add_mod(Writable::from_reg(regs::rdx())); + } divisor.get_regs_as_uses(collector); } Inst::MulHi { rhs, .. } => { @@ -1708,6 +1721,9 @@ fn x64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) { collector.add_def(*tmp); } } + Inst::SignExtendAlAh => { + collector.add_mod(Writable::from_reg(regs::rax())); + } Inst::SignExtendRaxRdx { .. } => { collector.add_use(regs::rax()); collector.add_def(Writable::from_reg(regs::rdx())); @@ -2012,7 +2028,7 @@ fn x64_map_regs(inst: &mut Inst, mapper: &RUM) { map_def(mapper, tmp) } } - Inst::SignExtendRaxRdx { .. } => {} + Inst::SignExtendAlAh | Inst::SignExtendRaxRdx { .. } => {} Inst::XmmUnaryRmR { ref mut src, ref mut dst, diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index 4c8e5988cb..caa8f6fbb1 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -2381,16 +2381,30 @@ fn lower_insn_to_regs>( let divisor = input_to_reg_mem(ctx, inputs[1]); // Fill in the high parts: - if kind.is_signed() { - // sign-extend the sign-bit of rax into rdx, for signed opcodes. - ctx.emit(Inst::sign_extend_rax_to_rdx(size)); + if input_ty == types::I8 { + if kind.is_signed() { + // sign-extend the sign-bit of al into ah, for signed opcodes. + ctx.emit(Inst::sign_extend_al_to_ah()); + } else { + ctx.emit(Inst::movzx_rm_r( + ExtMode::BL, + RegMem::reg(regs::rax()), + Writable::from_reg(regs::rax()), + /* infallible */ None, + )); + } } else { - // zero for unsigned opcodes. - ctx.emit(Inst::imm_r( - true, /* is_64 */ - 0, - Writable::from_reg(regs::rdx()), - )); + if kind.is_signed() { + // sign-extend the sign-bit of rax into rdx, for signed opcodes. + ctx.emit(Inst::sign_extend_rax_to_rdx(size)); + } else { + // zero for unsigned opcodes. + ctx.emit(Inst::imm_r( + true, /* is_64 */ + 0, + Writable::from_reg(regs::rdx()), + )); + } } // Emit the actual idiv.