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

[nothing to see here] this is probably a bad idea but I'm curious #137702

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Changes from all commits
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
86 changes: 86 additions & 0 deletions compiler/rustc_mir_transform/src/branch_duplicator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use std::mem;

use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
use tracing::{debug, instrument, trace};

pub(super) struct BranchDuplicator;

impl<'tcx> crate::MirPass<'tcx> for BranchDuplicator {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
sess.mir_opt_level() >= 2
}

#[instrument(skip_all level = "debug")]
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let def_id = body.source.def_id();
debug!(?def_id);

// Optimizing coroutines creates query cycles.
if tcx.is_coroutine(def_id) {
trace!("Skipped for coroutine {:?}", def_id);
return;
}

let is_branch = |targets: &SwitchTargets| {
targets.all_targets().len() == 2
|| (targets.all_values().len() == 2
&& body.basic_blocks[targets.otherwise()].is_empty_unreachable())
};

let mut candidates = Vec::new();
for (bb, bbdata) in body.basic_blocks.iter_enumerated() {
if let TerminatorKind::SwitchInt { targets, .. } = &bbdata.terminator().kind
&& is_branch(targets)
&& let Ok(preds) =
<[BasicBlock; 2]>::try_from(body.basic_blocks.predecessors()[bb].as_slice())
&& preds.iter().copied().all(|p| {
matches!(body.basic_blocks[p].terminator().kind, TerminatorKind::Goto { .. })
})
&& bbdata.statements.iter().all(|x| is_negligible(&x.kind))
{
candidates.push((bb, preds));
}
}

if candidates.is_empty() {
return;
}

let basic_blocks = body.basic_blocks.as_mut();
for (bb, [p0, p1]) in candidates {
let bbdata = &mut basic_blocks[bb];
let statements = mem::take(&mut bbdata.statements);
let unreachable = Terminator {
source_info: bbdata.terminator().source_info,
kind: TerminatorKind::Unreachable,
};
let terminator = mem::replace(bbdata.terminator_mut(), unreachable);

let pred0data = &mut basic_blocks[p0];
pred0data.statements.extend(statements.iter().cloned());
*pred0data.terminator_mut() = terminator.clone();

let pred1data = &mut basic_blocks[p1];
pred1data.statements.extend(statements);
*pred1data.terminator_mut() = terminator;
}
}

fn is_required(&self) -> bool {
false
}
}

