From 13af22b46bf6e9c912a53182ef3ae6f0b4abdf3a Mon Sep 17 00:00:00 2001 From: Jakob Stoklund Olesen Date: Wed, 17 Jan 2018 12:39:10 -0800 Subject: [PATCH] Track register pressure for dead EBB parameters. The spiller wasn't tracking register pressure correctly for dead EBB parameters in visit_ebb_header(). Make sure we free any dead EBB parameters. Fixes #223 --- .../filetests/regalloc/spill-noregs.cton | 176 ++++++++++++++++++ lib/cretonne/src/regalloc/coloring.rs | 2 +- .../src/regalloc/live_value_tracker.rs | 8 +- lib/cretonne/src/regalloc/reload.rs | 2 +- lib/cretonne/src/regalloc/spilling.rs | 33 ++-- 5 files changed, 204 insertions(+), 17 deletions(-) create mode 100644 cranelift/filetests/regalloc/spill-noregs.cton diff --git a/cranelift/filetests/regalloc/spill-noregs.cton b/cranelift/filetests/regalloc/spill-noregs.cton new file mode 100644 index 0000000000..de4fa618bf --- /dev/null +++ b/cranelift/filetests/regalloc/spill-noregs.cton @@ -0,0 +1,176 @@ +test regalloc +set is_64bit +isa intel + +; Test case found by the Binaryen fuzzer. +; +; The spiller panics with a +; 'Ran out of GPR registers when inserting copy before v68 = icmp.i32 eq v66, v67', +; lib/cretonne/src/regalloc/spilling.rs:425:28 message. +; +; The process_reg_uses() function is trying to insert a copy before the icmp instruction in ebb4 +; and runs out of registers to spill. Note that ebb7 has a lot of dead parameter values. +; +; The spiller was not releasing register pressure for dead EBB parameters. + +function %pr223(i32 [%rdi], i64 vmctx [%rsi]) -> i64 [%rax] native { +ebb0(v0: i32, v1: i64): + v2 = iconst.i32 0 + v3 = iconst.i64 0 + v4 = iconst.i32 0xffff_ffff_bb3f_4a2c + brz v4, ebb5 + jump ebb1 + +ebb1: + v5 = iconst.i32 0 + v6 = copy.i64 v3 + v7 = copy.i64 v3 + v8 = copy.i64 v3 + v9 = copy.i64 v3 + v10 = copy.i64 v3 + v11 = copy.i64 v3 + v12 = copy.i64 v3 + v13 = copy.i64 v3 + v14 = copy.i64 v3 + v15 = copy.i64 v3 + v16 = copy.i64 v3 + brnz v5, ebb4(v2, v3, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16) + jump ebb2 + +ebb2: + v17 = iconst.i32 0 + v18 = copy.i64 v3 + v19 = copy.i64 v3 + v20 = copy.i64 v3 + v21 = copy.i64 v3 + v22 = copy.i64 v3 + v23 = copy.i64 v3 + v24 = copy.i64 v3 + v25 = copy.i64 v3 + v26 = copy.i64 v3 + v27 = copy.i64 v3 + v28 = copy.i64 v3 + brnz v17, ebb4(v2, v3, v18, v19, v20, v21, v22, v23, v24, v25, v26, v27, v28) + jump ebb3 + +ebb3: + jump ebb1 + +ebb4(v29: i32, v30: i64, v31: i64, v32: i64, v33: i64, v34: i64, v35: i64, v36: i64, v37: i64, v38: i64, v39: i64, v40: i64, v41: i64): + jump ebb7(v29, v30, v31, v32, v33, v34, v35, v36, v37, v38, v39, v40, v41) + +ebb5: + jump ebb6 + +ebb6: + v42 = copy.i64 v3 + v43 = copy.i64 v3 + v44 = copy.i64 v3 + v45 = copy.i64 v3 + v46 = copy.i64 v3 + v47 = copy.i64 v3 + v48 = copy.i64 v3 + v49 = copy.i64 v3 + v50 = copy.i64 v3 + v51 = copy.i64 v3 + v52 = copy.i64 v3 + jump ebb7(v2, v3, v42, v43, v44, v45, v46, v47, v48, v49, v50, v51, v52) + +ebb7(v53: i32, v54: i64, v55: i64, v56: i64, v57: i64, v58: i64, v59: i64, v60: i64, v61: i64, v62: i64, v63: i64, v64: i64, v65: i64): + v66 = iconst.i32 0 + v67 = iconst.i32 0 + v68 = icmp eq v66, v67 + v69 = bint.i32 v68 + jump ebb8 + +ebb8: + jump ebb9 + +ebb9: + v70 = iconst.i32 0xffff_ffff_ffff_912f + brz v70, ebb10 + jump ebb35 + +ebb10: + v71 = iconst.i32 0 + brz v71, ebb11 + jump ebb27 + +ebb11: + jump ebb12 + +ebb12: + jump ebb13 + +ebb13: + jump ebb14 + +ebb14: + jump ebb15 + +ebb15: + jump ebb16 + +ebb16: + jump ebb17 + +ebb17: + jump ebb18 + +ebb18: + jump ebb19 + +ebb19: + jump ebb20 + +ebb20: + jump ebb21 + +ebb21: + jump ebb22 + +ebb22: + jump ebb23 + +ebb23: + jump ebb24 + +ebb24: + jump ebb25 + +ebb25: + jump ebb26 + +ebb26: + jump ebb27 + +ebb27: + jump ebb28 + +ebb28: + jump ebb29 + +ebb29: + jump ebb30 + +ebb30: + jump ebb31 + +ebb31: + jump ebb32 + +ebb32: + jump ebb33 + +ebb33: + jump ebb34 + +ebb34: + jump ebb35 + +ebb35: + jump ebb36 + +ebb36: + trap user0 +} diff --git a/lib/cretonne/src/regalloc/coloring.rs b/lib/cretonne/src/regalloc/coloring.rs index 2f52dfaabc..9d82c0b21a 100644 --- a/lib/cretonne/src/regalloc/coloring.rs +++ b/lib/cretonne/src/regalloc/coloring.rs @@ -158,7 +158,7 @@ impl<'a> Context<'a> { fn visit_ebb(&mut self, ebb: Ebb, tracker: &mut LiveValueTracker) { dbg!("Coloring {}:", ebb); let mut regs = self.visit_ebb_header(ebb, tracker); - tracker.drop_dead_args(); + tracker.drop_dead_params(); self.divert.clear(); // Now go through the instructions in `ebb` and color the values they define. diff --git a/lib/cretonne/src/regalloc/live_value_tracker.rs b/lib/cretonne/src/regalloc/live_value_tracker.rs index e6f53151e2..d0810a5cf7 100644 --- a/lib/cretonne/src/regalloc/live_value_tracker.rs +++ b/lib/cretonne/src/regalloc/live_value_tracker.rs @@ -160,9 +160,9 @@ impl LiveValueTracker { /// been visited first. /// /// Returns `(liveins, args)` as a pair of slices. The first slice is the set of live-in values - /// from the immediate dominator. The second slice is the set of `ebb` arguments that are live. + /// from the immediate dominator. The second slice is the set of `ebb` parameters. /// - /// Dead arguments with no uses are included in `args`. Call `drop_dead_args()` to remove them. + /// Dead parameters with no uses are included in `args`. Call `drop_dead_args()` to remove them. pub fn ebb_top( &mut self, ebb: Ebb, @@ -314,8 +314,8 @@ impl LiveValueTracker { /// Drop any values that are marked as `is_dead`. /// - /// Use this after calling `ebb_top` to clean out dead EBB arguments. - pub fn drop_dead_args(&mut self) { + /// Use this after calling `ebb_top` to clean out dead EBB parameters. + pub fn drop_dead_params(&mut self) { self.live.remove_dead_values(); } diff --git a/lib/cretonne/src/regalloc/reload.rs b/lib/cretonne/src/regalloc/reload.rs index 8ed95628fd..2cb37d7d6a 100644 --- a/lib/cretonne/src/regalloc/reload.rs +++ b/lib/cretonne/src/regalloc/reload.rs @@ -120,7 +120,7 @@ impl<'a> Context<'a> { fn visit_ebb(&mut self, ebb: Ebb, tracker: &mut LiveValueTracker) { dbg!("Reloading {}:", ebb); self.visit_ebb_header(ebb, tracker); - tracker.drop_dead_args(); + tracker.drop_dead_params(); // visit_ebb_header() places us at the first interesting instruction in the EBB. while let Some(inst) = self.cur.current_inst() { diff --git a/lib/cretonne/src/regalloc/spilling.rs b/lib/cretonne/src/regalloc/spilling.rs index 4c5b8b9ec7..84ef76fdff 100644 --- a/lib/cretonne/src/regalloc/spilling.rs +++ b/lib/cretonne/src/regalloc/spilling.rs @@ -120,7 +120,8 @@ impl<'a> Context<'a> { dbg!("Spilling {}:", ebb); self.cur.goto_top(ebb); self.visit_ebb_header(ebb, tracker); - tracker.drop_dead_args(); + tracker.drop_dead_params(); + self.process_spills(tracker); while let Some(inst) = self.cur.next_inst() { if let Some(constraints) = @@ -163,8 +164,22 @@ impl<'a> Context<'a> { } } + // Free all dead registers in `regs` from the pressure set. + fn free_dead_regs(&mut self, regs: &[LiveValue]) { + for lv in regs { + if lv.is_dead { + if let Affinity::Reg(rci) = lv.affinity { + if !self.spills.contains(&lv.value) { + let rc = self.reginfo.rc(rci); + self.pressure.free(rc); + } + } + } + } + } + fn visit_ebb_header(&mut self, ebb: Ebb, tracker: &mut LiveValueTracker) { - let (liveins, args) = tracker.ebb_top( + let (liveins, params) = tracker.ebb_top( ebb, &self.cur.func.dfg, self.liveness, @@ -177,22 +192,17 @@ impl<'a> Context<'a> { self.pressure.reset(); self.take_live_regs(liveins); - // An EBB can have an arbitrary (up to 2^16...) number of EBB arguments, so they are not + // An EBB can have an arbitrary (up to 2^16...) number of parameters, so they are not // guaranteed to fit in registers. - for lv in args { + for lv in params { if let Affinity::Reg(rci) = lv.affinity { let rc = self.reginfo.rc(rci); 'try_take: while let Err(mask) = self.pressure.take_transient(rc) { - dbg!( - "Need {} reg for EBB argument {} from {} live-ins", - rc, - lv.value, - liveins.len() - ); + dbg!("Need {} reg for EBB param {}", rc, lv.value); match self.spill_candidate(mask, liveins) { Some(cand) => { dbg!( - "Spilling live-in {} to make room for {} EBB argument {}", + "Spilling live-in {} to make room for {} EBB param {}", cand, rc, lv.value @@ -217,6 +227,7 @@ impl<'a> Context<'a> { // The transient pressure counts for the EBB arguments are accurate. Just preserve them. self.pressure.preserve_transient(); + self.free_dead_regs(params); } fn visit_inst(