diff --git a/cranelift/codegen/src/machinst/debug.rs b/cranelift/codegen/src/machinst/debug.rs index 79a7467c56..4768b8d06e 100644 --- a/cranelift/codegen/src/machinst/debug.rs +++ b/cranelift/codegen/src/machinst/debug.rs @@ -24,9 +24,12 @@ use crate::machinst::*; use crate::value_label::{LabelValueLoc, ValueLabelsRanges, ValueLocRange}; use log::trace; use regalloc::{Reg, RegUsageCollector}; -use std::collections::{HashMap, HashSet, VecDeque}; +use std::collections::{HashMap, HashSet}; use std::hash::Hash; +/// Location of a labeled value: in a register or in a stack slot. Note that a +/// value may live in more than one location; `AnalysisInfo` maps each +/// value-label to multiple `ValueLoc`s. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] enum ValueLoc { Reg(Reg), @@ -61,13 +64,19 @@ impl ValueLoc { /// Mappings at one program point. #[derive(Clone, Debug)] struct AnalysisInfo { - // nominal SP relative to real SP. + /// Nominal SP relative to real SP. If `None`, then the offset is + /// indeterminate (i.e., we merged to the lattice 'bottom' element). This + /// should not happen in well-formed code. nominal_sp_offset: Option, + /// Forward map from labeled values to sets of locations. label_to_locs: HashMap>, + /// Reverse map for each register indicating the value it holds, if any. reg_to_label: HashMap, + /// Reverse map for each stack offset indicating the value it holds, if any. stack_to_label: HashMap, } +/// Get the registers written (mod'd or def'd) by a machine instruction. fn get_inst_writes(m: &M) -> Vec { // TODO: expose this part of regalloc.rs's interface publicly. let mut vecs = RegUsageCollector::get_empty_reg_vecs_test_framework_only(false); @@ -78,6 +87,8 @@ fn get_inst_writes(m: &M) -> Vec { } impl AnalysisInfo { + /// Create a new analysis state. This is the "top" lattice element at which + /// the fixpoint dataflow analysis starts. fn new() -> Self { AnalysisInfo { nominal_sp_offset: Some(0), @@ -87,6 +98,8 @@ impl AnalysisInfo { } } + /// Remove all locations for a given labeled value. Used when the labeled + /// value is redefined (so old values become stale). fn clear_label(&mut self, label: ValueLabel) { if let Some(locs) = self.label_to_locs.remove(&label) { for loc in locs { @@ -101,6 +114,9 @@ impl AnalysisInfo { } } } + + /// Remove a label from a register, if any. Used, e.g., if the register is + /// overwritten. fn clear_reg(&mut self, reg: Reg) { if let Some(label) = self.reg_to_label.remove(®) { if let Some(locs) = self.label_to_locs.get_mut(&label) { @@ -108,6 +124,9 @@ impl AnalysisInfo { } } } + + /// Remove a label from a stack offset, if any. Used, e.g., when the stack + /// slot is overwritten. fn clear_stack_off(&mut self, off: i64) { if let Some(label) = self.stack_to_label.remove(&off) { if let Some(locs) = self.label_to_locs.get_mut(&label) { @@ -115,6 +134,9 @@ impl AnalysisInfo { } } } + + /// Indicate that a labeled value is newly defined and its new value is in + /// `reg`. fn def_label_at_reg(&mut self, label: ValueLabel, reg: Reg) { self.clear_label(label); self.label_to_locs @@ -123,6 +145,8 @@ impl AnalysisInfo { .insert(ValueLoc::Reg(reg)); self.reg_to_label.insert(reg, label); } + + /// Process a store from a register to a stack slot (offset). fn store_reg(&mut self, reg: Reg, off: i64) { self.clear_stack_off(off); if let Some(label) = self.reg_to_label.get(®) { @@ -132,6 +156,8 @@ impl AnalysisInfo { self.stack_to_label.insert(off, *label); } } + + /// Process a load from a stack slot (offset) to a register. fn load_reg(&mut self, reg: Reg, off: i64) { self.clear_reg(reg); if let Some(&label) = self.stack_to_label.get(&off) { @@ -141,6 +167,8 @@ impl AnalysisInfo { self.reg_to_label.insert(reg, label); } } + + /// Process a move from one register to another. fn move_reg(&mut self, to: Reg, from: Reg) { self.clear_reg(to); if let Some(&label) = self.reg_to_label.get(&from) { @@ -151,6 +179,9 @@ impl AnalysisInfo { } } + /// Update the analysis state w.r.t. an instruction's effects. Given the + /// state just before `inst`, this method updates `self` to be the state + /// just after `inst`. fn step(&mut self, inst: &M) { for write in get_inst_writes(inst) { self.clear_reg(write); @@ -179,12 +210,29 @@ impl AnalysisInfo { } } +/// Trait used to implement the dataflow analysis' meet (intersect) function +/// onthe `AnalysisInfo` components. For efficiency, this is implemented as a +/// mutation on the LHS, rather than a pure functional operation. trait IntersectFrom { fn intersect_from(&mut self, other: &Self) -> IntersectResult; } + +/// Result of an intersection operation. Indicates whether the mutated LHS +/// (which becomes the intersection result) differs from the original LHS. Also +/// indicates if the value has become "empty" and should be removed from a +/// parent container, if any. struct IntersectResult { + /// Did the intersection change the LHS input (the one that was mutated into + /// the result)? This is needed to drive the fixpoint loop; when no more + /// changes occur, then we have converted. changed: bool, - remove: bool, + /// Is the resulting value "empty"? This can be used when a container, such + /// as a map, holds values of this (intersection result) type; when + /// `is_empty` is true for the merge of the values at a particular key, we + /// can remove that key from the merged (intersected) result. This is not + /// necessary for analysis correctness but reduces the memory and runtime + /// cost of the fixpoint loop. + is_empty: bool, } impl IntersectFrom for AnalysisInfo { @@ -208,7 +256,7 @@ impl IntersectFrom for AnalysisInfo { .changed; IntersectResult { changed, - remove: false, + is_empty: false, } } } @@ -218,6 +266,8 @@ where K: Copy + Eq + Hash, V: IntersectFrom, { + /// Intersection for hashmap: remove keys that are not in both inputs; + /// recursively intersect values for keys in common. fn intersect_from(&mut self, other: &Self) -> IntersectResult { let mut changed = false; let mut remove_keys = vec![]; @@ -226,29 +276,29 @@ where remove_keys.push(*k); } } - for k in remove_keys { + for k in &remove_keys { changed = true; - self.remove(&k); + self.remove(k); } - let mut remove_keys = vec![]; + remove_keys.clear(); for k in other.keys() { if let Some(v) = self.get_mut(k) { let result = v.intersect_from(other.get(k).unwrap()); changed |= result.changed; - if result.remove { + if result.is_empty { remove_keys.push(*k); } } } - for k in remove_keys { + for k in &remove_keys { changed = true; - self.remove(&k); + self.remove(k); } IntersectResult { changed, - remove: self.len() == 0, + is_empty: self.len() == 0, } } } @@ -256,6 +306,7 @@ impl IntersectFrom for HashSet where T: Copy + Eq + Hash, { + /// Intersection for hashset: just take the set intersection. fn intersect_from(&mut self, other: &Self) -> IntersectResult { let mut changed = false; let mut remove = vec![]; @@ -271,16 +322,18 @@ where IntersectResult { changed, - remove: self.len() == 0, + is_empty: self.len() == 0, } } } impl IntersectFrom for ValueLabel { + // Intersection for labeled value: remove if not equal. This is equivalent + // to a three-level lattice with top, bottom, and unordered set of + // individual labels in between. fn intersect_from(&mut self, other: &Self) -> IntersectResult { - // Remove if not equal (simple top -> values -> bottom lattice) IntersectResult { changed: false, - remove: *self != *other, + is_empty: *self != *other, } } } @@ -288,6 +341,8 @@ impl IntersectFrom for Option where T: Copy + Eq, { + /// Intersectino for Option: recursively intersect if both `Some`, else + /// `None`. fn intersect_from(&mut self, other: &Self) -> IntersectResult { let mut changed = false; if !(self.is_some() && other.is_some() && self == other) { @@ -296,15 +351,35 @@ where } IntersectResult { changed, - remove: self.is_none(), + is_empty: self.is_none(), } } } +/// Compute the value-label ranges (locations for program-point ranges for +/// labeled values) from a given `VCode` compilation result. +/// +/// In order to compute this information, we perform a dataflow analysis on the +/// machine code. To do so, and translate the results into a form usable by the +/// debug-info consumers, we need to know two additional things: +/// +/// - The machine-code layout (code offsets) of the instructions. DWARF is +/// encoded in terms of instruction *ends* (and we reason about value +/// locations at program points *after* instructions, to match this), so we +/// take an array `inst_ends`, giving us code offsets for each instruction's +/// end-point. (Note that this is one *past* the last byte; so a 4-byte +/// instruction at offset 0 has an end offset of 4.) +/// +/// - The locations of the labels to which branches will jump. Branches can tell +/// us about their targets in terms of `MachLabel`s, but we don't know where +/// those `MachLabel`s will be placed in the linear array of instructions. We +/// take the array `label_insn_index` to provide this info: for a label with +/// index `l`, `label_insn_index[l]` is the index of the instruction before +/// which that label is bound. pub(crate) fn compute( insts: &[I], inst_ends: &[u32], - label_insn_iix: &[u32], + label_insn_index: &[u32], ) -> ValueLabelsRanges { let inst_start = |idx: usize| if idx == 0 { 0 } else { inst_ends[idx - 1] }; @@ -312,7 +387,7 @@ pub(crate) fn compute( for i in 0..insts.len() { trace!(" #{} end: {} -> {:?}", i, inst_ends[i], insts[i]); } - trace!("label_insn_iix: {:?}", label_insn_iix); + trace!("label_insn_index: {:?}", label_insn_index); // Info at each block head, indexed by label. let mut block_starts: HashMap = HashMap::new(); @@ -321,26 +396,26 @@ pub(crate) fn compute( block_starts.insert(0, AnalysisInfo::new()); // Worklist: label indices for basic blocks. - let mut worklist = VecDeque::new(); + let mut worklist = Vec::new(); let mut worklist_set = HashSet::new(); - worklist.push_back(0); + worklist.push(0); worklist_set.insert(0); while !worklist.is_empty() { - let block = worklist.pop_front().unwrap(); + let block = worklist.pop().unwrap(); worklist_set.remove(&block); let mut state = block_starts.get(&block).unwrap().clone(); trace!("at block {} -> state: {:?}", block, state); // Iterate for each instruction in the block (we break at the first // terminator we see). - let mut iix = label_insn_iix[block as usize]; - while iix < insts.len() as u32 { - state.step(&insts[iix as usize]); - trace!(" -> inst #{}: {:?}", iix, insts[iix as usize]); + let mut index = label_insn_index[block as usize]; + while index < insts.len() as u32 { + state.step(&insts[index as usize]); + trace!(" -> inst #{}: {:?}", index, insts[index as usize]); trace!(" --> state: {:?}", state); - let term = insts[iix as usize].is_term(); + let term = insts[index as usize].is_term(); if term.is_term() { for succ in term.get_succs() { trace!(" SUCCESSOR block {}", succ.get()); @@ -348,7 +423,7 @@ pub(crate) fn compute( trace!(" orig state: {:?}", succ_state); if succ_state.intersect_from(&state).changed { if worklist_set.insert(succ.get()) { - worklist.push_back(succ.get()); + worklist.push(succ.get()); } trace!(" (changed)"); } @@ -356,14 +431,14 @@ pub(crate) fn compute( } else { // First time seeing this block block_starts.insert(succ.get(), state.clone()); - worklist.push_back(succ.get()); + worklist.push(succ.get()); worklist_set.insert(succ.get()); } } break; } - iix += 1; + index += 1; } } @@ -371,32 +446,41 @@ pub(crate) fn compute( // value-label locations. let mut value_labels_ranges: ValueLabelsRanges = HashMap::new(); - for block in 0..label_insn_iix.len() { - let start_iix = label_insn_iix[block]; - let end_iix = if block == label_insn_iix.len() - 1 { + for block in 0..label_insn_index.len() { + let start_index = label_insn_index[block]; + let end_index = if block == label_insn_index.len() - 1 { insts.len() as u32 } else { - label_insn_iix[block + 1] + label_insn_index[block + 1] }; let block = block as u32; let mut state = block_starts.get(&block).unwrap().clone(); - for iix in start_iix..end_iix { - let offset = inst_start(iix as usize); - let end = inst_ends[iix as usize]; - state.step(&insts[iix as usize]); + for index in start_index..end_index { + let offset = inst_start(index as usize); + let end = inst_ends[index as usize]; + state.step(&insts[index as usize]); for (label, locs) in &state.label_to_locs { - trace!(" inst {} has label {:?} -> locs {:?}", iix, label, locs); + trace!(" inst {} has label {:?} -> locs {:?}", index, label, locs); // Find an appropriate loc: a register if possible, otherwise pick the first stack // loc. let reg = locs.iter().cloned().find(|l| l.is_reg()); - let stack = locs.iter().cloned().find(|l| l.is_stack()); - if let Some(loc) = reg.or(stack) { + let loc = reg.or_else(|| locs.iter().cloned().find(|l| l.is_stack())); + if let Some(loc) = loc { let loc = LabelValueLoc::from(loc); let list = value_labels_ranges.entry(*label).or_insert_with(|| vec![]); - if list.is_empty() - || list.last().unwrap().end <= offset - || list.last().unwrap().loc != loc + // If the existing location list for this value-label is + // either empty, or has an end location that does not extend + // to the current offset, then we have to append a new + // entry. Otherwise, we can extend the current entry. + // + // Note that `end` is one past the end of the instruction; + // it appears that `end` is exclusive, so a mapping valid at + // offset 5 will have start = 5, end = 6. + if list + .last() + .map(|last| last.end <= offset || last.loc != loc) + .unwrap_or(true) { list.push(ValueLocRange { loc, diff --git a/crates/debug/src/transform/expression.rs b/crates/debug/src/transform/expression.rs index 44d2299aaf..57f3cc7a21 100644 --- a/crates/debug/src/transform/expression.rs +++ b/crates/debug/src/transform/expression.rs @@ -226,8 +226,8 @@ fn append_memory_deref( } } LabelValueLoc::Reg(r) => { - let reg = isa.map_regalloc_reg_to_dwarf(r)? as u8; - writer.write_u8(gimli::constants::DW_OP_breg0.0 + reg)?; + let reg = isa.map_regalloc_reg_to_dwarf(r)?; + writer.write_op_breg(reg)?; let memory_offset = match frame_info.vmctx_memory_offset() { Some(offset) => offset, None => {