fn is_negligible<'tcx>(stmt: &StatementKind<'tcx>) -> bool {
use Rvalue::*;
use StatementKind::*;
match stmt {
StorageLive(..) | StorageDead(..) => true,
Assign(place_and_rvalue) => match &place_and_rvalue.1 {
Ref(..) | RawPtr(..) | Discriminant(..) | NullaryOp(..) => true,
_ => false,
},
_ => false,
}
}
4 changes: 4 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
@@ -118,6 +118,7 @@ declare_passes! {
mod add_moves_for_packed_drops : AddMovesForPackedDrops;
mod add_retag : AddRetag;
mod add_subtyping_projections : Subtyper;
mod branch_duplicator: BranchDuplicator;
mod check_inline : CheckForceInline;
mod check_call_recursion : CheckCallRecursion, CheckDropRecursion;
mod check_alignment : CheckAlignment;
@@ -670,6 +671,9 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&unreachable_enum_branching::UnreachableEnumBranching,
&unreachable_prop::UnreachablePropagation,
&o1(simplify::SimplifyCfg::AfterUnreachableEnumBranching),
// If we inlined something returning `Option` or `Result`,
// duplicating the discriminant branches on those helps later passes.
&branch_duplicator::BranchDuplicator,
// Inlining may have introduced a lot of redundant code and a large move pattern.
// Now, we need to shrink the generated MIR.
&ref_prop::ReferencePropagation,
21 changes: 6 additions & 15 deletions tests/mir-opt/const_goto_const_eval_fail.f.JumpThreading.diff
Original file line number Diff line number Diff line change
@@ -12,36 +12,27 @@

bb1: {
_1 = const true;
- goto -> bb3;
+ goto -> bb7;
goto -> bb3;
}

bb2: {
_1 = const B;
goto -> bb3;
switchInt(copy _1) -> [0: bb4, otherwise: bb3];
}

bb3: {
switchInt(copy _1) -> [0: bb5, otherwise: bb4];
}

bb4: {
_0 = const 2_u64;
goto -> bb6;
goto -> bb5;
}

bb5: {
bb4: {
_0 = const 1_u64;
goto -> bb6;
goto -> bb5;
}

bb6: {
bb5: {
StorageDead(_1);
return;
+ }
+
+ bb7: {
+ goto -> bb4;
}
}

27 changes: 26 additions & 1 deletion tests/mir-opt/pre-codegen/checked_ops.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// skip-filecheck
//@ compile-flags: -O -Zmir-opt-level=2
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY

@@ -8,10 +7,36 @@
// EMIT_MIR checked_ops.step_forward.PreCodegen.after.mir
pub fn step_forward(x: u16, n: usize) -> u16 {
// This uses `u16` so that the conversion to usize is always widening.

// CHECK-LABEL: fn step_forward
// CHECK: inlined{{.+}}forward
std::iter::Step::forward(x, n)
}

// EMIT_MIR checked_ops.checked_shl.PreCodegen.after.mir
pub fn checked_shl(x: u32, rhs: u32) -> Option<u32> {
// CHECK-LABEL: fn checked_shl
// CHECK: [[TEMP:_[0-9]+]] = ShlUnchecked(copy _1, copy _2)
// CHECK: _0 = Option::<u32>::Some({{move|copy}} [[TEMP]])
x.checked_shl(rhs)
}

// EMIT_MIR checked_ops.use_checked_sub.PreCodegen.after.mir
// EMIT_MIR checked_ops.use_checked_sub.BranchDuplicator.diff
// EMIT_MIR checked_ops.use_checked_sub.GVN.diff
pub fn use_checked_sub(x: u32, rhs: u32) {
// We want this to be equivalent to open-coding it, leaving no `Option`s around.

// CHECK-LABEL: fn use_checked_sub
// FIXME-CHECK-NOT: let{{.+}}Option
// CHECK: inlined{{.+}}u32{{.+}}checked_sub
// CHECK: [[DELTA:_[0-9]+]] = SubUnchecked(copy _1, copy _2)
// FIXME-CHECK: do_something({{move|copy}} [[DELTA]])
if let Some(delta) = x.checked_sub(rhs) {
do_something(delta);
}
}

unsafe extern "Rust" {
safe fn do_something(_: u32);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
- // MIR for `use_checked_sub` before BranchDuplicator
+ // MIR for `use_checked_sub` after BranchDuplicator

fn use_checked_sub(_1: u32, _2: u32) -> () {
debug x => _1;
debug rhs => _2;
let mut _0: ();
let mut _3: std::option::Option<u32>;
let mut _4: u32;
let mut _5: u32;
let mut _6: isize;
let _8: ();
let mut _9: u32;
scope 1 {
debug delta => _7;
let _7: u32;
scope 2 (inlined core::num::<impl u32>::checked_sub) {
let mut _10: bool;
let mut _11: u32;
}
}

bb0: {
StorageLive(_3);
StorageLive(_4);
_4 = copy _1;
StorageLive(_5);
_5 = copy _2;
StorageLive(_10);
_10 = Lt(copy _4, copy _5);
switchInt(move _10) -> [0: bb5, otherwise: bb4];
}

bb1: {
StorageLive(_7);
_7 = copy ((_3 as Some).0: u32);
StorageLive(_9);
_9 = copy _7;
_8 = do_something(move _9) -> [return: bb2, unwind unreachable];
}

bb2: {
StorageDead(_9);
StorageDead(_7);
goto -> bb3;
}

bb3: {
StorageDead(_3);
return;
}

bb4: {
_3 = const Option::<u32>::None;
- goto -> bb6;
+ StorageDead(_10);
+ StorageDead(_5);
+ StorageDead(_4);
+ _6 = discriminant(_3);
+ switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
}

bb5: {
StorageLive(_11);
_11 = SubUnchecked(copy _4, copy _5);
_3 = Option::<u32>::Some(move _11);
StorageDead(_11);
- goto -> bb6;
- }
-
- bb6: {
StorageDead(_10);
StorageDead(_5);
StorageDead(_4);
_6 = discriminant(_3);
switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
+ }
+
+ bb6: {
+ unreachable;
}

bb7: {
unreachable;
}
}

ALLOC0 (size: 8, align: 4) {
00 00 00 00 __ __ __ __ │ ....░░░░
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
- // MIR for `use_checked_sub` before BranchDuplicator
+ // MIR for `use_checked_sub` after BranchDuplicator

fn use_checked_sub(_1: u32, _2: u32) -> () {
debug x => _1;
debug rhs => _2;
let mut _0: ();
let mut _3: std::option::Option<u32>;
let mut _4: u32;
let mut _5: u32;
let mut _6: isize;
let _8: ();
let mut _9: u32;
scope 1 {
debug delta => _7;
let _7: u32;
scope 2 (inlined core::num::<impl u32>::checked_sub) {
let mut _10: bool;
let mut _11: u32;
}
}

bb0: {
StorageLive(_3);
StorageLive(_4);
_4 = copy _1;
StorageLive(_5);
_5 = copy _2;
StorageLive(_10);
_10 = Lt(copy _4, copy _5);
switchInt(move _10) -> [0: bb5, otherwise: bb4];
}

bb1: {
StorageLive(_7);
_7 = copy ((_3 as Some).0: u32);
StorageLive(_9);
_9 = copy _7;
_8 = do_something(move _9) -> [return: bb2, unwind continue];
}

bb2: {
StorageDead(_9);
StorageDead(_7);
goto -> bb3;
}

bb3: {
StorageDead(_3);
return;
}

bb4: {
_3 = const Option::<u32>::None;
- goto -> bb6;
+ StorageDead(_10);
+ StorageDead(_5);
+ StorageDead(_4);
+ _6 = discriminant(_3);
+ switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
}

bb5: {
StorageLive(_11);
_11 = SubUnchecked(copy _4, copy _5);
_3 = Option::<u32>::Some(move _11);
StorageDead(_11);
- goto -> bb6;
- }
-
- bb6: {
StorageDead(_10);
StorageDead(_5);
StorageDead(_4);
_6 = discriminant(_3);
switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
+ }
+
+ bb6: {
+ unreachable;
}

bb7: {
unreachable;
}
}

ALLOC0 (size: 8, align: 4) {
00 00 00 00 __ __ __ __ │ ....░░░░
}

Loading
Loading