From 2d364f75bd944ecbe38251dfa0f328e7b06b529c Mon Sep 17 00:00:00 2001 From: Johnnie Birch <45402135+jlb6740@users.noreply.github.com> Date: Sat, 6 Jun 2020 16:20:08 -0700 Subject: [PATCH] Remove xmm_r_r inst data structure and cases after related refactoring Removes unneeded data structure that was holding instructions for xmm based move instructions. These instructions can should be categorized as rm not just r. This change is intended to simplify organization and cases when lowering. --- cranelift/codegen/src/isa/x64/abi.rs | 10 +++-- cranelift/codegen/src/isa/x64/inst/args.rs | 2 +- cranelift/codegen/src/isa/x64/inst/emit.rs | 26 ----------- .../codegen/src/isa/x64/inst/emit_tests.rs | 15 ------- cranelift/codegen/src/isa/x64/inst/mod.rs | 45 ++++--------------- cranelift/codegen/src/isa/x64/lower.rs | 8 +++- 6 files changed, 23 insertions(+), 83 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/abi.rs b/cranelift/codegen/src/isa/x64/abi.rs index 51fbddeacb..0188dc2d4a 100644 --- a/cranelift/codegen/src/isa/x64/abi.rs +++ b/cranelift/codegen/src/isa/x64/abi.rs @@ -280,7 +280,11 @@ impl ABIBody for X64ABIBody { return Inst::mov_r_r(/*is64=*/ true, from_reg.to_reg(), to_reg); } else if from_reg.get_class() == RegClass::V128 { // TODO: How to support Movss. Should is64 always be true? - return Inst::xmm_r_r(SseOpcode::Movsd, from_reg.to_reg(), to_reg); + return Inst::xmm_mov_rm_r( + SseOpcode::Movsd, + RegMem::reg(from_reg.to_reg()), + to_reg, + ); } unimplemented!("moving from non-int arg to vreg {:?}", from_reg.get_class()); } @@ -316,9 +320,9 @@ impl ABIBody for X64ABIBody { Writable::::from_reg(to_reg.to_reg()), )) } else if to_reg.get_class() == RegClass::V128 { - ret.push(Inst::xmm_r_r( + ret.push(Inst::xmm_mov_rm_r( SseOpcode::Movsd, - from_reg.to_reg(), + RegMem::reg(from_reg.to_reg()), Writable::::from_reg(to_reg.to_reg()), )) } else { diff --git a/cranelift/codegen/src/isa/x64/inst/args.rs b/cranelift/codegen/src/isa/x64/inst/args.rs index 6c35b6dcb7..96e34886da 100644 --- a/cranelift/codegen/src/isa/x64/inst/args.rs +++ b/cranelift/codegen/src/isa/x64/inst/args.rs @@ -302,7 +302,7 @@ impl SseOpcode { } } - /// Returns src register operand size for an instruction + /// Returns the src operand size for an instruction pub(crate) fn src_size(&self) -> u8 { match self { SseOpcode::Movd => 4, diff --git a/cranelift/codegen/src/isa/x64/inst/emit.rs b/cranelift/codegen/src/isa/x64/inst/emit.rs index 1c9c039693..6f6cea52fc 100644 --- a/cranelift/codegen/src/isa/x64/inst/emit.rs +++ b/cranelift/codegen/src/isa/x64/inst/emit.rs @@ -1025,32 +1025,6 @@ pub(crate) fn emit(inst: &Inst, sink: &mut MachBuffer) { } } - Inst::XMM_R_R { op, src, dst } => { - let opcode = match op { - SseOpcode::Movss => 0x0F10, - SseOpcode::Movsd => 0x0F10, - SseOpcode::Movd => 0x0F6E, - _ => unimplemented!("XMM_R_R opcode"), - }; - - let prefix = match op { - SseOpcode::Movss => LegacyPrefix::_F3, - SseOpcode::Movsd => LegacyPrefix::_F2, - SseOpcode::Movd => LegacyPrefix::_66, - _ => unimplemented!("XMM_R_R opcode"), - }; - - emit_std_reg_reg( - sink, - prefix, - opcode, - 2, - dst.to_reg(), - *src, - RexFlags::clear_w(), - ); - } - Inst::XMM_MOV_RM_R { op, src: src_e, diff --git a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs index 783bfbea42..94c0b7a8b2 100644 --- a/cranelift/codegen/src/isa/x64/inst/emit_tests.rs +++ b/cranelift/codegen/src/isa/x64/inst/emit_tests.rs @@ -2408,21 +2408,6 @@ fn test_x64_emit() { "movsd %xmm14, %xmm3", )); - // ======================================================== - // XMM_R_R - - insns.push(( - Inst::xmm_r_r(SseOpcode::Movss, xmm3, w_xmm2), - "F30F10D3", - "movss %xmm3, %xmm2", - )); - - insns.push(( - Inst::xmm_r_r(SseOpcode::Movsd, xmm4, w_xmm3), - "F20F10DC", - "movsd %xmm4, %xmm3", - )); - // ======================================================== // Actually run the tests! let flags = settings::Flags::new(settings::builder()); diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs index 0ffc56a83b..e13d0df73d 100644 --- a/cranelift/codegen/src/isa/x64/inst/mod.rs +++ b/cranelift/codegen/src/isa/x64/inst/mod.rs @@ -169,13 +169,6 @@ pub(crate) enum Inst { src: RegMem, dst: Writable, }, - - /// mov (64 32) reg reg - XMM_R_R { - op: SseOpcode, - src: Reg, - dst: Writable, - }, } // Handy constructors for Insts. @@ -226,12 +219,6 @@ impl Inst { Inst::Mov_R_R { is_64, src, dst } } - pub(crate) fn xmm_r_r(op: SseOpcode, src: Reg, dst: Writable) -> Inst { - debug_assert!(src.get_class() == RegClass::V128); - debug_assert!(dst.to_reg().get_class() == RegClass::V128); - Inst::XMM_R_R { op, src, dst } - } - pub(crate) fn xmm_mov_rm_r(op: SseOpcode, src: RegMem, dst: Writable) -> Inst { debug_assert!(dst.to_reg().get_class() == RegClass::V128); Inst::XMM_MOV_RM_R { op, src, dst } @@ -427,12 +414,6 @@ impl ShowWithRRU for Inst { show_ireg_sized(*src, mb_rru, sizeLQ(*is_64)), show_ireg_sized(dst.to_reg(), mb_rru, sizeLQ(*is_64)) ), - Inst::XMM_R_R { op, src, dst } => format!( - "{} {}, {}", - ljustify(op.to_string()), - show_ireg_sized(*src, mb_rru, 8), - show_ireg_sized(dst.to_reg(), mb_rru, 8) - ), Inst::MovZX_M_R { extMode, addr, dst } => { if *extMode == ExtMode::LQ { format!( @@ -574,10 +555,6 @@ fn x64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) { collector.add_use(*src); collector.add_def(*dst); } - Inst::XMM_R_R { op: _, src, dst } => { - collector.add_use(*src); - collector.add_def(*dst); - } Inst::MovZX_M_R { extMode: _, addr, @@ -755,14 +732,6 @@ fn x64_map_regs(inst: &mut Inst, mapper: &RUM) { map_use(mapper, src); map_def(mapper, dst); } - Inst::XMM_R_R { - op: _, - ref mut src, - ref mut dst, - } => { - map_use(mapper, src); - map_def(mapper, dst); - } Inst::MovZX_M_R { extMode: _, ref mut addr, @@ -851,12 +820,16 @@ impl MachInst for Inst { // %reg. match self { Self::Mov_R_R { is_64, src, dst } if *is_64 => Some((*dst, *src)), - Self::XMM_R_R { op, src, dst } + Self::XMM_MOV_RM_R { op, src, dst } if *op == SseOpcode::Movss || *op == SseOpcode::Movsd || *op == SseOpcode::Movaps => { - Some((*dst, *src)) + if let RegMem::Reg { reg } = src { + Some((*dst, *reg)) + } else { + None + } } _ => None, } @@ -893,11 +866,11 @@ impl MachInst for Inst { match rc_dst { RegClass::I64 => Inst::mov_r_r(true, src_reg, dst_reg), RegClass::V128 => match ty { - F32 => Inst::xmm_r_r(SseOpcode::Movss, src_reg, dst_reg), - F64 => Inst::xmm_r_r(SseOpcode::Movsd, src_reg, dst_reg), + F32 => Inst::xmm_mov_rm_r(SseOpcode::Movss, RegMem::reg(src_reg), dst_reg), + F64 => Inst::xmm_mov_rm_r(SseOpcode::Movsd, RegMem::reg(src_reg), dst_reg), _ => panic!("unexpected V128 type in gen_move"), }, - _ => panic!("gen_move(x64): unhandled gen_move"), + _ => panic!("gen_move(x64): unhandled regclass"), } } diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index 4edb232c31..13c9c37a6a 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -189,7 +189,11 @@ fn lower_insn_to_regs<'a>(ctx: Ctx<'a>, inst: IRInst) { if src_reg.get_class() == RegClass::I64 { ctx.emit(Inst::mov_r_r(true, src_reg, retval_reg)); } else if src_reg.get_class() == RegClass::V128 { - ctx.emit(Inst::xmm_r_r(SseOpcode::Movsd, src_reg, retval_reg)); + ctx.emit(Inst::xmm_mov_rm_r( + SseOpcode::Movsd, + RegMem::reg(src_reg), + retval_reg, + )); } } // N.B.: the Ret itself is generated by the ABI. @@ -209,7 +213,7 @@ fn lower_insn_to_regs<'a>(ctx: Ctx<'a>, inst: IRInst) { // TODO Fmax, Fmin. _ => unimplemented!(), }; - ctx.emit(Inst::xmm_r_r(SseOpcode::Movss, lhs, dst)); + ctx.emit(Inst::xmm_mov_rm_r(SseOpcode::Movss, RegMem::reg(lhs), dst)); ctx.emit(Inst::xmm_rm_r(sse_op, RegMem::reg(rhs), dst)); } else { unimplemented!("unimplemented lowering for opcode {:?}", op);