Skip to content

Commit

Permalink
We don't need NonNull::as_ptr debuginfo
Browse files Browse the repository at this point in the history
Stop pessimizing the use of local variables in core by skipping debug info for MIR temporaries in tiny (single-BB) functions.

For functions as simple as this -- `Pin::new`, etc -- nobody every actually wants debuginfo for them in the first place.  They're more like intrinsics than real functions, and stepping over them is good.
  • Loading branch information
scottmcm committed Dec 10, 2024
1 parent 96e51d9 commit a7fc76a
Show file tree
Hide file tree
Showing 11 changed files with 170 additions and 106 deletions.
3 changes: 3 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ declare_passes! {
mod simplify_comparison_integral : SimplifyComparisonIntegral;
mod single_use_consts : SingleUseConsts;
mod sroa : ScalarReplacementOfAggregates;
mod strip_debuginfo : StripDebugInfo;
mod unreachable_enum_branching : UnreachableEnumBranching;
mod unreachable_prop : UnreachablePropagation;
mod validate : Validator;
Expand Down Expand Up @@ -699,6 +700,8 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&o1(simplify_branches::SimplifyConstCondition::Final),
&o1(remove_noop_landing_pads::RemoveNoopLandingPads),
&o1(simplify::SimplifyCfg::Final),
// After the last SimplifyCfg, because this wants one-block functions.
&strip_debuginfo::StripDebugInfo,
&copy_prop::CopyProp,
&dead_store_elimination::DeadStoreElimination::Final,
&nrvo::RenameReturnPlace,
Expand Down
34 changes: 34 additions & 0 deletions compiler/rustc_mir_transform/src/strip_debuginfo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
use rustc_session::config::MirStripDebugInfo;

/// Conditionally remove some of the VarDebugInfo in MIR.
///
/// In particular, stripping non-parameter debug info for tiny, primitive-like
/// methods in core saves work later, and nobody ever wanted to use it anyway.
pub(super) struct StripDebugInfo;

impl<'tcx> crate::MirPass<'tcx> for StripDebugInfo {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
sess.opts.unstable_opts.mir_strip_debuginfo != MirStripDebugInfo::None
}

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
match tcx.sess.opts.unstable_opts.mir_strip_debuginfo {
MirStripDebugInfo::None => return,
MirStripDebugInfo::AllLocals => {}
MirStripDebugInfo::LocalsInTinyFunctions
if let TerminatorKind::Return { .. } =
body.basic_blocks[START_BLOCK].terminator().kind => {}
MirStripDebugInfo::LocalsInTinyFunctions => return,
}

body.var_debug_info.retain(|vdi| {
matches!(
vdi.value,
VarDebugInfoContents::Place(place)
if place.local.as_usize() <= body.arg_count && place.local != RETURN_PLACE,
)
});
}
}
16 changes: 12 additions & 4 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,13 @@ impl ToString for DebugInfoCompression {
}
}

#[derive(Clone, Copy, Debug, PartialEq, Hash)]
pub enum MirStripDebugInfo {
None,
LocalsInTinyFunctions,
AllLocals,
}

