Skip to content

Introduce debuginfo to statements in MIR #142771

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/mir/terminator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,14 @@ impl<'tcx> Terminator<'tcx> {
self.kind.successors()
}

/// Return `Some` if all successors are identical.
#[inline]
pub fn identical_successor(&self) -> Option<BasicBlock> {
let mut successors = self.successors();
let first_succ = successors.next()?;
if successors.all(|succ| first_succ == succ) { Some(first_succ) } else { None }
}

#[inline]
pub fn successors_mut<'a>(&'a mut self, f: impl FnMut(&'a mut BasicBlock)) {
self.kind.successors_mut(f)
Expand Down
30 changes: 27 additions & 3 deletions compiler/rustc_mir_transform/src/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
// statements itself to avoid moving the (relatively) large statements twice.
// We do not push the statements directly into the target block (`bb`) as that is slower
// due to additional reallocations
let mut merged_blocks = Vec::new();
let mut merged_blocks: Vec<BasicBlock> = Vec::new();
let mut outer_changed = false;
loop {
let mut changed = false;
Expand All @@ -158,8 +158,17 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
let mut terminator =
self.basic_blocks[bb].terminator.take().expect("invalid terminator state");

terminator
.successors_mut(|successor| self.collapse_goto_chain(successor, &mut changed));
let identical_succ = terminator.identical_successor();
terminator.successors_mut(|successor| {
self.collapse_goto_chain(successor, &mut changed);
});
if changed && let Some(identical_succ) = identical_succ {
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 introduce a more localized chain_changed variable?

// Add debugging information from the goto chain only when all successors are identical,
// otherwise, we may provide misleading debugging information within a branch.
let mut succ_debuginfos =
self.basic_blocks[identical_succ].after_last_stmt_debuginfos.clone();
self.basic_blocks[bb].after_last_stmt_debuginfos.append(&mut succ_debuginfos);
}

let mut inner_changed = true;
merged_blocks.clear();
Expand All @@ -176,10 +185,18 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
if statements_to_merge > 0 {
let mut statements = std::mem::take(&mut self.basic_blocks[bb].statements);
statements.reserve(statements_to_merge);
let mut parent_bb_last_debuginfos =
std::mem::take(&mut self.basic_blocks[bb].after_last_stmt_debuginfos);
for &from in &merged_blocks {
if let Some(stmt) = self.basic_blocks[from].statements.first_mut() {
stmt.debuginfos.prepend(&mut parent_bb_last_debuginfos);
}
statements.append(&mut self.basic_blocks[from].statements);
parent_bb_last_debuginfos =
std::mem::take(&mut self.basic_blocks[from].after_last_stmt_debuginfos);
}
self.basic_blocks[bb].statements = statements;
self.basic_blocks[bb].after_last_stmt_debuginfos = parent_bb_last_debuginfos;
}

self.basic_blocks[bb].terminator = Some(terminator);
Expand Down Expand Up @@ -229,11 +246,18 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
let last = current;
*changed |= *start != last;
*start = last;
let mut succ = last;
while let Some((current, mut terminator)) = terminators.pop() {
let Terminator { kind: TerminatorKind::Goto { ref mut target }, .. } = terminator
else {
unreachable!();
};
if *target != last {
let mut succ_debuginfos =
self.basic_blocks[succ].after_last_stmt_debuginfos.clone();
self.basic_blocks[current].after_last_stmt_debuginfos.extend(&mut succ_debuginfos);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to look at pred_count to avoid a clone?

succ = current;
*changed |= *target != last;
*target = last;
debug!("collapsing goto chain from {:?} to {:?}", current, target);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@
-
- bb1: {
- // DBG: _3 = &((*_1).0: i32);
- nop;
- goto -> bb2;
- }
-
- bb2: {
// DBG: _4 = &((*_1).1: i64);
- nop;
_0 = copy ((*_1).2: i32);
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@
- bb1: {
(*_2) = const true;
// DBG: _3 = &((*_1).0: i32);
- nop;
- goto -> bb2;
- }
-
- bb2: {
// DBG: _4 = &((*_1).1: i64);
- nop;
_0 = copy ((*_1).2: i32);
// DBG: _5 = &((*_1).2: i32);
- nop;
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@
-
- bb1: {
// DBG: _2 = &((*_1).0: i32);
- nop;
- goto -> bb2;
- }
-
- bb2: {
// DBG: _3 = &((*_1).1: i64);
- nop;
_0 = copy ((*_1).2: i32);
// DBG: _4 = &((*_1).2: i32);
- nop;
return;
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
- // MIR for `preserve_debuginfo_identical_succs` before SimplifyCfg-final
+ // MIR for `preserve_debuginfo_identical_succs` after SimplifyCfg-final

fn preserve_debuginfo_identical_succs(_1: &Foo, _2: bool) -> i32 {
debug foo_a => _3;
debug foo_b => _4;
debug foo_c => _5;
let mut _0: i32;
let mut _3: &i32;
let mut _4: &i64;
let mut _5: &i32;

bb0: {
- switchInt(copy _2) -> [1: bb1, otherwise: bb1];
- }
-
- bb1: {
// DBG: _3 = &((*_1).0: i32);
- goto -> bb2;
- }
-
- bb2: {
// DBG: _4 = &((*_1).1: i64);
_0 = copy ((*_1).2: i32);
// DBG: _5 = &((*_1).2: i32);
return;
}
}

159 changes: 159 additions & 0 deletions tests/mir-opt/debuginfo/simplifycfg.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
//@ test-mir-pass: SimplifyCfg-final
//@ compile-flags: -Zmir-enable-passes=+DeadStoreElimination-initial

#![feature(core_intrinsics, custom_mir)]
#![crate_type = "lib"]

use std::intrinsics::mir::*;

pub struct Foo {
a: i32,
b: i64,
c: i32,
}

// EMIT_MIR simplifycfg.drop_debuginfo.SimplifyCfg-final.diff
#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
pub fn drop_debuginfo(foo: &Foo, c: bool) -> i32 {
// CHECK-LABEL: fn drop_debuginfo
// CHECK: debug foo_b => [[foo_b:_[0-9]+]];
// CHECK: bb0: {
// CHECK-NEXT: DBG: [[foo_b]] = &((*_1).1: i64)
// CHECK-NEXT: _0 = copy ((*_1).2: i32);
// CHECK-NEXT: return;
mir! {
let _foo_a: &i32;
let _foo_b: &i64;
debug foo_a => _foo_a;
debug foo_b => _foo_b;
{
match c {
true => tmp,
_ => ret,
}
}
tmp = {
// Because we don't know if `c` is always true, we must drop this debuginfo.
_foo_a = &(*foo).a;
Goto(ret)
}
ret = {
_foo_b = &(*foo).b;
RET = (*foo).c;
Return()
}
}
}

// EMIT_MIR simplifycfg.preserve_debuginfo_1.SimplifyCfg-final.diff
#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
pub fn preserve_debuginfo_1(foo: &Foo, v: &mut bool) -> i32 {
// CHECK-LABEL: fn preserve_debuginfo_1
// CHECK: debug foo_a => [[foo_a:_[0-9]+]];
// CHECK: debug foo_b => [[foo_b:_[0-9]+]];
// CHECK: debug foo_c => [[foo_c:_[0-9]+]];
// CHECK: bb0: {
// CHECK-NEXT: (*_2) = const true;
// CHECK-NEXT: DBG: [[foo_a]] = &((*_1).0: i32)
// CHECK-NEXT: DBG: [[foo_b]] = &((*_1).1: i64)
// CHECK-NEXT: _0 = copy ((*_1).2: i32);
// CHECK-NEXT: DBG: [[foo_c]] = &((*_1).2: i32)
// CHECK-NEXT: return;
mir! {
let _foo_a: &i32;
let _foo_b: &i64;
let _foo_c: &i32;
debug foo_a => _foo_a;
debug foo_b => _foo_b;
debug foo_c => _foo_c;
{
Goto(tmp)
}
tmp = {
*v = true;
_foo_a = &(*foo).a;
Goto(ret)
}
ret = {
_foo_b = &(*foo).b;
RET = (*foo).c;
_foo_c = &(*foo).c;
Return()
}
}
}

// EMIT_MIR simplifycfg.preserve_debuginfo_2.SimplifyCfg-final.diff
#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
pub fn preserve_debuginfo_2(foo: &Foo) -> i32 {
// CHECK-LABEL: fn preserve_debuginfo_2
// CHECK: debug foo_a => [[foo_a:_[0-9]+]];
// CHECK: debug foo_b => [[foo_b:_[0-9]+]];
// CHECK: debug foo_c => [[foo_c:_[0-9]+]];
// CHECK: bb0: {
// CHECK-NEXT: DBG: [[foo_a]] = &((*_1).0: i32)
// CHECK-NEXT: DBG: [[foo_b]] = &((*_1).1: i64)
// CHECK-NEXT: _0 = copy ((*_1).2: i32);
// CHECK-NEXT: DBG: [[foo_c]] = &((*_1).2: i32)
// CHECK-NEXT: return;
mir! {
let _foo_a: &i32;
let _foo_b: &i64;
let _foo_c: &i32;
debug foo_a => _foo_a;
debug foo_b => _foo_b;
debug foo_c => _foo_c;
{
Goto(tmp)
}
tmp = {
_foo_a = &(*foo).a;
Goto(ret)
}
ret = {
_foo_b = &(*foo).b;
RET = (*foo).c;
_foo_c = &(*foo).c;
Return()
}
}
}

// EMIT_MIR simplifycfg.preserve_debuginfo_identical_succs.SimplifyCfg-final.diff
#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
pub fn preserve_debuginfo_identical_succs(foo: &Foo, c: bool) -> i32 {
// CHECK-LABEL: fn preserve_debuginfo_identical_succs
// CHECK: debug foo_a => [[foo_a:_[0-9]+]];
// CHECK: debug foo_b => [[foo_b:_[0-9]+]];
// CHECK: debug foo_c => [[foo_c:_[0-9]+]];
// CHECK: bb0: {
// CHECK-NEXT: DBG: [[foo_a]] = &((*_1).0: i32)
// CHECK-NEXT: DBG: [[foo_b]] = &((*_1).1: i64)
// CHECK-NEXT: _0 = copy ((*_1).2: i32);
// CHECK-NEXT: DBG: [[foo_c]] = &((*_1).2: i32)
// CHECK-NEXT: return;
mir! {
let _foo_a: &i32;
let _foo_b: &i64;
let _foo_c: &i32;
debug foo_a => _foo_a;
debug foo_b => _foo_b;
debug foo_c => _foo_c;
{
match c {
true => tmp,
_ => tmp,
}
}
tmp = {
_foo_a = &(*foo).a;
Goto(ret)
}
ret = {
_foo_b = &(*foo).b;
RET = (*foo).c;
_foo_c = &(*foo).c;
Return()
}
}
}