diff --git a/cranelift/src/filetest/binemit.rs b/cranelift/src/filetest/binemit.rs index da0e2012a0..ae1c741d13 100644 --- a/cranelift/src/filetest/binemit.rs +++ b/cranelift/src/filetest/binemit.rs @@ -116,6 +116,7 @@ impl SubTest for TestBinEmit { // Give an encoding to any instruction that doesn't already have one. let mut divert = RegDiversions::new(); for ebb in func.layout.ebbs() { + divert.clear(); for inst in func.layout.ebb_insts(ebb) { if !func.encodings[inst].is_legal() { let mut legal_encodings = isa.legal_encodings( @@ -180,7 +181,6 @@ impl SubTest for TestBinEmit { } // Now emit all instructions. - divert.clear(); let mut sink = TextSink::new(isa); for ebb in func.layout.ebbs() { divert.clear(); diff --git a/lib/cretonne/src/binemit/relaxation.rs b/lib/cretonne/src/binemit/relaxation.rs index 4e6d1a2408..94835b736e 100644 --- a/lib/cretonne/src/binemit/relaxation.rs +++ b/lib/cretonne/src/binemit/relaxation.rs @@ -166,5 +166,24 @@ fn relax_branch( return encinfo.bytes(enc); } - unimplemented!(); + // Note: On some RISC ISAs, conditional branches have shorter range than unconditional + // branches, so one way of extending the range of a conditional branch is to invert its + // condition and make it branch over an unconditional jump which has the larger range. + // + // Splitting the EBB is problematic this late because there may be register diversions in + // effect across the conditional branch, and they can't survive the control flow edge to a new + // EBB. We have two options for handling that: + // + // 1. Set a flag on the new EBB that indicates it wants the preserve the register diversions of + // its layout predecessor, or + // 2. Use an encoding macro for the branch-over-jump pattern so we don't need to split the EBB. + // + // It seems that 1. would allow us to share code among RISC ISAs that need this. + // + // We can't allow register diversions to survive from the layout predecessor because the layout + // predecessor could contain kill points for some values that are live in this EBB, and + // diversions are not automatically cancelled when the live range of a value ends. + + // This assumes solution 2. above: + panic!("No branch in range for {:#x}-{:#x}", offset, dest_offset); } diff --git a/lib/cretonne/src/regalloc/coloring.rs b/lib/cretonne/src/regalloc/coloring.rs index 9a92df0270..69e1086416 100644 --- a/lib/cretonne/src/regalloc/coloring.rs +++ b/lib/cretonne/src/regalloc/coloring.rs @@ -468,7 +468,7 @@ impl<'a> Context<'a> { // // Values with a global live range that are not live in to `dest` could appear as branch // arguments, so they can't always be un-diverted. - self.undivert_regs(|lr, func| lr.livein_local_end(dest, &func.layout).is_some()); + self.undivert_regs(|lr, func| lr.is_livein(dest, &func.layout)); // Now handle the EBB arguments. let br_args = self.cur.func.dfg.inst_variable_args(inst); diff --git a/lib/cretonne/src/regalloc/context.rs b/lib/cretonne/src/regalloc/context.rs index 5e5c17c113..06d27ff325 100644 --- a/lib/cretonne/src/regalloc/context.rs +++ b/lib/cretonne/src/regalloc/context.rs @@ -17,7 +17,7 @@ use regalloc::spilling::Spilling; use regalloc::virtregs::VirtRegs; use result::CtonResult; use topo_order::TopoOrder; -use verifier::{verify_context, verify_liveness, verify_cssa}; +use verifier::{verify_context, verify_liveness, verify_cssa, verify_locations}; /// Persistent memory allocations for register allocation. pub struct Context { @@ -138,6 +138,7 @@ impl Context { if isa.flags().enable_verifier() { verify_context(func, cfg, domtree, isa)?; verify_liveness(isa, func, cfg, &self.liveness)?; + verify_locations(isa, func, Some(&self.liveness))?; verify_cssa(func, cfg, domtree, &self.liveness, &self.virtregs)?; } Ok(()) diff --git a/lib/cretonne/src/regalloc/liverange.rs b/lib/cretonne/src/regalloc/liverange.rs index 3da125ef3a..7b94fce280 100644 --- a/lib/cretonne/src/regalloc/liverange.rs +++ b/lib/cretonne/src/regalloc/liverange.rs @@ -383,6 +383,13 @@ impl LiveRange { }) } + /// Is this value live-in to `ebb`? + /// + /// An EBB argument is not considered to be live in. + pub fn is_livein(&self, ebb: Ebb, order: &PO) -> bool { + self.livein_local_end(ebb, order).is_some() + } + /// Get all the live-in intervals. pub fn liveins(&self) -> &[Interval] { &self.liveins diff --git a/lib/cretonne/src/verifier/locations.rs b/lib/cretonne/src/verifier/locations.rs new file mode 100644 index 0000000000..5f84f0403d --- /dev/null +++ b/lib/cretonne/src/verifier/locations.rs @@ -0,0 +1,316 @@ +//! Verify value locations. + +use ir; +use isa; +use regalloc::RegDiversions; +use regalloc::liveness::Liveness; +use verifier::Result; + +/// Verify value locations for `func`. +/// +/// After register allocation, every value must be assigned to a location - either a register or a +/// stack slot. These locations must be compatible with the constraints described by the +/// instruction encoding recipes. +/// +/// Values can be temporarily diverted to a different location by using the `regmove`, `regspill`, +/// and `regfill` instructions, but only inside an EBB. +/// +/// If a liveness analysis is provided, it is used to verify that there are no active register +/// diversions across control flow edges. +pub fn verify_locations( + isa: &isa::TargetIsa, + func: &ir::Function, + liveness: Option<&Liveness>, +) -> Result { + let verifier = LocationVerifier { + isa, + func, + reginfo: isa.register_info(), + encinfo: isa.encoding_info(), + liveness, + }; + verifier.check_constraints()?; + Ok(()) +} + +struct LocationVerifier<'a> { + isa: &'a isa::TargetIsa, + func: &'a ir::Function, + reginfo: isa::RegInfo, + encinfo: isa::EncInfo, + liveness: Option<&'a Liveness>, +} + +impl<'a> LocationVerifier<'a> { + /// Check that the assigned value locations match the operand constraints of their uses. + fn check_constraints(&self) -> Result { + let dfg = &self.func.dfg; + let mut divert = RegDiversions::new(); + + for ebb in self.func.layout.ebbs() { + // Diversions are reset at the top of each EBB. No diversions can exist across control + // flow edges. + divert.clear(); + for inst in self.func.layout.ebb_insts(ebb) { + let enc = self.func.encodings[inst]; + + if enc.is_legal() { + self.check_enc_constraints(inst, enc, &divert)? + } else { + self.check_ghost_results(inst)?; + } + + if let Some(sig) = dfg.call_signature(inst) { + self.check_call_abi(inst, sig, &divert)?; + } + + let opcode = dfg[inst].opcode(); + if opcode.is_return() { + self.check_return_abi(inst, &divert)?; + } + + if opcode.is_branch() && !divert.is_empty() { + self.check_cfg_edges(inst, &divert)?; + } + + self.update_diversions(inst, &mut divert)?; + } + } + + Ok(()) + } + + /// Check encoding constraints against the current value locations. + fn check_enc_constraints( + &self, + inst: ir::Inst, + enc: isa::Encoding, + divert: &RegDiversions, + ) -> Result { + let constraints = self.encinfo.operand_constraints(enc).expect( + "check_enc_constraints requires a legal encoding", + ); + + if constraints.satisfied(inst, divert, self.func) { + return Ok(()); + } + + // TODO: We could give a better error message here. + err!( + inst, + "{} constraints not satisfied", + self.encinfo.display(enc) + ) + } + + /// Check that the result values produced by a ghost instruction are not assigned a value + /// location. + fn check_ghost_results(&self, inst: ir::Inst) -> Result { + let results = self.func.dfg.inst_results(inst); + + for &res in results { + let loc = self.func.locations[res]; + if loc.is_assigned() { + return err!( + inst, + "ghost result {} value must not have a location ({}).", + res, + loc.display(&self.reginfo) + ); + } + } + + Ok(()) + } + + /// Check the ABI argument and result locations for a call. + fn check_call_abi(&self, inst: ir::Inst, sig: ir::SigRef, divert: &RegDiversions) -> Result { + let sig = &self.func.dfg.signatures[sig]; + let varargs = self.func.dfg.inst_variable_args(inst); + let results = self.func.dfg.inst_results(inst); + + for (abi, &value) in sig.argument_types.iter().zip(varargs) { + self.check_abi_location( + inst, + value, + abi, + divert.get(value, &self.func.locations), + ir::StackSlotKind::OutgoingArg, + )?; + } + + for (abi, &value) in sig.return_types.iter().zip(results) { + self.check_abi_location( + inst, + value, + abi, + self.func.locations[value], + ir::StackSlotKind::OutgoingArg, + )?; + } + + Ok(()) + } + + /// Check the ABI argument locations for a return. + fn check_return_abi(&self, inst: ir::Inst, divert: &RegDiversions) -> Result { + let sig = &self.func.signature; + let varargs = self.func.dfg.inst_variable_args(inst); + + for (abi, &value) in sig.return_types.iter().zip(varargs) { + self.check_abi_location( + inst, + value, + abi, + divert.get(value, &self.func.locations), + ir::StackSlotKind::IncomingArg, + )?; + } + + Ok(()) + } + + /// Check a single ABI location. + fn check_abi_location( + &self, + inst: ir::Inst, + value: ir::Value, + abi: &ir::ArgumentType, + loc: ir::ValueLoc, + want_kind: ir::StackSlotKind, + ) -> Result { + match abi.location { + ir::ArgumentLoc::Unassigned => {} + ir::ArgumentLoc::Reg(reg) => { + if loc != ir::ValueLoc::Reg(reg) { + return err!( + inst, + "ABI expects {} in {}, got {}", + value, + abi.location.display(&self.reginfo), + loc.display(&self.reginfo) + ); + } + } + ir::ArgumentLoc::Stack(offset) => { + if let ir::ValueLoc::Stack(ss) = loc { + let slot = &self.func.stack_slots[ss]; + if slot.kind != want_kind { + return err!( + inst, + "call argument {} should be in a {} slot, but {} is {}", + value, + want_kind, + ss, + slot.kind + ); + } + if slot.offset != offset { + return err!( + inst, + "ABI expects {} at stack offset {}, but {} is at {}", + value, + offset, + ss, + slot.offset + ); + } + } else { + return err!( + inst, + "ABI expects {} at stack offset {}, got {}", + value, + offset, + loc.display(&self.reginfo) + ); + } + } + } + + Ok(()) + } + + /// Update diversions to reflect the current instruction and check their consistency. + fn update_diversions(&self, inst: ir::Inst, divert: &mut RegDiversions) -> Result { + let (arg, src) = match self.func.dfg[inst] { + ir::InstructionData::RegMove { arg, src, .. } => (arg, ir::ValueLoc::Reg(src)), + ir::InstructionData::RegSpill { arg, src, .. } => (arg, ir::ValueLoc::Reg(src)), + ir::InstructionData::RegFill { arg, src, .. } => (arg, ir::ValueLoc::Stack(src)), + _ => return Ok(()), + }; + + if let Some(d) = divert.diversion(arg) { + if d.to != src { + return err!( + inst, + "inconsistent with current diversion to {}", + d.to.display(&self.reginfo) + ); + } + } else if self.func.locations[arg] != src { + return err!( + inst, + "inconsistent with global location {}", + self.func.locations[arg].display(&self.reginfo) + ); + } + + divert.apply(&self.func.dfg[inst]); + + Ok(()) + } + + /// We have active diversions before a branch. Make sure none of the diverted values are live + /// on the outgoing CFG edges. + fn check_cfg_edges(&self, inst: ir::Inst, divert: &RegDiversions) -> Result { + use ir::instructions::BranchInfo::*; + + // We can only check CFG edges if we have a liveness analysis. + let liveness = match self.liveness { + Some(l) => l, + None => return Ok(()), + }; + let dfg = &self.func.dfg; + + match dfg[inst].analyze_branch(&dfg.value_lists) { + NotABranch => { + panic!( + "No branch information for {}", + dfg.display_inst(inst, self.isa) + ) + } + SingleDest(ebb, _) => { + for d in divert.all() { + let lr = &liveness[d.value]; + if lr.is_livein(ebb, &self.func.layout) { + return err!( + inst, + "{} is diverted to {} and live in to {}", + d.value, + d.to.display(&self.reginfo), + ebb + ); + } + } + } + Table(jt) => { + for d in divert.all() { + let lr = &liveness[d.value]; + for (_, ebb) in self.func.jump_tables[jt].entries() { + if lr.is_livein(ebb, &self.func.layout) { + return err!( + inst, + "{} is diverted to {} and live in to {}", + d.value, + d.to.display(&self.reginfo), + ebb + ); + } + } + } + } + } + + Ok(()) + } +} diff --git a/lib/cretonne/src/verifier/mod.rs b/lib/cretonne/src/verifier/mod.rs index 735f78a754..196054bec3 100644 --- a/lib/cretonne/src/verifier/mod.rs +++ b/lib/cretonne/src/verifier/mod.rs @@ -72,8 +72,9 @@ use std::collections::BTreeSet; use std::cmp::Ordering; use iterators::IteratorExtras; -pub use self::liveness::verify_liveness; pub use self::cssa::verify_cssa; +pub use self::liveness::verify_liveness; +pub use self::locations::verify_locations; // Create an `Err` variant of `Result` from a location and `format!` arguments. macro_rules! err { @@ -94,6 +95,7 @@ macro_rules! err { mod cssa; mod liveness; +mod locations; /// A verifier error. #[derive(Debug, PartialEq, Eq)]