Skip to content

Commit

Permalink
Auto merge of #73210 - wesleywiser:consts_in_debuginfo, r=oli-obk
Browse files Browse the repository at this point in the history
[mir-opt] Allow debuginfo to be generated for a constant or a Place

Prior to this commit, debuginfo was always generated by mapping a name
to a Place. This has the side-effect that `SimplifyLocals` cannot remove
locals that are only used for debuginfo because their other uses have
been const-propagated.

To allow these locals to be removed, we now allow debuginfo to point to
a constant value. The `ConstProp` pass detects when debuginfo points to
a local with a known constant value and replaces it with the value. This
allows the later `SimplifyLocals` pass to remove the local.
  • Loading branch information
bors committed Dec 15, 2020
2 parents e261649 + 0b18ed8 commit e99a89c
Show file tree
Hide file tree
Showing 16 changed files with 398 additions and 79 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1409,10 +1409,11 @@ fn generator_layout_and_saved_local_names(

let state_arg = mir::Local::new(1);
for var in &body.var_debug_info {
if var.place.local != state_arg {
let place = if let mir::VarDebugInfoContents::Place(p) = var.value { p } else { continue };
if place.local != state_arg {
continue;
}
match var.place.projection[..] {
match place.projection[..] {
[
// Deref of the `Pin<&mut Self>` state argument.
mir::ProjectionElem::Field(..),
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use super::FunctionCx;

impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
pub fn eval_mir_constant_to_operand(
&mut self,
&self,
bx: &mut Bx,
constant: &mir::Constant<'tcx>,
) -> Result<OperandRef<'tcx, Bx::Value>, ErrorHandled> {
Expand All @@ -21,7 +21,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}

pub fn eval_mir_constant(
&mut self,
&self,
constant: &mir::Constant<'tcx>,
) -> Result<ConstValue<'tcx>, ErrorHandled> {
match self.monomorphize(constant.literal).val {
Expand Down
105 changes: 73 additions & 32 deletions compiler/rustc_codegen_ssa/src/mir/debuginfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use rustc_span::symbol::{kw, Symbol};
use rustc_span::{BytePos, Span};
use rustc_target::abi::{LayoutOf, Size};

use super::operand::OperandValue;
use super::operand::{OperandRef, OperandValue};
use super::place::PlaceRef;
use super::{FunctionCx, LocalRef};

Expand Down Expand Up @@ -116,6 +116,24 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
span
}

fn spill_operand_to_stack(
operand: &OperandRef<'tcx, Bx::Value>,
name: Option<String>,
bx: &mut Bx,
) -> PlaceRef<'tcx, Bx::Value> {
// "Spill" the value onto the stack, for debuginfo,
// without forcing non-debuginfo uses of the local
// to also load from the stack every single time.
// FIXME(#68817) use `llvm.dbg.value` instead,
// at least for the cases which LLVM handles correctly.
let spill_slot = PlaceRef::alloca(bx, operand.layout);
if let Some(name) = name {
bx.set_var_name(spill_slot.llval, &(name + ".dbg.spill"));
}
operand.val.store(bx, spill_slot);
spill_slot
}

/// Apply debuginfo and/or name, after creating the `alloca` for a local,
/// or initializing the local with an operand (whichever applies).
pub fn debug_introduce_local(&self, bx: &mut Bx, local: mir::Local) {
Expand Down Expand Up @@ -226,17 +244,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
return;
}

// "Spill" the value onto the stack, for debuginfo,
// without forcing non-debuginfo uses of the local
// to also load from the stack every single time.
// FIXME(#68817) use `llvm.dbg.value` instead,
// at least for the cases which LLVM handles correctly.
let spill_slot = PlaceRef::alloca(bx, operand.layout);
if let Some(name) = name {
bx.set_var_name(spill_slot.llval, &(name + ".dbg.spill"));
}
operand.val.store(bx, spill_slot);
spill_slot
Self::spill_operand_to_stack(operand, name, bx)
}

LocalRef::Place(place) => *place,
Expand Down Expand Up @@ -308,6 +316,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
/// Partition all `VarDebugInfo` in `self.mir`, by their base `Local`.
pub fn compute_per_local_var_debug_info(
&self,
bx: &mut Bx,
) -> Option<IndexVec<mir::Local, Vec<PerLocalVarDebugInfo<'tcx, Bx::DIVariable>>>> {
let full_debug_info = self.cx.sess().opts.debuginfo == DebugInfo::Full;

Expand All @@ -322,31 +331,63 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
} else {
None
};

let dbg_var = dbg_scope_and_span.map(|(dbg_scope, _, span)| {
let place = var.place;
let var_ty = self.monomorphized_place_ty(place.as_ref());
let var_kind = if self.mir.local_kind(place.local) == mir::LocalKind::Arg
&& place.projection.is_empty()
&& var.source_info.scope == mir::OUTERMOST_SOURCE_SCOPE
{
let arg_index = place.local.index() - 1;

// FIXME(eddyb) shouldn't `ArgumentVariable` indices be
// offset in closures to account for the hidden environment?
// Also, is this `+ 1` needed at all?
VariableKind::ArgumentVariable(arg_index + 1)
} else {
VariableKind::LocalVariable
let (var_ty, var_kind) = match var.value {
mir::VarDebugInfoContents::Place(place) => {
let var_ty = self.monomorphized_place_ty(place.as_ref());
let var_kind = if self.mir.local_kind(place.local) == mir::LocalKind::Arg
&& place.projection.is_empty()
&& var.source_info.scope == mir::OUTERMOST_SOURCE_SCOPE
{
let arg_index = place.local.index() - 1;

// FIXME(eddyb) shouldn't `ArgumentVariable` indices be
// offset in closures to account for the hidden environment?
// Also, is this `+ 1` needed at all?
VariableKind::ArgumentVariable(arg_index + 1)
} else {
VariableKind::LocalVariable
};
(var_ty, var_kind)
}
mir::VarDebugInfoContents::Const(c) => {
let ty = self.monomorphize(c.literal.ty);
(ty, VariableKind::LocalVariable)
}
};

self.cx.create_dbg_var(var.name, var_ty, dbg_scope, var_kind, span)
});

per_local[var.place.local].push(PerLocalVarDebugInfo {
name: var.name,
source_info: var.source_info,
dbg_var,
projection: var.place.projection,
});
match var.value {
mir::VarDebugInfoContents::Place(place) => {
per_local[place.local].push(PerLocalVarDebugInfo {
name: var.name,
source_info: var.source_info,
dbg_var,
projection: place.projection,
});
}
mir::VarDebugInfoContents::Const(c) => {
if let Some(dbg_var) = dbg_var {
let dbg_loc = match self.dbg_loc(var.source_info) {
Some(dbg_loc) => dbg_loc,
None => continue,
};

if let Ok(operand) = self.eval_mir_constant_to_operand(bx, &c) {
let base = Self::spill_operand_to_stack(
&operand,
Some(var.name.to_string()),
bx,
);

bx.dbg_var_addr(dbg_var, dbg_loc, base.llval, Size::ZERO, &[]);
}
}
}
}
}
Some(per_local)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
caller_location: None,
};

fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info();
fx.per_local_var_debug_info = fx.compute_per_local_var_debug_info(&mut bx);

for const_ in &mir.required_consts {
if let Err(err) = fx.eval_mir_constant(const_) {
Expand Down
21 changes: 18 additions & 3 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1060,6 +1060,23 @@ impl<'tcx> LocalDecl<'tcx> {
}
}

#[derive(Clone, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
pub enum VarDebugInfoContents<'tcx> {
/// NOTE(eddyb) There's an unenforced invariant that this `Place` is
/// based on a `Local`, not a `Static`, and contains no indexing.
Place(Place<'tcx>),
Const(Constant<'tcx>),
}

impl<'tcx> Debug for VarDebugInfoContents<'tcx> {
fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
match self {
VarDebugInfoContents::Const(c) => write!(fmt, "{}", c),
VarDebugInfoContents::Place(p) => write!(fmt, "{:?}", p),
}
}
}

/// Debug information pertaining to a user variable.
#[derive(Clone, Debug, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
pub struct VarDebugInfo<'tcx> {
Expand All @@ -1071,9 +1088,7 @@ pub struct VarDebugInfo<'tcx> {
pub source_info: SourceInfo,

/// Where the data for this user variable is to be found.
/// NOTE(eddyb) There's an unenforced invariant that this `Place` is
/// based on a `Local`, not a `Static`, and contains no indexing.
pub place: Place<'tcx>,
pub value: VarDebugInfoContents<'tcx>,
}

///////////////////////////////////////////////////////////////////////////
Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,16 +829,20 @@ macro_rules! make_mir_visitor {
let VarDebugInfo {
name: _,
source_info,
place,
value,
} = var_debug_info;

self.visit_source_info(source_info);
let location = START_BLOCK.start_location();
self.visit_place(
place,
PlaceContext::NonUse(NonUseContext::VarDebugInfo),
location,
);
match value {
VarDebugInfoContents::Const(c) => self.visit_constant(c, location),
VarDebugInfoContents::Place(place) =>
self.visit_place(
place,
PlaceContext::NonUse(NonUseContext::VarDebugInfo),
location
),
}
}

fn super_source_scope(&mut self,
Expand Down
26 changes: 14 additions & 12 deletions compiler/rustc_mir/src/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_infer::infer::{InferCtxt, TyCtxtInferExt};
use rustc_middle::hir::place::PlaceBase as HirPlaceBase;
use rustc_middle::mir::{
traversal, Body, ClearCrossCrate, Local, Location, Mutability, Operand, Place, PlaceElem,
PlaceRef,
PlaceRef, VarDebugInfoContents,
};
use rustc_middle::mir::{AggregateKind, BasicBlock, BorrowCheckResult, BorrowKind};
use rustc_middle::mir::{Field, ProjectionElem, Promoted, Rvalue, Statement, StatementKind};
Expand Down Expand Up @@ -135,19 +135,21 @@ fn do_mir_borrowck<'a, 'tcx>(

let mut local_names = IndexVec::from_elem(None, &input_body.local_decls);
for var_debug_info in &input_body.var_debug_info {
if let Some(local) = var_debug_info.place.as_local() {
if let Some(prev_name) = local_names[local] {
if var_debug_info.name != prev_name {
span_bug!(
var_debug_info.source_info.span,
"local {:?} has many names (`{}` vs `{}`)",
local,
prev_name,
var_debug_info.name
);
if let VarDebugInfoContents::Place(place) = var_debug_info.value {
if let Some(local) = place.as_local() {
if let Some(prev_name) = local_names[local] {
if var_debug_info.name != prev_name {
span_bug!(
var_debug_info.source_info.span,
"local {:?} has many names (`{}` vs `{}`)",
local,
prev_name,
var_debug_info.name
);
}
}
local_names[local] = Some(var_debug_info.name);
}
local_names[local] = Some(var_debug_info.name);
}
}

Expand Down
102 changes: 102 additions & 0 deletions compiler/rustc_mir/src/transform/const_debuginfo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
//! Finds locals which are assigned once to a const and unused except for debuginfo and converts
//! their debuginfo to use the const directly, allowing the local to be removed.
use rustc_middle::{
mir::{
visit::{PlaceContext, Visitor},
Body, Constant, Local, Location, Operand, Rvalue, StatementKind, VarDebugInfoContents,
},
ty::TyCtxt,
};

use crate::transform::MirPass;
use rustc_index::{bit_set::BitSet, vec::IndexVec};

pub struct ConstDebugInfo;

impl<'tcx> MirPass<'tcx> for ConstDebugInfo {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if !tcx.sess.opts.debugging_opts.unsound_mir_opts {
return;
}

trace!("running ConstDebugInfo on {:?}", body.source);

for (local, constant) in find_optimization_oportunities(body) {
for debuginfo in &mut body.var_debug_info {
if let VarDebugInfoContents::Place(p) = debuginfo.value {
if p.local == local && p.projection.is_empty() {
trace!(
"changing debug info for {:?} from place {:?} to constant {:?}",
debuginfo.name,
p,
constant
);
debuginfo.value = VarDebugInfoContents::Const(constant);
}
}
}
}
}
}

struct LocalUseVisitor {
local_mutating_uses: IndexVec<Local, u8>,
local_assignment_locations: IndexVec<Local, Option<Location>>,
}

fn find_optimization_oportunities<'tcx>(body: &Body<'tcx>) -> Vec<(Local, Constant<'tcx>)> {
let mut visitor = LocalUseVisitor {
local_mutating_uses: IndexVec::from_elem(0, &body.local_decls),
local_assignment_locations: IndexVec::from_elem(None, &body.local_decls),
};

visitor.visit_body(body);

let mut locals_to_debuginfo = BitSet::new_empty(body.local_decls.len());
for debuginfo in &body.var_debug_info {
if let VarDebugInfoContents::Place(p) = debuginfo.value {
if let Some(l) = p.as_local() {
locals_to_debuginfo.insert(l);
}
}
}

let mut eligable_locals = Vec::new();
for (local, mutating_uses) in visitor.local_mutating_uses.drain_enumerated(..) {
if mutating_uses != 1 || !locals_to_debuginfo.contains(local) {
continue;
}

if let Some(location) = visitor.local_assignment_locations[local] {
let bb = &body[location.block];

// The value is assigned as the result of a call, not a constant
if bb.statements.len() == location.statement_index {
continue;
}

if let StatementKind::Assign(box (p, Rvalue::Use(Operand::Constant(box c)))) =
&bb.statements[location.statement_index].kind
{
if let Some(local) = p.as_local() {
eligable_locals.push((local, *c));
}
}
}
}

eligable_locals
}

impl<'tcx> Visitor<'tcx> for LocalUseVisitor {
fn visit_local(&mut self, local: &Local, context: PlaceContext, location: Location) {
if context.is_mutating_use() {
self.local_mutating_uses[*local] = self.local_mutating_uses[*local].saturating_add(1);

if context.is_place_assignment() {
self.local_assignment_locations[*local] = Some(location);
}
}
}
}
Loading

0 comments on commit e99a89c

Please sign in to comment.