Skip to content

Commit

Permalink
Draft: detailed debug-info (DWARF) support in new backends (initially…
Browse files Browse the repository at this point in the history
… x64).

This PR is an initial attempt at propagating "value labels" all the way
from CLIF to DWARF metadata on the emitted machine code. The key idea is
as follows:

- Translate value-label metadata on the input into "value_label"
  pseudo-instructions when lowering into VCode. These
  pseudo-instructions take a register as input, denote a value label,
  and semantically are like a "move into value label" -- i.e., they
  update the current value (as seen by debugging tools) of the given
  local. These pseudo-instructions emit no machine code.

- Perform a dataflow analysis *at the machine-code level*, tracking
  value-labels that propagate into registers and into [SP+constant]
  stack storage. This is a forward dataflow fixpoint analysis where each
  storage location can contain a *set* of value labels, and each value
  label can reside in a *set* of storage locations. (Meet function is
  pairwise intersection by storage location.)

  This analysis traces value labels symbolically through loads and
  stores and reg-to-reg moves, so it will naturally handle spills and
  reloads without knowing anything special about them.

- When this analysis converges, we have, at each machine-code offset, a
  mapping from value labels to some number of storage locations; for
  each offset for each label, we choose the best location (prefer
  registers). Note that we can choose any location, as the symbolic
  dataflow analysis is sound and guarantees that the value at the
  value_label instruction propagates to all of the named locations.

- Then we can convert this mapping into a format that the DWARF
  generation code (wasmtime's debug crate) can use.

Unfortunately, it doesn't *quite* work yet with the gdb tests in
tests/all/debug/gdb.rs. Strangely, these tests are marked `#[ignore]`.
Needs more debugging, I'd like to publish this draft now as-is to share
the design and get feedback / bringup help on the tests.
  • Loading branch information
cfallin committed Jan 9, 2021
1 parent cacebfb commit f0c68e2
Show file tree
Hide file tree
Showing 20 changed files with 821 additions and 91 deletions.
2 changes: 1 addition & 1 deletion cranelift/codegen/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ all-arch = [
]

# For dependent crates that want to serialize some parts of cranelift
enable-serde = ["serde"]
enable-serde = ["serde", "regalloc/enable-serde"]

# Allow snapshotting regalloc test cases. Useful only to report bad register
# allocation failures, or for regalloc.rs developers.
Expand Down
1 change: 1 addition & 0 deletions cranelift/codegen/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ impl Context {
Ok(build_value_labels_ranges::<ComparableSourceLoc>(
&self.func,
&self.regalloc,
self.mach_compile_result.as_ref(),
isa,
))
}
Expand Down
1 change: 1 addition & 0 deletions cranelift/codegen/src/ir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub use crate::ir::table::TableData;
pub use crate::ir::trapcode::TrapCode;
pub use crate::ir::types::Type;
pub use crate::ir::valueloc::{ArgumentLoc, ValueLoc};
pub use crate::value_label::LabelValueLoc;
pub use cranelift_codegen_shared::condcodes;

use crate::binemit;
Expand Down
3 changes: 3 additions & 0 deletions cranelift/codegen/src/isa/aarch64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2365,6 +2365,9 @@ impl MachInstEmit for Inst {
sink.bind_label(jump_around_label);
}
}
&Inst::ValueLabelMarker { .. } => {
// Nothing; this is only used to compute debug info.
}
}

let end_off = sink.cur_offset();
Expand Down
29 changes: 28 additions & 1 deletion cranelift/codegen/src/isa/aarch64/inst/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::binemit::CodeOffset;
use crate::ir::types::{
B1, B128, B16, B32, B64, B8, F32, F64, FFLAGS, I128, I16, I32, I64, I8, I8X16, IFLAGS, R32, R64,
};
use crate::ir::{ExternalName, MemFlags, Opcode, SourceLoc, TrapCode, Type};
use crate::ir::{ExternalName, MemFlags, Opcode, SourceLoc, TrapCode, Type, ValueLabel};
use crate::isa::CallConv;
use crate::machinst::*;
use crate::{settings, CodegenError, CodegenResult};
Expand Down Expand Up @@ -1208,6 +1208,12 @@ pub enum Inst {
/// The needed space before the next deadline.
needed_space: CodeOffset,
},

/// A definition of a value label.
ValueLabelMarker {
reg: Reg,
label: ValueLabel,
},
}

fn count_zero_half_words(mut value: u64, num_half_words: u8) -> usize {
Expand Down Expand Up @@ -2015,6 +2021,9 @@ fn aarch64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) {
memarg_regs(mem, collector);
}
&Inst::VirtualSPOffsetAdj { .. } => {}
&Inst::ValueLabelMarker { reg, .. } => {
collector.add_use(reg);
}
&Inst::EmitIsland { .. } => {}
}
}
Expand Down Expand Up @@ -2765,6 +2774,9 @@ fn aarch64_map_regs<RUM: RegUsageMapper>(inst: &mut Inst, mapper: &RUM) {
}
&mut Inst::VirtualSPOffsetAdj { .. } => {}
&mut Inst::EmitIsland { .. } => {}
&mut Inst::ValueLabelMarker { ref mut reg, .. } => {
map_use(mapper, reg);
}
}
}

