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.

Sign up for GitHub

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
Open

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

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
29e9024
Don't treat a local as used just because it's mentioned in debug info…
scottmcm Apr 22, 2023
22e9177
Bless or tweak the mir-opt tests
scottmcm May 7, 2023
c50de3c
Update a codegen test to not optimize away its checks
scottmcm Apr 22, 2023
e95ff7a
And fix the UI tests that also had their error optimized out
scottmcm Apr 22, 2023
c8df0f1
Yay, it broke some incremental compilation tests too
scottmcm Apr 22, 2023
0129219
Cool; somehow this fixed a duplicate diagnostic
scottmcm Jul 4, 2024
fdf4fff
Updates for the crashes tests
scottmcm Jul 4, 2024
83744cb
Preserve unused locals in some stable-mir tests
scottmcm Jul 4, 2024
1c585a5
Clarify the purpose of the single-Local check, and check it more flex…
scottmcm Jul 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 69 additions & 5 deletions compiler/rustc_mir_transform/src/simplify.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use rustc_index::{Idx, IndexSlice, IndexVec};
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::{DebugInfo, Options};
use smallvec::SmallVec;

pub enum SimplifyCfg {
Expand Down Expand Up @@ -373,9 +374,15 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals {
}
}

pub fn remove_unused_definitions<'tcx>(body: &mut Body<'tcx>) {
/// Go through the basic blocks and remove statements that assign locals that aren't read.
///
/// This does *not* clean up the `local_decl`s. If you want it to do that too,
/// call [`simplify_locals`] instead of this.
pub(crate) fn remove_unused_definitions<'tcx>(body: &mut Body<'tcx>) {
let preserve_debug = true;

// First, we're going to get a count of *actual* uses for every `Local`.
let mut used_locals = UsedLocals::new(body);
let mut used_locals = UsedLocals::new(body, preserve_debug);

// Next, we're going to remove any `Local` with zero actual uses. When we remove those
// `Locals`, we're also going to subtract any uses of other `Locals` from the `used_locals`
Expand All @@ -385,9 +392,18 @@ pub fn remove_unused_definitions<'tcx>(body: &mut Body<'tcx>) {
remove_unused_definitions_helper(&mut used_locals, body);
}

pub fn simplify_locals<'tcx>(body: &mut Body<'tcx>, tcx: TyCtxt<'tcx>) {
/// Go through the basic blocks and remove statements that assign locals that aren't read.
///
/// Then go through and remove unneeded `local_decl`s, rewriting all mentions of them
/// in all the statements.
///
/// If you only want the (faster) statement pruning, call [`remove_unused_definitions`]
/// instead of this.
pub(crate) fn simplify_locals<'tcx>(body: &mut Body<'tcx>, tcx: TyCtxt<'tcx>) {
let preserve_debug = preserve_debug_even_if_never_generated(&tcx.sess.opts);

// First, we're going to get a count of *actual* uses for every `Local`.
let mut used_locals = UsedLocals::new(body);
let mut used_locals = UsedLocals::new(body, preserve_debug);

// Next, we're going to remove any `Local` with zero actual uses. When we remove those
// `Locals`, we're also going to subtract any uses of other `Locals` from the `used_locals`
Expand Down Expand Up @@ -438,15 +454,17 @@ struct UsedLocals {
increment: bool,
arg_count: u32,
use_count: IndexVec<Local, u32>,
preserve_debug: bool,
}

impl UsedLocals {
/// Determines which locals are used & unused in the given body.
fn new(body: &Body<'_>) -> Self {
fn new(body: &Body<'_>, preserve_debug: bool) -> Self {
let mut this = Self {
increment: true,
arg_count: body.arg_count.try_into().unwrap(),
use_count: IndexVec::from_elem(0, &body.local_decls),
preserve_debug,
};
this.visit_body(body);
this
Expand Down Expand Up @@ -527,6 +545,14 @@ impl<'tcx> Visitor<'tcx> for UsedLocals {
self.use_count[local] -= 1;
}
}

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() {
return;
}

self.super_var_debug_info(var_debug_info);
}
}

/// Removes unused definitions. Updates the used locals to reflect the changes made.
Expand All @@ -540,6 +566,27 @@ fn remove_unused_definitions_helper(used_locals: &mut UsedLocals, body: &mut Bod
while modified {
modified = false;

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

if !keep {
trace!("removing var_debug_info {:?}", info);

// While we did modify the debug info, we don't need to set the
// `modified` flag, as we didn't change `used_locals`, and thus
// we don't need to re-run the loop to look again.
}

keep
});
}

