From 1d5a678124e0f035f7614cafe43066c834a5113b Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Fri, 6 Mar 2020 15:22:53 +0100 Subject: [PATCH] Fixes #1240: Add a new accessor to indicate that an opcode requires spilling all registers; --- cranelift/codegen/meta/src/cdsl/instructions.rs | 10 ++++++++++ cranelift/codegen/meta/src/gen_inst.rs | 7 +++++++ cranelift/codegen/meta/src/isa/x86/instructions.rs | 6 ++++++ cranelift/codegen/src/regalloc/spilling.rs | 5 +---- 4 files changed, 24 insertions(+), 4 deletions(-) diff --git a/cranelift/codegen/meta/src/cdsl/instructions.rs b/cranelift/codegen/meta/src/cdsl/instructions.rs index d8d9c814663c..6d4b6738dfc2 100644 --- a/cranelift/codegen/meta/src/cdsl/instructions.rs +++ b/cranelift/codegen/meta/src/cdsl/instructions.rs @@ -134,6 +134,8 @@ pub(crate) struct InstructionContent { pub other_side_effects: bool, /// Does this instruction write to CPU flags? pub writes_cpu_flags: bool, + /// Should this opcode be considered to clobber all live registers, during regalloc? + pub clobbers_all_regs: bool, } impl InstructionContent { @@ -214,6 +216,7 @@ pub(crate) struct InstructionBuilder { can_store: bool, can_trap: bool, other_side_effects: bool, + clobbers_all_regs: bool, } impl InstructionBuilder { @@ -236,6 +239,7 @@ impl InstructionBuilder { can_store: false, can_trap: false, other_side_effects: false, + clobbers_all_regs: false, } } @@ -313,6 +317,11 @@ impl InstructionBuilder { self } + pub fn clobbers_all_regs(mut self, val: bool) -> Self { + self.clobbers_all_regs = val; + self + } + fn build(self, opcode_number: OpcodeNumber) -> Instruction { let operands_in = self.operands_in.unwrap_or_else(Vec::new); let operands_out = self.operands_out.unwrap_or_else(Vec::new); @@ -369,6 +378,7 @@ impl InstructionBuilder { can_trap: self.can_trap, other_side_effects: self.other_side_effects, writes_cpu_flags, + clobbers_all_regs: self.clobbers_all_regs, }) } } diff --git a/cranelift/codegen/meta/src/gen_inst.rs b/cranelift/codegen/meta/src/gen_inst.rs index af54257fea55..193412e8b945 100644 --- a/cranelift/codegen/meta/src/gen_inst.rs +++ b/cranelift/codegen/meta/src/gen_inst.rs @@ -523,6 +523,13 @@ fn gen_opcodes(all_inst: &AllInstructions, fmt: &mut Formatter) { "Does this instruction write to CPU flags?", fmt, ); + gen_bool_accessor( + all_inst, + |inst| inst.clobbers_all_regs, + "clobbers_all_regs", + "Should this opcode be considered to clobber all the registers, during regalloc?", + fmt, + ); }); fmt.line("}"); fmt.empty_line(); diff --git a/cranelift/codegen/meta/src/isa/x86/instructions.rs b/cranelift/codegen/meta/src/isa/x86/instructions.rs index 3be23cea9dfa..44ac3eeab9ca 100644 --- a/cranelift/codegen/meta/src/isa/x86/instructions.rs +++ b/cranelift/codegen/meta/src/isa/x86/instructions.rs @@ -562,6 +562,10 @@ pub(crate) fn define( "#, &formats.unary_global_value, ) + // This is a bit overly broad to mark as clobbering *all* the registers, because it should + // only preserve caller-saved registers. There's no way to indicate this to register + // allocation yet, though, so mark as clobbering all registers instead. + .clobbers_all_regs(true) .operands_in(vec![GV]) .operands_out(vec![addr]), ); @@ -574,6 +578,8 @@ pub(crate) fn define( "#, &formats.unary_global_value, ) + // See above comment for x86_elf_tls_get_addr. + .clobbers_all_regs(true) .operands_in(vec![GV]) .operands_out(vec![addr]), ); diff --git a/cranelift/codegen/src/regalloc/spilling.rs b/cranelift/codegen/src/regalloc/spilling.rs index 2e4b8bcb06c0..33d0fe9bd6bc 100644 --- a/cranelift/codegen/src/regalloc/spilling.rs +++ b/cranelift/codegen/src/regalloc/spilling.rs @@ -268,10 +268,7 @@ impl<'a> Context<'a> { // This means that we don't currently take advantage of callee-saved registers. // TODO: Be more sophisticated. let opcode = self.cur.func.dfg[inst].opcode(); - if call_sig.is_some() - || opcode == crate::ir::Opcode::X86ElfTlsGetAddr - || opcode == crate::ir::Opcode::X86MachoTlsGetAddr - { + if call_sig.is_some() || opcode.clobbers_all_regs() { for lv in throughs { if lv.affinity.is_reg() && !self.spills.contains(&lv.value) { self.spill_reg(lv.value);