Expand Down Expand Up @@ -2960,6 +2972,17 @@ impl MachInst for Inst {
fn ref_type_regclass(_: &settings::Flags) -> RegClass {
RegClass::I64
}

fn gen_value_label_marker(label: ValueLabel, reg: Reg) -> Self {
Inst::ValueLabelMarker { label, reg }
}

fn defines_value_label(&self) -> Option<(ValueLabel, Reg)> {
match self {
Inst::ValueLabelMarker { label, reg } => Some((*label, *reg)),
_ => None,
}
}
}

//=============================================================================
Expand Down Expand Up @@ -4068,6 +4091,10 @@ impl Inst {
format!("virtual_sp_offset_adjust {}", offset)
}
&Inst::EmitIsland { needed_space } => format!("emit_island {}", needed_space),

&Inst::ValueLabelMarker { label, reg } => {
format!("value_label {:?}, {}", label, reg.show_rru(mb_rru))
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions cranelift/codegen/src/isa/aarch64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ impl MachBackend for AArch64Backend {
frame_size,
disasm,
unwind_info,
value_labels_ranges: None,
})
}

Expand Down
1 change: 1 addition & 0 deletions cranelift/codegen/src/isa/arm32/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ impl MachBackend for Arm32Backend {
frame_size,
disasm,
unwind_info: None,
value_labels_ranges: None,
})
}

Expand Down
6 changes: 6 additions & 0 deletions cranelift/codegen/src/isa/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,12 @@ pub trait TargetIsa: fmt::Display + Send + Sync {
Err(RegisterMappingError::UnsupportedArchitecture)
}

#[cfg(feature = "unwind")]
/// Map a regalloc::Reg to its corresponding DWARF register.
fn map_regalloc_reg_to_dwarf(&self, _: ::regalloc::Reg) -> Result<u16, RegisterMappingError> {
Err(RegisterMappingError::UnsupportedArchitecture)
}

/// Returns an iterator over legal encodings for the instruction.
fn legal_encodings<'a>(
&'a self,
Expand Down
4 changes: 4 additions & 0 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2962,6 +2962,10 @@ pub(crate) fn emit(
Inst::EpiloguePlaceholder => {
// Generate no code.
}

Inst::ValueLabelMarker { .. } => {
// Nothing; this is only used to compute debug info.
}
}

state.clear_post_insn();
Expand Down
47 changes: 45 additions & 2 deletions cranelift/codegen/src/isa/x64/inst/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! This module defines x86_64-specific machine instruction types.

use crate::binemit::{CodeOffset, StackMap};
use crate::ir::{types, ExternalName, Opcode, SourceLoc, TrapCode, Type};
use crate::ir::{types, ExternalName, Opcode, SourceLoc, TrapCode, Type, ValueLabel};
use crate::isa::x64::settings as x64_settings;
use crate::machinst::*;
use crate::{settings, settings::Flags, CodegenError, CodegenResult};
Expand Down Expand Up @@ -474,6 +474,9 @@ pub enum Inst {
/// reports its own `def`s/`use`s/`mod`s; this adds complexity (the instruction list is no
/// longer flat) and requires knowledge about semantics and initial-value independence anyway.
XmmUninitializedValue { dst: Writable<Reg> },

/// A definition of a value label.
ValueLabelMarker { reg: Reg, label: ValueLabel },
}

pub(crate) fn low32_will_sign_extend_to_64(x: u64) -> bool {
Expand Down Expand Up @@ -532,7 +535,8 @@ impl Inst {
| Inst::XmmCmpRmR { .. }
| Inst::XmmLoadConst { .. }
| Inst::XmmMinMaxSeq { .. }
| Inst::XmmUninitializedValue { .. } => None,
| Inst::XmmUninitializedValue { .. }
| Inst::ValueLabelMarker { .. } => None,

// These use dynamic SSE opcodes.
Inst::GprToXmm { op, .. }
Expand Down Expand Up @@ -1762,6 +1766,10 @@ impl PrettyPrint for Inst {
Inst::Hlt => "hlt".into(),

Inst::Ud2 { trap_code } => format!("ud2 {}", trap_code),

Inst::ValueLabelMarker { label, reg } => {
format!("value_label {:?}, {}", label, reg.show_rru(mb_rru))
}
}
}
}
Expand Down Expand Up @@ -2021,6 +2029,9 @@ fn x64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) {
| Inst::Fence { .. } => {
// No registers are used.
}
Inst::ValueLabelMarker { reg, .. } => {
collector.add_use(*reg);
}
}
}