/// Split debug-information is enabled by `-C split-debuginfo`, this enum is only used if split
/// debug-information is enabled (in either `Packed` or `Unpacked` modes), and the platform
/// uses DWARF for debug-information.
Expand Down Expand Up @@ -2900,10 +2907,10 @@ pub(crate) mod dep_tracking {
BranchProtection, CFGuard, CFProtection, CollapseMacroDebuginfo, CoverageOptions,
CrateType, DebugInfo, DebugInfoCompression, ErrorOutputType, FmtDebug, FunctionReturn,
InliningThreshold, InstrumentCoverage, InstrumentXRay, LinkerPluginLto, LocationDetail,
LtoCli, NextSolverConfig, OomStrategy, OptLevel, OutFileName, OutputType, OutputTypes,
PatchableFunctionEntry, Polonius, RemapPathScopeComponents, ResolveDocLinks,
SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath, SymbolManglingVersion,
WasiExecModel,
LtoCli, MirStripDebugInfo, NextSolverConfig, OomStrategy, OptLevel, OutFileName,
OutputType, OutputTypes, PatchableFunctionEntry, Polonius, RemapPathScopeComponents,
ResolveDocLinks, SourceFileHashAlgorithm, SplitDwarfKind, SwitchWithOptPath,
SymbolManglingVersion, WasiExecModel,
};
use crate::lint;
use crate::utils::NativeLib;
Expand Down Expand Up @@ -2971,6 +2978,7 @@ pub(crate) mod dep_tracking {
LtoCli,
DebugInfo,
DebugInfoCompression,
MirStripDebugInfo,
CollapseMacroDebuginfo,
UnstableFeatures,
NativeLib,
Expand Down
14 changes: 14 additions & 0 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@ mod desc {
pub(crate) const parse_cfprotection: &str = "`none`|`no`|`n` (default), `branch`, `return`, or `full`|`yes`|`y` (equivalent to `branch` and `return`)";
pub(crate) const parse_debuginfo: &str = "either an integer (0, 1, 2), `none`, `line-directives-only`, `line-tables-only`, `limited`, or `full`";
pub(crate) const parse_debuginfo_compression: &str = "one of `none`, `zlib`, or `zstd`";
pub(crate) const parse_mir_strip_debuginfo: &str =
"one of `none`, `locals-in-tiny-functions`, or `all-locals`";
pub(crate) const parse_collapse_macro_debuginfo: &str = "one of `no`, `external`, or `yes`";
pub(crate) const parse_strip: &str = "either `none`, `debuginfo`, or `symbols`";
pub(crate) const parse_linker_flavor: &str = ::rustc_target::spec::LinkerFlavorCli::one_of();
Expand Down Expand Up @@ -925,6 +927,16 @@ pub mod parse {
true
}

pub(crate) fn parse_mir_strip_debuginfo(slot: &mut MirStripDebugInfo, v: Option<&str>) -> bool {
match v {
Some("none") => *slot = MirStripDebugInfo::None,
Some("locals-in-tiny-functions") => *slot = MirStripDebugInfo::LocalsInTinyFunctions,
Some("all-locals") => *slot = MirStripDebugInfo::AllLocals,
_ => return false,
};
true
}

pub(crate) fn parse_linker_flavor(slot: &mut Option<LinkerFlavorCli>, v: Option<&str>) -> bool {
match v.and_then(LinkerFlavorCli::from_str) {
Some(lf) => *slot = Some(lf),
Expand Down Expand Up @@ -1893,6 +1905,8 @@ options! {
#[rustc_lint_opt_deny_field_access("use `Session::mir_opt_level` instead of this field")]
mir_opt_level: Option<usize> = (None, parse_opt_number, [TRACKED],
"MIR optimization level (0-4; default: 1 in non optimized builds and 2 in optimized builds)"),
mir_strip_debuginfo: MirStripDebugInfo = (MirStripDebugInfo::None, parse_mir_strip_debuginfo, [TRACKED],
"Whether to remove some of the MIR debug info from methods. Default: None"),
move_size_limit: Option<usize> = (None, parse_opt_number, [TRACKED],
"the size at which the `large_assignments` lint starts to be emitted"),
mutable_noalias: bool = (true, parse_bool, [TRACKED],
Expand Down
5 changes: 5 additions & 0 deletions src/bootstrap/src/core/builder/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,11 @@ impl Builder<'_> {
// even if we're not going to output debuginfo for the crate we're currently building,
// so that it'll be available when downstream consumers of std try to use it.
rustflags.arg("-Zinline-mir-preserve-debug");

// FIXME: always pass this after the next `#[cfg(bootstrap)]` update.
if compiler.stage != 0 {
rustflags.arg("-Zmir_strip_debuginfo=locals-in-tiny-functions");
}
}

Cargo {
Expand Down
5 changes: 3 additions & 2 deletions tests/codegen/mem-replace-big-type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ pub fn replace_big(dst: &mut Big, src: Big) -> Big {
// CHECK-NOT: call void @llvm.memcpy

// For a large type, we expect exactly three `memcpy`s
// CHECK-LABEL: define internal void @{{.+}}mem{{.+}}replace{{.+}}sret([56 x i8])
// CHECK-LABEL: define internal void @{{.+}}mem{{.+}}replace{{.+}}(ptr
// CHECK-SAME: sret([56 x i8]){{.+}}[[RESULT:%.+]], ptr{{.+}}%dest, ptr{{.+}}%src)
// CHECK-NOT: call void @llvm.memcpy
// CHECK: call void @llvm.memcpy.{{.+}}(ptr align 8 %result, ptr align 8 %dest, i{{.*}} 56, i1 false)
// CHECK: call void @llvm.memcpy.{{.+}}(ptr align 8 [[RESULT]], ptr align 8 %dest, i{{.*}} 56, i1 false)
// CHECK-NOT: call void @llvm.memcpy
// CHECK: call void @llvm.memcpy.{{.+}}(ptr align 8 %dest, ptr align 8 %src, i{{.*}} 56, i1 false)
// CHECK-NOT: call void @llvm.memcpy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
+ scope 3 (inlined Pin::<&mut {coroutine@$DIR/inline_coroutine.rs:20:5: 20:8}>::new) {
+ debug pointer => _3;
+ scope 4 (inlined Pin::<&mut {coroutine@$DIR/inline_coroutine.rs:20:5: 20:8}>::new_unchecked) {
+ debug pointer => _3;
+ }
+ }
+ scope 5 (inlined g::{closure#0}) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
+ scope 3 (inlined Pin::<&mut {coroutine@$DIR/inline_coroutine.rs:20:5: 20:8}>::new) {
+ debug pointer => _3;
+ scope 4 (inlined Pin::<&mut {coroutine@$DIR/inline_coroutine.rs:20:5: 20:8}>::new_unchecked) {
+ debug pointer => _3;
+ }
+ }
+ scope 5 (inlined g::{closure#0}) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,48 +7,36 @@ fn vec_deref_to_slice(_1: &Vec<u8>) -> &[u8] {
debug self => _1;
scope 2 (inlined Vec::<u8>::as_slice) {
debug self => _1;
let mut _9: *const u8;
let mut _10: usize;
let mut _7: *const u8;
let mut _8: usize;
scope 3 (inlined Vec::<u8>::as_ptr) {
debug self => _1;
let mut _2: &alloc::raw_vec::RawVec<u8>;
let mut _8: *mut u8;
let mut _6: *mut u8;
scope 4 (inlined alloc::raw_vec::RawVec::<u8>::ptr) {
debug self => _2;
let mut _3: &alloc::raw_vec::RawVecInner;
scope 5 (inlined alloc::raw_vec::RawVecInner::ptr::<u8>) {
debug self => _3;
let mut _7: std::ptr::NonNull<u8>;
let mut _5: std::ptr::NonNull<u8>;
scope 6 (inlined alloc::raw_vec::RawVecInner::non_null::<u8>) {
debug self => _3;
let mut _4: std::ptr::NonNull<u8>;
let mut _2: std::ptr::NonNull<u8>;
scope 7 (inlined Unique::<u8>::cast::<u8>) {
debug ((self: Unique<u8>).0: std::ptr::NonNull<u8>) => _4;
debug ((self: Unique<u8>).1: std::marker::PhantomData<u8>) => const PhantomData::<u8>;
scope 8 (inlined NonNull::<u8>::cast::<u8>) {
debug self => _4;
let mut _5: *mut u8;
let mut _6: *const u8;
let mut _3: *mut u8;
let mut _4: *const u8;
scope 9 (inlined NonNull::<u8>::as_ptr) {
debug self => _4;
}
}
}
scope 10 (inlined Unique::<u8>::as_non_null_ptr) {
debug ((self: Unique<u8>).0: std::ptr::NonNull<u8>) => _7;
debug ((self: Unique<u8>).1: std::marker::PhantomData<u8>) => const PhantomData::<u8>;
}
}
scope 11 (inlined NonNull::<u8>::as_ptr) {
debug self => _7;
}
}
}
}
scope 12 (inlined std::slice::from_raw_parts::<'_, u8>) {
debug data => _9;
debug len => _10;
let _11: *const [u8];
debug data => _7;
debug len => _8;
let _9: *const [u8];
scope 13 (inlined core::ub_checks::check_language_ub) {
scope 14 (inlined core::ub_checks::check_language_ub::runtime) {
}
Expand All @@ -58,49 +46,42 @@ fn vec_deref_to_slice(_1: &Vec<u8>) -> &[u8] {
scope 16 (inlined align_of::<u8>) {
}
scope 17 (inlined slice_from_raw_parts::<u8>) {
debug data => _9;
debug len => _10;
debug data => _7;
debug len => _8;
scope 18 (inlined std::ptr::from_raw_parts::<[u8], u8>) {
debug data_pointer => _9;
debug metadata => _10;
debug data_pointer => _7;
}
}
}
}
}

bb0: {
StorageLive(_8);
StorageLive(_9);
StorageLive(_6);
StorageLive(_7);
StorageLive(_5);
StorageLive(_2);
_2 = &((*_1).0: alloc::raw_vec::RawVec<u8>);
_2 = copy (((((*_1).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
StorageLive(_3);
_3 = &(((*_1).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner);
StorageLive(_7);
StorageLive(_4);
_4 = copy (((((*_1).0: alloc::raw_vec::RawVec<u8>).0: alloc::raw_vec::RawVecInner).0: std::ptr::Unique<u8>).0: std::ptr::NonNull<u8>);
StorageLive(_5);
StorageLive(_6);
_5 = copy _4 as *mut u8 (Transmute);
_6 = copy _5 as *const u8 (PtrToPtr);
_7 = NonNull::<u8> { pointer: move _6 };
StorageDead(_6);
StorageDead(_5);
_3 = copy _2 as *mut u8 (Transmute);
_4 = copy _3 as *const u8 (PtrToPtr);
_5 = NonNull::<u8> { pointer: move _4 };
StorageDead(_4);
_8 = copy _7 as *mut u8 (Transmute);
StorageDead(_7);
StorageDead(_3);
_9 = copy _8 as *const u8 (PtrToPtr);
StorageDead(_2);
StorageLive(_10);
_10 = copy ((*_1).1: usize);
StorageLive(_11);
_11 = *const [u8] from (copy _9, copy _10);
_0 = &(*_11);
StorageDead(_11);
StorageDead(_10);
_6 = copy _5 as *mut u8 (Transmute);
StorageDead(_5);
_7 = copy _6 as *const u8 (PtrToPtr);
StorageLive(_8);
_8 = copy ((*_1).1: usize);
StorageLive(_9);
_9 = *const [u8] from (copy _7, copy _8);
_0 = &(*_9);
StorageDead(_9);
StorageDead(_8);
StorageDead(_7);
StorageDead(_6);
return;
}
}
Loading

0 comments on commit a7fc76a

Please sign in to comment.