diff --git a/cranelift/filetests/regalloc/ghost-param.cton b/cranelift/filetests/regalloc/ghost-param.cton new file mode 100644 index 0000000000..8b0ba6c2ab --- /dev/null +++ b/cranelift/filetests/regalloc/ghost-param.cton @@ -0,0 +1,43 @@ +test regalloc +set is_64bit +isa intel haswell + +; This test case would create an EBB parameter that was a ghost value. +; The coalescer would insert a copy of the ghost value, leading to verifier errors. +; +; We don't allow EBB parameters to be ghost values any longer. +; +; Test case by binaryen fuzzer! + +function %pr215(i64 vmctx [%rdi]) native { +ebb0(v0: i64): + v10 = iconst.i64 0 + v1 = bitcast.f64 v10 + jump ebb5(v1) + +ebb5(v9: f64): + v11 = iconst.i64 0xffff_ffff_ff9a_421a + v4 = bitcast.f64 v11 + v6 = iconst.i32 0 + v7 = iconst.i32 1 + brnz v7, ebb4(v6) + v8 = iconst.i32 0 + jump ebb7(v8) + +ebb7(v5: i32): + brnz v5, ebb3(v4) + jump ebb5(v4) + +ebb4(v3: i32): + brnz v3, ebb2 + jump ebb3(v9) + +ebb3(v2: f64): + jump ebb2 + +ebb2: + jump ebb1 + +ebb1: + return +} diff --git a/lib/cretonne/src/regalloc/liveness.rs b/lib/cretonne/src/regalloc/liveness.rs index 6eee4f0c8c..2778f47f63 100644 --- a/lib/cretonne/src/regalloc/liveness.rs +++ b/lib/cretonne/src/regalloc/liveness.rs @@ -229,9 +229,9 @@ fn get_or_create<'a>( // signature. affinity = Affinity::abi(&func.signature.params[num], isa); } else { - // Don't apply any affinity to normal EBB parameters. - // They could be in a register or on the stack. - affinity = Default::default(); + // Give normal EBB parameters a register affinity matching their type. + let rc = isa.regclass_for_abi_type(func.dfg.value_type(value)); + affinity = Affinity::Reg(rc.into()); } } }; @@ -294,9 +294,6 @@ pub struct Liveness { /// This vector is always empty, except for inside that function. /// It lives here to avoid repeated allocation of scratch memory. worklist: Vec, - - /// Working space for the `propagate_ebb_params` algorithm. - ebb_params: Vec, } impl Liveness { @@ -309,7 +306,6 @@ impl Liveness { ranges: LiveRangeSet::new(), forest: LiveRangeForest::new(), worklist: Vec::new(), - ebb_params: Vec::new(), } } @@ -323,7 +319,6 @@ impl Liveness { self.ranges.clear(); self.forest.clear(); self.worklist.clear(); - self.ebb_params.clear(); } /// Get the live range for `value`, if it exists. @@ -444,53 +439,6 @@ impl Liveness { // EBB arguments or call/return ABI arguments. if let Some(constraint) = operand_constraints.next() { lr.affinity.merge(constraint, ®_info); - } else if lr.affinity.is_none() && encoding.is_legal() && - !func.dfg[inst].opcode().is_branch() - { - // This is a real encoded instruction using a value that doesn't yet have a - // concrete affinity. Most likely a call argument or a return value. Give - // the value a register affinity matching the ABI type. - // - // EBB arguments on a branch are not required to have an affinity. - let rc = isa.regclass_for_abi_type(func.dfg.value_type(arg)); - lr.affinity = Affinity::Reg(rc.into()); - } - } - } - } - - self.propagate_ebb_params(func, cfg); - } - - /// Propagate affinities for EBB parameters. - /// - /// If an EBB argument value has an affinity, all predecessors must pass a value with an - /// affinity. - pub fn propagate_ebb_params(&mut self, func: &Function, cfg: &ControlFlowGraph) { - assert!(self.ebb_params.is_empty()); - - for ebb in func.layout.ebbs() { - for &arg in func.dfg.ebb_params(ebb) { - let affinity = self.ranges.get(arg).unwrap().affinity; - if affinity.is_none() { - continue; - } - self.ebb_params.push(arg); - - // Now apply the affinity to all predecessors recursively. - while let Some(succ_arg) = self.ebb_params.pop() { - let (succ_ebb, num) = match func.dfg.value_def(succ_arg) { - ValueDef::Param(e, n) => (e, n), - _ => continue, - }; - - for (_, pred_branch) in cfg.pred_iter(succ_ebb) { - let pred_arg = func.dfg.inst_variable_args(pred_branch)[num]; - let pred_affinity = &mut self.ranges.get_mut(pred_arg).unwrap().affinity; - if pred_affinity.is_none() { - *pred_affinity = affinity; - self.ebb_params.push(pred_arg); - } } } } diff --git a/lib/cretonne/src/verifier/liveness.rs b/lib/cretonne/src/verifier/liveness.rs index a58d04caf7..95691625c8 100644 --- a/lib/cretonne/src/verifier/liveness.rs +++ b/lib/cretonne/src/verifier/liveness.rs @@ -100,7 +100,7 @@ impl<'a> LivenessVerifier<'a> { } // Check the uses. - for (idx, &val) in self.func.dfg.inst_args(inst).iter().enumerate() { + for &val in self.func.dfg.inst_args(inst) { let lr = match self.liveness.get(val) { Some(lr) => lr, None => return err!(inst, "{} has no live range", val), @@ -111,10 +111,7 @@ impl<'a> LivenessVerifier<'a> { if encoding.is_legal() { // A legal instruction is not allowed to depend on ghost values. - // - // A branch argument can be a ghost value if the corresponding destination - // EBB argument is a ghost value. - if lr.affinity.is_none() && !self.is_ghost_branch_argument(inst, idx) { + if lr.affinity.is_none() { return err!( inst, "{} is a ghost value used by a real [{}] instruction", @@ -147,32 +144,6 @@ impl<'a> LivenessVerifier<'a> { } } - /// Is argument `argnum` on `inst` a branch argument that leads to a ghost EBB argument? - fn is_ghost_branch_argument(&self, inst: Inst, argnum: usize) -> bool { - let dest = match self.func.dfg[inst].branch_destination() { - Some(d) => d, - None => return false, - }; - - let fixed_args = self.func.dfg[inst] - .opcode() - .constraints() - .fixed_value_arguments(); - if argnum < fixed_args { - return false; - } - - // If the EBB argument value in the destination is a ghost value, we'll allow a ghost - // branch argument. - self.func - .dfg - .ebb_params(dest) - .get(argnum - fixed_args) - .and_then(|&v| self.liveness.get(v)) - .map(|lr| lr.affinity.is_none()) - .unwrap_or(false) - } - /// Check the integrity of the live range `lr`. fn check_lr(&self, def: ProgramPoint, val: Value, lr: &LiveRange) -> Result { let l = &self.func.layout;