From 9b25b06d86263c97babcfbf32d2f232ec6e86d1b Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 13 Apr 2021 12:38:35 -0700 Subject: [PATCH 1/2] x64: store to all scalar sizes Previously, `Inst::store` only understood a subset of the scalar types, which resulted in failures seen in #2826. This change allows `Inst::store` to generate instructions for all scalar widths (`8 | 16 | 32 | 64`) since all of these are supported in the emission code of `Inst::MovRM`. --- cranelift/codegen/src/isa/x64/inst/mod.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/inst/mod.rs b/cranelift/codegen/src/isa/x64/inst/mod.rs index 099c975531..0e8b8d9f17 100644 --- a/cranelift/codegen/src/isa/x64/inst/mod.rs +++ b/cranelift/codegen/src/isa/x64/inst/mod.rs @@ -1126,16 +1126,7 @@ impl Inst { pub(crate) fn store(ty: Type, from_reg: Reg, to_addr: impl Into) -> Inst { let rc = from_reg.get_class(); match rc { - RegClass::I64 => Inst::mov_r_m( - match ty { - types::B1 => OperandSize::Size8, - types::I32 | types::R32 => OperandSize::Size32, - types::I64 | types::R64 => OperandSize::Size64, - _ => unimplemented!("integer store of type: {}", ty), - }, - from_reg, - to_addr, - ), + RegClass::I64 => Inst::mov_r_m(OperandSize::from_ty(ty), from_reg, to_addr), RegClass::V128 => { let opcode = match ty { types::F32 => SseOpcode::Movss, From 6bdef484737906607743c09d02366914f395dcb4 Mon Sep 17 00:00:00 2001 From: Andrew Brown Date: Tue, 13 Apr 2021 13:09:07 -0700 Subject: [PATCH 2/2] x64: refactor to use Inst::store during lowering This re-factoring replaces uses of `Inst::mov_r_m` with `Inst::store` to ensure there is only one code location to troubleshoot when generating store instructions for a specific type. --- cranelift/codegen/src/isa/x64/lower.rs | 25 ++++--------------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/cranelift/codegen/src/isa/x64/lower.rs b/cranelift/codegen/src/isa/x64/lower.rs index dc812a9e8f..26c0b89740 100644 --- a/cranelift/codegen/src/isa/x64/lower.rs +++ b/cranelift/codegen/src/isa/x64/lower.rs @@ -4822,28 +4822,11 @@ fn lower_insn_to_regs>( if elem_ty == types::I128 { let srcs = put_input_in_regs(ctx, inputs[0]); - ctx.emit(Inst::mov_r_m( - OperandSize::Size64, - srcs.regs()[0], - addr.clone(), - )); - ctx.emit(Inst::mov_r_m( - OperandSize::Size64, - srcs.regs()[1], - addr.offset(8), - )); + ctx.emit(Inst::store(types::I64, srcs.regs()[0], addr.clone())); + ctx.emit(Inst::store(types::I64, srcs.regs()[1], addr.offset(8))); } else { let src = put_input_in_reg(ctx, inputs[0]); - - ctx.emit(match elem_ty { - types::F32 => Inst::xmm_mov_r_m(SseOpcode::Movss, src, addr), - types::F64 => Inst::xmm_mov_r_m(SseOpcode::Movsd, src, addr), - _ if elem_ty.is_vector() && elem_ty.bits() == 128 => { - // TODO Specialize for different types: MOVUPD, MOVDQU, etc. - Inst::xmm_mov_r_m(SseOpcode::Movups, src, addr) - } - _ => Inst::mov_r_m(OperandSize::from_ty(elem_ty), src, addr), - }); + ctx.emit(Inst::store(elem_ty, src, addr)); } } @@ -4947,7 +4930,7 @@ fn lower_insn_to_regs>( let ty_access = ctx.input_ty(insn, 0); assert!(is_valid_atomic_transaction_ty(ty_access)); - ctx.emit(Inst::mov_r_m(OperandSize::from_ty(ty_access), data, addr)); + ctx.emit(Inst::store(ty_access, data, addr)); ctx.emit(Inst::Fence { kind: FenceKind::MFence, });