Skip to content

Commit

Permalink
Fixes #1240: Add a new accessor to indicate that an opcode requires s…
Browse files Browse the repository at this point in the history
…pilling all registers;
  • Loading branch information
bnjbvr committed Mar 23, 2020
1 parent c202a8e commit 1d5a678
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 4 deletions.
10 changes: 10 additions & 0 deletions cranelift/codegen/meta/src/cdsl/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -214,6 +216,7 @@ pub(crate) struct InstructionBuilder {
can_store: bool,
can_trap: bool,
other_side_effects: bool,
clobbers_all_regs: bool,
}

impl InstructionBuilder {
Expand All @@ -236,6 +239,7 @@ impl InstructionBuilder {
can_store: false,
can_trap: false,
other_side_effects: false,
clobbers_all_regs: false,
}
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
})
}
}
Expand Down
7 changes: 7 additions & 0 deletions cranelift/codegen/meta/src/gen_inst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
6 changes: 6 additions & 0 deletions cranelift/codegen/meta/src/isa/x86/instructions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
);
Expand All @@ -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]),
);
Expand Down
5 changes: 1 addition & 4 deletions cranelift/codegen/src/regalloc/spilling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 1d5a678

Please sign in to comment.