Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mir-opt] SimplifyLocals should also clean up debuginfo #110702

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
31 changes: 24 additions & 7 deletions compiler/rustc_mir_transform/src/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,10 @@ impl<'tcx> Visitor<'tcx> for UsedLocals {
}

fn visit_var_debug_info(&mut self, var_debug_info: &VarDebugInfo<'tcx>) {
scottmcm marked this conversation as resolved.
Show resolved Hide resolved
if !self.preserve_debug && debug_info_is_for_simple_local(var_debug_info).is_some() {
// We don't want to have to track *conditional* uses (such as
// "`_4` is used iff `_5` is used" from `debug x => Foo(_4, _5)`),
// so if this mentions multiple locals we just treat them all as used.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just drop parts of the composite? If _4 is unused, go from Foo { .0 => _4, .1 => _5 } to just Foo { .1 => _5 }?
This line can definitely just be a if !self.preserve_debug, and complexity will go in remove_unused_definitions_helper.

if !self.preserve_debug && debug_info_is_for_single_local(var_debug_info).is_some() {
return;
}

Expand All @@ -568,10 +571,9 @@ fn remove_unused_definitions_helper(used_locals: &mut UsedLocals, body: &mut Bod

if !used_locals.preserve_debug {
body.var_debug_info.retain(|info| {
let keep = if let Some(local) = debug_info_is_for_simple_local(info) {
let keep = if let Some(local) = debug_info_is_for_single_local(info) {
used_locals.is_used(local)
} else {
// Keep non-simple debuginfo no matter what
true
};

Expand Down Expand Up @@ -640,8 +642,23 @@ fn preserve_debug_even_if_never_generated(opts: &Options) -> bool {
}
}

// For now we only remove basic debuginfo, like `foo => _3`, and don't attempt
// to clean up more complicated things like `foo => Foo { .0 => _2, .1 => _4 }`
fn debug_info_is_for_simple_local(info: &VarDebugInfo<'_>) -> Option<Local> {
if let VarDebugInfoContents::Place(place) = info.value { place.as_local() } else { None }
/// Returns the only [`Local`] mentioned in `info`, if there's exactly one.
/// Otherwise return `None` if this mentions no `Local`s (probably because
/// it's [`VarDebugInfoContents::Const`]) or multiple `Local`s.
fn debug_info_is_for_single_local(info: &VarDebugInfo<'_>) -> Option<Local> {
struct SingleLocalFinder(Result<Option<Local>, ()>);
impl Visitor<'_> for SingleLocalFinder {
fn visit_local(&mut self, local: Local, _ctx: PlaceContext, _location: Location) {
match &mut self.0 {
Err(()) => {}
Ok(opt @ None) => *opt = Some(local),
Ok(Some(current)) if *current == local => {}
res @ Ok(Some(_)) => *res = Err(()),
}
}
}

let mut finder = SingleLocalFinder(Ok(None));
finder.visit_var_debug_info(info);
if let Ok(Some(local)) = finder.0 { Some(local) } else { None }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index projections are forbidden in debuginfo, so this can be simplified.

}
Loading