Browse Source

Address review comments.

pull/2565/head
Chris Fallin 4 years ago
parent
commit
f54d0d05c7
No known key found for this signature in database GPG Key ID: 31649E4FE65EB465
  1. 168
      cranelift/codegen/src/machinst/debug.rs
  2. 4
      crates/debug/src/transform/expression.rs

168
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<i64>,
/// Forward map from labeled values to sets of locations.
label_to_locs: HashMap<ValueLabel, HashSet<ValueLoc>>,
/// Reverse map for each register indicating the value it holds, if any.
reg_to_label: HashMap<Reg, ValueLabel>,
/// Reverse map for each stack offset indicating the value it holds, if any.
stack_to_label: HashMap<i64, ValueLabel>,
}
/// Get the registers written (mod'd or def'd) by a machine instruction.
fn get_inst_writes<M: MachInst>(m: &M) -> Vec<Reg> {
// 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: MachInst>(m: &M) -> Vec<Reg> {
}
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(&reg) {
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(&reg) {
@ -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<M: MachInst>(&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<T> IntersectFrom for HashSet<T>
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<T> IntersectFrom for Option<T>
where
T: Copy + Eq,
{
/// Intersectino for Option<T>: 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<I: VCodeInst>(
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<I: VCodeInst>(
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<u32, AnalysisInfo> = HashMap::new();
@ -321,26 +396,26 @@ pub(crate) fn compute<I: VCodeInst>(
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<I: VCodeInst>(
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<I: VCodeInst>(
} 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<I: VCodeInst>(
// 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,

4
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 => {

Loading…
Cancel
Save