for data in body.basic_blocks.as_mut_preserves_cfg() {
// Remove unnecessary StorageLive and StorageDead annotations.
data.statements.retain(|statement| {
Expand Down Expand Up @@ -581,3 +628,20 @@ impl<'tcx> MutVisitor<'tcx> for LocalUpdater<'tcx> {
*l = self.map[*l].unwrap();
}
}

fn preserve_debug_even_if_never_generated(opts: &Options) -> bool {
if let Some(p) = opts.unstable_opts.inline_mir_preserve_debug {
Copy link
Member Author

Choose a reason for hiding this comment

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

annot: this is the material change from when I first tried to do this.

The problems from before were that config.toml affected the standard library build, making the tests inconsistent, but obeying this -Z flag fixes those problems.

return p;
}

match opts.debuginfo {
DebugInfo::None | DebugInfo::LineDirectivesOnly | DebugInfo::LineTablesOnly => false,
DebugInfo::Limited | DebugInfo::Full => true,
}
}

// 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 }`
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not foo => _3.projection?
Why not composite fields?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated it to handle projections too, so long as there's only one local mentioned.

Added some comments to clarify why it doesn't accept composites. Basically I don't want to start tracking enough state to be able to handle that, at least in this PR.

fn debug_info_is_for_simple_local(info: &VarDebugInfo<'_>) -> Option<Local> {
if let VarDebugInfoContents::Place(place) = info.value { place.as_local() } else { None }
}
2 changes: 1 addition & 1 deletion tests/codegen/slice-init.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ compile-flags: -C no-prepopulate-passes
//@ compile-flags: -C no-prepopulate-passes -C debuginfo=2

#![crate_type = "lib"]

Expand Down
11 changes: 0 additions & 11 deletions tests/crashes/101962.rs
View file
Open in desktop

This file was deleted.

2 changes: 2 additions & 0 deletions tests/crashes/79409.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
//@ known-bug: #79409
//@ compile-flags: -Z mir-opt-level=0
// (Only fails if the use of the place isn't optimized out)

#![feature(extern_types)]
#![feature(unsized_locals)]
Expand Down
8 changes: 4 additions & 4 deletions tests/incremental/hashes/enum_constructors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,9 @@ pub fn change_constructor_path_c_like() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,optimized_mir,typeck")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,optimized_mir,typeck")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_constructor_path_c_like() {
let _x = Clike2::B;
Expand All @@ -318,9 +318,9 @@ pub fn change_constructor_variant_c_like() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn change_constructor_variant_c_like() {
let _x = Clike::C;
Expand Down
14 changes: 7 additions & 7 deletions tests/incremental/hashes/for_loops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pub fn change_loop_body() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn change_loop_body() {
let mut _x = 0;
Expand All @@ -53,9 +53,9 @@ pub fn change_iteration_variable_name() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn change_iteration_variable_name() {
let mut _x = 0;
Expand All @@ -78,9 +78,9 @@ pub fn change_iteration_variable_pattern() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, optimized_mir, typeck")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, optimized_mir, typeck")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_iteration_variable_pattern() {
let mut _x = 0;
Expand Down Expand Up @@ -180,7 +180,7 @@ pub fn add_loop_label_to_break() {
#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn add_loop_label_to_break() {
let mut _x = 0;
Expand Down
4 changes: 2 additions & 2 deletions tests/incremental/hashes/if_expressions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ pub fn change_condition_if_let(x: Option<u32>) -> u32 {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,optimized_mir,typeck")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,optimized_mir,typeck")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_condition_if_let(x: Option<u32>) -> u32 {
if let Some(_ ) = x {
Expand Down
40 changes: 20 additions & 20 deletions tests/incremental/hashes/let_expressions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ pub fn change_name() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn change_name() {
let _y = 2u64;
Expand Down Expand Up @@ -57,9 +57,9 @@ pub fn change_type() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_type() {
let _x: u8 = 2;
Expand All @@ -74,9 +74,9 @@ pub fn change_mutability_of_reference_type() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_mutability_of_reference_type() {
let _x: &mut u64;
Expand All @@ -93,7 +93,7 @@ pub fn change_mutability_of_slot() {
#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_mutability_of_slot() {
let _x: u64 = 0;
Expand All @@ -108,9 +108,9 @@ pub fn change_simple_binding_to_pattern() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_simple_binding_to_pattern() {
let (_a, _b) = (0u8, 'x');
Expand All @@ -125,9 +125,9 @@ pub fn change_name_in_pattern() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn change_name_in_pattern() {
let (_a, _c) = (1u8, 'y');
Expand All @@ -142,9 +142,9 @@ pub fn add_ref_in_pattern() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn add_ref_in_pattern() {
let (ref _a, _b) = (1u8, 'y');
Expand All @@ -159,9 +159,9 @@ pub fn add_amp_in_pattern() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn add_amp_in_pattern() {
let (&_a, _b) = (&1u8, 'y');
Expand All @@ -178,7 +178,7 @@ pub fn change_mutability_of_binding_in_pattern() {
#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn change_mutability_of_binding_in_pattern() {
let (mut _a, _b) = (99u8, 'q');
Expand All @@ -193,9 +193,9 @@ pub fn add_initializer() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck,optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail6")]
pub fn add_initializer() {
let _x: i16 = 3i16;
Expand All @@ -210,9 +210,9 @@ pub fn change_initializer() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn change_initializer() {
let _x = 5u16;
Expand Down
4 changes: 2 additions & 2 deletions tests/incremental/hashes/loop_expressions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ pub fn change_loop_body() {
}

#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail2", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes, optimized_mir")]
#[rustc_clean(cfg="cfail5", except="opt_hir_owner_nodes")]
#[rustc_clean(cfg="cfail6")]
pub fn change_loop_body() {
let mut _x = 0;
Expand Down
Loading
Loading