Expand Down Expand Up @@ -2385,6 +2396,8 @@ fn x64_map_regs<RUM: RegUsageMapper>(inst: &mut Inst, mapper: &RUM) {
dst.map_uses(mapper);
}

Inst::ValueLabelMarker { ref mut reg, .. } => map_use(mapper, reg),

Inst::Ret
| Inst::EpiloguePlaceholder
| Inst::JmpKnown { .. }
Expand Down Expand Up @@ -2473,6 +2486,25 @@ impl MachInst for Inst {
}
}

fn stack_op_info(&self) -> Option<MachInstStackOpInfo> {
match self {
Self::VirtualSPOffsetAdj { offset } => Some(MachInstStackOpInfo::NomSPAdj(*offset)),
Self::MovRM {
size: 8,
src,
dst: SyntheticAmode::NominalSPOffset { simm32 },
} => Some(MachInstStackOpInfo::StoreNomSPOff(*src, *simm32 as i64)),
Self::Mov64MR {
src: SyntheticAmode::NominalSPOffset { simm32 },
dst,
} => Some(MachInstStackOpInfo::LoadNomSPOff(
dst.to_reg(),
*simm32 as i64,
)),
_ => None,
}
}

fn gen_move(dst_reg: Writable<Reg>, src_reg: Reg, ty: Type) -> Inst {
let rc_dst = dst_reg.to_reg().get_class();
let rc_src = src_reg.get_class();
Expand Down Expand Up @@ -2636,6 +2668,17 @@ impl MachInst for Inst {
RegClass::I64
}

fn gen_value_label_marker(label: ValueLabel, reg: Reg) -> Self {
Inst::ValueLabelMarker { label, reg }
}

fn defines_value_label(&self) -> Option<(ValueLabel, Reg)> {
match self {
Inst::ValueLabelMarker { label, reg } => Some((*label, *reg)),
_ => None,
}
}

type LabelUse = LabelUse;
}

Expand Down
10 changes: 9 additions & 1 deletion cranelift/codegen/src/isa/x64/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ use self::inst::EmitInfo;

use super::TargetIsa;
use crate::ir::{condcodes::IntCC, Function};
use crate::isa::unwind::systemv::RegisterMappingError;
use crate::isa::x64::{inst::regs::create_reg_universe_systemv, settings as x64_settings};
use crate::isa::Builder as IsaBuilder;
use crate::machinst::{compile, MachBackend, MachCompileResult, TargetIsaAdapter, VCode};
use crate::result::CodegenResult;
use crate::settings::{self as shared_settings, Flags};
use alloc::boxed::Box;
use regalloc::{PrettyPrint, RealRegUniverse};
use regalloc::{PrettyPrint, RealRegUniverse, Reg};
use target_lexicon::Triple;

mod abi;
Expand Down Expand Up @@ -60,6 +61,7 @@ impl MachBackend for X64Backend {
let buffer = buffer.finish();
let frame_size = vcode.frame_size();
let unwind_info = vcode.unwind_info()?;
let value_labels_ranges = vcode.value_labels_ranges()?;

let disasm = if want_disasm {
Some(vcode.show_rru(Some(&create_reg_universe_systemv(flags))))
Expand All @@ -72,6 +74,7 @@ impl MachBackend for X64Backend {
frame_size,
disasm,
unwind_info,
value_labels_ranges,
})
}

Expand Down Expand Up @@ -127,6 +130,11 @@ impl MachBackend for X64Backend {
fn create_systemv_cie(&self) -> Option<gimli::write::CommonInformationEntry> {
Some(inst::unwind::systemv::create_cie())
}

#[cfg(feature = "unwind")]
fn map_reg_to_dwarf(&self, reg: Reg) -> Result<u16, RegisterMappingError> {
inst::unwind::systemv::map_reg(reg).map(|reg| reg.0)
}
}

/// Create a new `isa::Builder`.
Expand Down
6 changes: 6 additions & 0 deletions cranelift/codegen/src/machinst/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::binemit;
use crate::ir;
use crate::isa::unwind::systemv::RegisterMappingError;
use crate::isa::{EncInfo, Encoding, Encodings, Legalize, RegClass, RegInfo, TargetIsa};
use crate::machinst::*;
use crate::regalloc::RegisterSet;
Expand Down Expand Up @@ -134,6 +135,11 @@ impl TargetIsa for TargetIsaAdapter {
self.backend.create_systemv_cie()
}

#[cfg(feature = "unwind")]
fn map_regalloc_reg_to_dwarf(&self, r: Reg) -> Result<u16, RegisterMappingError> {
self.backend.map_reg_to_dwarf(r)
}

fn as_any(&self) -> &dyn Any {
self as &dyn Any
}
Expand Down
Loading

0 comments on commit f0c68e2

Please sign in to comment.