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

At debuginfo=0, don't inline debuginfo when inlining #123949

Merged
merged 5 commits into from
Apr 18, 2024
Merged

At debuginfo=0, don't inline debuginfo when inlining #123949

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d05545c
At debuginfo=0, don't inline debuginfo when inlining
scottmcm Apr 15, 2024
6094063
Update `checked_ops` so 32- and 64-bit gets the same checks
scottmcm Apr 16, 2024
90b4c86
Ensure `[rust] debuginfo-level-std` doesn't change core's MIR
scottmcm Apr 18, 2024
b8ac5c0
Update src/bootstrap/src/core/builder.rs
scottmcm Apr 18, 2024
20cf595
The ICE in 121127 needs debuginfo
scottmcm Apr 18, 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
16 changes: 14 additions & 2 deletions compiler/rustc_mir_transform/src/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::TypeVisitableExt;
use rustc_middle::ty::{self, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_session::config::OptLevel;
use rustc_session::config::{DebugInfo, OptLevel};
use rustc_span::source_map::Spanned;
use rustc_span::sym;
use rustc_target::abi::FieldIdx;
Expand Down Expand Up @@ -699,7 +699,19 @@ impl<'tcx> Inliner<'tcx> {
// Insert all of the (mapped) parts of the callee body into the caller.
caller_body.local_decls.extend(callee_body.drain_vars_and_temps());
caller_body.source_scopes.extend(&mut callee_body.source_scopes.drain(..));
caller_body.var_debug_info.append(&mut callee_body.var_debug_info);
if self
.tcx
.sess
.opts
.unstable_opts
.inline_mir_preserve_debug
.unwrap_or(self.tcx.sess.opts.debuginfo != DebugInfo::None)
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't this == DebugInfo::Full? Isn't this current implementation retaining variable debuginfo when compiling with line-tables-only?

Copy link
Contributor

Choose a reason for hiding this comment

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

Before, it was always copied during inlining, so it at least shouldn't be a regression, right? And when the debuginfo is not set to generate variable debuginfo, shouldn't var_debug_info be empty anyway?

Copy link
Member

Choose a reason for hiding this comment

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

so it at least shouldn't be a regression, right?

Yeah, this ought not to be a regression, but I was reading this code and this condition is surprising.

And when the debuginfo is not set to generate variable debuginfo, shouldn't var_debug_info be empty anyway?

When we load optimized_mir for the standard library, it should always have variable debuginfo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see. So, I thought about familiarizing myself a bit with the debuginfo handling, I'll try to send a PR that improves this then.

{
// Note that we need to preserve these in the standard library so that
// people working on rust can build with or without debuginfo while
// still getting consistent results from the mir-opt tests.
caller_body.var_debug_info.append(&mut callee_body.var_debug_info);
}
caller_body.basic_blocks_mut().extend(callee_body.basic_blocks_mut().drain(..));

caller_body[callsite.block].terminator = Some(Terminator {
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_session/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1717,6 +1717,9 @@ options! {
"enable MIR inlining (default: no)"),
inline_mir_hint_threshold: Option<usize> = (None, parse_opt_number, [TRACKED],
"inlining threshold for functions with inline hint (default: 100)"),
inline_mir_preserve_debug: Option<bool> = (None, parse_opt_bool, [TRACKED],
"when MIR inlining, whether to preserve debug info for callee variables \
(default: preserve for debuginfo != None, otherwise remove)"),
inline_mir_threshold: Option<usize> = (None, parse_opt_number, [TRACKED],
"a default MIR inlining threshold (default: 50)"),
input_stats: bool = (false, parse_bool, [UNTRACKED],
Expand Down
8 changes: 8 additions & 0 deletions src/bootstrap/src/core/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2102,6 +2102,14 @@ impl<'a> Builder<'a> {
// break when incremental compilation is enabled. So this overrides the "no inlining
// during incremental builds" heuristic for the standard library.
rustflags.arg("-Zinline-mir");

// FIXME: always pass this after the next `#[cfg(bootstrap)]` update.
if compiler.stage != 0 {
// Similarly, we need to keep debug info for functions inlined into other std functions,
// 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");
}
}

if self.config.rustc_parallel
Expand Down
6 changes: 4 additions & 2 deletions tests/codegen/mem-replace-simple-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,20 @@ pub fn replace_ref_str<'a>(r: &mut &'a str, v: &'a str) -> &'a str {

#[no_mangle]
// CHECK-LABEL: @replace_short_array_3(
// CHECK-SAME: ptr{{.+}}sret{{.+}}%[[RET:.+]], ptr{{.+}}%r, ptr{{.+}}%v
pub fn replace_short_array_3(r: &mut [u32; 3], v: [u32; 3]) -> [u32; 3] {
// CHECK-NOT: alloca
// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %result, ptr align 4 %r, i64 12, i1 false)
// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[RET]], ptr align 4 %r, i64 12, i1 false)
// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %r, ptr align 4 %v, i64 12, i1 false)
std::mem::replace(r, v)
}

#[no_mangle]
// CHECK-LABEL: @replace_short_array_4(
// CHECK-SAME: ptr{{.+}}sret{{.+}}%[[RET:.+]], ptr{{.+}}%r, ptr{{.+}}%v
pub fn replace_short_array_4(r: &mut [u32; 4], v: [u32; 4]) -> [u32; 4] {
// CHECK-NOT: alloca
// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %result, ptr align 4 %r, i64 16, i1 false)
// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %[[RET]], ptr align 4 %r, i64 16, i1 false)
// CHECK: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %r, ptr align 4 %v, i64 16, i1 false)
std::mem::replace(r, v)
}
32 changes: 16 additions & 16 deletions tests/codegen/slice-ref-equality.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,48 +43,48 @@ pub fn is_zero_array(data: &[u8; 4]) -> bool {
// equality for non-byte types also just emit a `bcmp`, not a loop.

// CHECK-LABEL: @eq_slice_of_nested_u8(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
#[no_mangle]
fn eq_slice_of_nested_u8(x: &[[u8; 3]], y: &[[u8; 3]]) -> bool {
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = mul nsw [[USIZE]] %1, 3
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = mul nsw [[USIZE]] {{%x.1|%y.1}}, 3
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
}

// CHECK-LABEL: @eq_slice_of_i32(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
#[no_mangle]
fn eq_slice_of_i32(x: &[i32], y: &[i32]) -> bool {
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] {{%x.1|%y.1}}, 2
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
}

// CHECK-LABEL: @eq_slice_of_nonzero(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
#[no_mangle]
fn eq_slice_of_nonzero(x: &[NonZero<i32>], y: &[NonZero<i32>]) -> bool {
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 2
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] {{%x.1|%y.1}}, 2
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
}

// CHECK-LABEL: @eq_slice_of_option_of_nonzero(
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %1
// CHECK-SAME: [[USIZE]] noundef %3
// CHECK-SAME: [[USIZE:i16|i32|i64]] noundef %x.1
// CHECK-SAME: [[USIZE]] noundef %y.1
#[no_mangle]
fn eq_slice_of_option_of_nonzero(x: &[Option<NonZero<i16>>], y: &[Option<NonZero<i16>>]) -> bool {
// CHECK: icmp eq [[USIZE]] %1, %3
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] %1, 1
// CHECK: icmp eq [[USIZE]] %x.1, %y.1
// CHECK: %[[BYTES:.+]] = shl nsw [[USIZE]] {{%x.1|%y.1}}, 1
// CHECK: tail call{{( noundef)?}} i32 @{{bcmp|memcmp}}(ptr
// CHECK-SAME: , [[USIZE]]{{( noundef)?}} %[[BYTES]])
x == y
Expand Down
2 changes: 0 additions & 2 deletions tests/mir-opt/const_prop/inherit_overflow.main.GVN.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
scope 1 {
}
scope 2 (inlined #[track_caller] <u8 as Add>::add) {
debug self => _2;
debug other => _3;
let mut _4: (u8, bool);
}

Expand Down
2 changes: 0 additions & 2 deletions tests/mir-opt/const_prop/inherit_overflow.main.GVN.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
scope 1 {
}
Comment on lines 9 to 10
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to PR, but this empty scopes looks useless, is they needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

These empty ones that aren't from inlining do seem useless to me, but I don't know.

As you say, not this PR, so can you open an issue or a zulip thread about this? Cleaning up these scopes after SimplifyLocals, or something, does seem like a reasonable option.

scope 2 (inlined #[track_caller] <u8 as Add>::add) {
debug self => _2;
debug other => _3;
let mut _4: (u8, bool);
}

Expand Down
4 changes: 0 additions & 4 deletions ...opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions ...pt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions ...opt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions ...pt/dataflow-const-prop/default_boxed_slice.main.DataflowConstProp.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
4 changes: 0 additions & 4 deletions tests/mir-opt/dataflow-const-prop/default_boxed_slice.main.GVN.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
let _3: std::ptr::Unique<[bool]>;
let mut _4: std::ptr::Unique<[bool; 0]>;
scope 3 {
debug ptr => _3;
}
scope 4 (inlined Unique::<[bool; 0]>::dangling) {
let mut _5: std::ptr::NonNull<[bool; 0]>;
scope 5 (inlined NonNull::<[bool; 0]>::dangling) {
let _6: *mut [bool; 0];
scope 6 {
debug ptr => _6;
scope 10 (inlined NonNull::<[bool; 0]>::new_unchecked) {
debug ptr => _6;
let mut _8: bool;
let _9: ();
let mut _10: *mut ();
Expand All @@ -37,7 +34,6 @@
scope 8 (inlined align_of::<[bool; 0]>) {
}
scope 9 (inlined without_provenance_mut::<[bool; 0]>) {
debug addr => _7;
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions tests/mir-opt/dataflow-const-prop/inherit_overflow.main.DataflowConstProp.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
scope 1 {
}
scope 2 (inlined #[track_caller] <u8 as Add>::add) {
debug self => _2;
debug other => _3;
let mut _4: (u8, bool);
}

Expand Down
2 changes: 0 additions & 2 deletions tests/mir-opt/dataflow-const-prop/inherit_overflow.main.DataflowConstProp.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
scope 1 {
}
scope 2 (inlined #[track_caller] <u8 as Add>::add) {
debug self => _2;
debug other => _3;
let mut _4: (u8, bool);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/dest-prop/union.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// skip-filecheck
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
//! Tests that we can propagate into places that are projections into unions
//@ compile-flags: -Zunsound-mir-opts
//@ compile-flags: -Zunsound-mir-opts -C debuginfo=full
fn val() -> u32 {
1
}
Expand Down
Loading
Loading