Skip to content

Commit 2f00e86

Browse files
committed
Introduce MIR optimizations for simplifying x? on Results.
This optimization depends on inlining for the identity conversions introduced by the lowering of the `?`. To take advantage of `SimplifyArmIdentity`, `-Z mir-opt-level=2` is required because that triggers the inlining MIR optimization.
1 parent 35ef33a commit 2f00e86

File tree

11 files changed

+432
-12
lines changed

11 files changed

+432
-12
lines changed

Cargo.lock

+1
Original file line numberDiff line numberDiff line change
@@ -3709,6 +3709,7 @@ dependencies = [
37093709
"arena",
37103710
"either",
37113711
"graphviz",
3712+
"itertools 0.8.0",
37123713
"log",
37133714
"log_settings",
37143715
"polonius-engine",

src/librustc/hir/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2052,7 +2052,7 @@ pub enum TyKind {
20522052
Err,
20532053
}
20542054

2055-
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
2055+
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable, PartialEq)]
20562056
pub struct InlineAsmOutput {
20572057
pub constraint: Symbol,
20582058
pub is_rw: bool,
@@ -2062,7 +2062,7 @@ pub struct InlineAsmOutput {
20622062

20632063
// NOTE(eddyb) This is used within MIR as well, so unlike the rest of the HIR,
20642064
// it needs to be `Clone` and use plain `Vec<T>` instead of `HirVec<T>`.
2065-
#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
2065+
#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable, PartialEq)]
20662066
pub struct InlineAsmInner {
20672067
pub asm: Symbol,
20682068
pub asm_str_style: StrStyle,

src/librustc/mir/interpret/error.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ impl<'tcx> From<InterpError<'tcx>> for InterpErrorInfo<'tcx> {
248248
}
249249
}
250250

251-
#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)]
251+
#[derive(Clone, RustcEncodable, RustcDecodable, HashStable, PartialEq)]
252252
pub enum PanicInfo<O> {
253253
Panic {
254254
msg: Symbol,

src/librustc/mir/mod.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -999,7 +999,7 @@ pub struct Terminator<'tcx> {
999999
pub kind: TerminatorKind<'tcx>,
10001000
}
10011001

1002-
#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)]
1002+
#[derive(Clone, RustcEncodable, RustcDecodable, HashStable, PartialEq)]
10031003
pub enum TerminatorKind<'tcx> {
10041004
/// Block should have one successor in the graph; we jump there.
10051005
Goto { target: BasicBlock },
@@ -1528,7 +1528,7 @@ impl Statement<'_> {
15281528
}
15291529
}
15301530

1531-
#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
1531+
#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
15321532
pub enum StatementKind<'tcx> {
15331533
/// Write the RHS Rvalue to the LHS Place.
15341534
Assign(Box<(Place<'tcx>, Rvalue<'tcx>)>),
@@ -1594,7 +1594,7 @@ pub enum RetagKind {
15941594
}
15951595

15961596
/// The `FakeReadCause` describes the type of pattern why a FakeRead statement exists.
1597-
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
1597+
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable, PartialEq)]
15981598
pub enum FakeReadCause {
15991599
/// Inject a fake read of the borrowed input at the end of each guards
16001600
/// code.
@@ -1636,7 +1636,7 @@ pub enum FakeReadCause {
16361636
ForIndex,
16371637
}
16381638

1639-
#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
1639+
#[derive(Clone, Debug, PartialEq, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
16401640
pub struct InlineAsm<'tcx> {
16411641
pub asm: hir::InlineAsmInner,
16421642
pub outputs: Box<[Place<'tcx>]>,
@@ -2068,7 +2068,7 @@ impl<'tcx> Operand<'tcx> {
20682068
///////////////////////////////////////////////////////////////////////////
20692069
/// Rvalues
20702070
2071-
#[derive(Clone, RustcEncodable, RustcDecodable, HashStable)]
2071+
#[derive(Clone, RustcEncodable, RustcDecodable, HashStable, PartialEq)]
20722072
pub enum Rvalue<'tcx> {
20732073
/// x (either a move or copy, depending on type of x)
20742074
Use(Operand<'tcx>),
@@ -2444,7 +2444,7 @@ impl<'tcx> UserTypeProjections {
24442444
/// * `let (x, _): T = ...` -- here, the `projs` vector would contain
24452445
/// `field[0]` (aka `.0`), indicating that the type of `s` is
24462446
/// determined by finding the type of the `.0` field from `T`.
2447-
#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable)]
2447+
#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, PartialEq)]
24482448
pub struct UserTypeProjection {
24492449
pub base: UserTypeAnnotationIndex,
24502450
pub projs: Vec<ProjectionKind>,

src/librustc_mir/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ doctest = false
1313
arena = { path = "../libarena" }
1414
either = "1.5.0"
1515
dot = { path = "../libgraphviz", package = "graphviz" }
16+
itertools = "0.8"
1617
log = "0.4"
1718
log_settings = "0.1.1"
1819
polonius-engine = "0.10.0"

src/librustc_mir/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
1616
#![feature(decl_macro)]
1717
#![feature(drain_filter)]
1818
#![feature(exhaustive_patterns)]
19+
#![feature(iter_order_by)]
1920
#![cfg_attr(bootstrap, feature(never_type))]
2021
#![feature(specialization)]
2122
#![feature(try_trait)]

src/librustc_mir/transform/mod.rs

+4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ pub mod cleanup_post_borrowck;
1818
pub mod check_consts;
1919
pub mod check_unsafety;
2020
pub mod simplify_branches;
21+
pub mod simplify_try;
2122
pub mod simplify;
2223
pub mod erase_regions;
2324
pub mod no_landing_pads;
@@ -305,6 +306,9 @@ fn run_optimization_passes<'tcx>(
305306
&copy_prop::CopyPropagation,
306307
&simplify_branches::SimplifyBranches::new("after-copy-prop"),
307308
&remove_noop_landing_pads::RemoveNoopLandingPads,
309+
&simplify::SimplifyCfg::new("after-remove-noop-landing-pads"),
310+
&simplify_try::SimplifyArmIdentity,
311+
&simplify_try::SimplifyBranchSame,
308312
&simplify::SimplifyCfg::new("final"),
309313
&simplify::SimplifyLocals,
310314

+201
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,201 @@
1+
//! The general point of the optimizations provided here is to simplify something like:
2+
//!
3+
//! ```rust
4+
//! match x {
5+
//! Ok(x) => Ok(x),
6+
//! Err(x) => Err(x)
7+
//! }
8+
//! ```
9+
//!
10+
//! into just `x`.
11+
12+
use crate::transform::{MirPass, MirSource, simplify};
13+
use rustc::ty::{TyCtxt, Ty};
14+
use rustc::mir::*;
15+
use rustc_target::abi::VariantIdx;
16+
use itertools::Itertools as _;
17+
18+
/// Simplifies arms of form `Variant(x) => Variant(x)` to just a move.
19+
///
20+
/// This is done by transforming basic blocks where the statements match:
21+
///
22+
/// ```rust
23+
/// _LOCAL_TMP = ((_LOCAL_1 as Variant ).FIELD: TY );
24+
/// ((_LOCAL_0 as Variant).FIELD: TY) = move _LOCAL_TMP;
25+
/// discriminant(_LOCAL_0) = VAR_IDX;
26+
/// ```
27+
///
28+
/// into:
29+
///
30+
/// ```rust
31+
/// _LOCAL_0 = move _LOCAL_1
32+
/// ```
33+
pub struct SimplifyArmIdentity;
34+
35+
impl<'tcx> MirPass<'tcx> for SimplifyArmIdentity {
36+
fn run_pass(&self, _: TyCtxt<'tcx>, _: MirSource<'tcx>, body: &mut Body<'tcx>) {
37+
for bb in body.basic_blocks_mut() {
38+
// Need 3 statements:
39+
let (s0, s1, s2) = match &mut *bb.statements {
40+
[s0, s1, s2] => (s0, s1, s2),
41+
_ => continue,
42+
};
43+
44+
// Pattern match on the form we want:
45+
let (local_tmp_s0, local_1, vf_s0) = match match_get_variant_field(s0) {
46+
None => continue,
47+
Some(x) => x,
48+
};
49+
let (local_tmp_s1, local_0, vf_s1) = match match_set_variant_field(s1) {
50+
None => continue,
51+
Some(x) => x,
52+
};
53+
if local_tmp_s0 != local_tmp_s1
54+
|| vf_s0 != vf_s1
55+
|| Some((local_0, vf_s0.var_idx)) != match_set_discr(s2)
56+
{
57+
continue;
58+
}
59+
60+
// Right shape; transform!
61+
match &mut s0.kind {
62+
StatementKind::Assign(box (place, rvalue)) => {
63+
*place = local_0.into();
64+
*rvalue = Rvalue::Use(Operand::Move(local_1.into()));
65+
}
66+
_ => unreachable!(),
67+
}
68+
s1.make_nop();
69+
s2.make_nop();
70+
}
71+
}
72+
}
73+
74+
/// Match on:
75+
/// ```rust
76+
/// _LOCAL_INTO = ((_LOCAL_FROM as Variant).FIELD: TY);
77+
/// ```
78+
fn match_get_variant_field<'tcx>(stmt: &Statement<'tcx>) -> Option<(Local, Local, VarField<'tcx>)> {
79+
match &stmt.kind {
80+
StatementKind::Assign(box (place_into, rvalue_from)) => match rvalue_from {
81+
Rvalue::Use(Operand::Copy(pf)) | Rvalue::Use(Operand::Move(pf)) => {
82+
let local_into = place_into.as_local()?;
83+
let (local_from, vf) = match_variant_field_place(&pf)?;
84+
Some((local_into, local_from, vf))
85+
}
86+
_ => None,
87+
},
88+
_ => None,
89+
}
90+
}
91+
92+
/// Match on:
93+
/// ```rust
94+
/// ((_LOCAL_FROM as Variant).FIELD: TY) = move _LOCAL_INTO;
95+
/// ```
96+
fn match_set_variant_field<'tcx>(stmt: &Statement<'tcx>) -> Option<(Local, Local, VarField<'tcx>)> {
97+
match &stmt.kind {
98+
StatementKind::Assign(box (place_from, rvalue_into)) => match rvalue_into {
99+
Rvalue::Use(Operand::Move(place_into)) => {
100+
let local_into = place_into.as_local()?;
101+
let (local_from, vf) = match_variant_field_place(&place_from)?;
102+
Some((local_into, local_from, vf))
103+
}
104+
_ => None,
105+
},
106+
_ => None,
107+
}
108+
}
109+
110+
/// Match on:
111+
/// ```rust
112+
/// discriminant(_LOCAL_TO_SET) = VAR_IDX;
113+
/// ```
114+
fn match_set_discr<'tcx>(stmt: &Statement<'tcx>) -> Option<(Local, VariantIdx)> {
115+
match &stmt.kind {
116+
StatementKind::SetDiscriminant { place, variant_index } => Some((
117+
place.as_local()?,
118+
*variant_index
119+
)),
120+
_ => None,
121+
}
122+
}
123+
124+
#[derive(PartialEq)]
125+
struct VarField<'tcx> {
126+
field: Field,
127+
field_ty: Ty<'tcx>,
128+
var_idx: VariantIdx,
129+
}
130+
131+
/// Match on `((_LOCAL as Variant).FIELD: TY)`.
132+
fn match_variant_field_place<'tcx>(place: &Place<'tcx>) -> Option<(Local, VarField<'tcx>)> {
133+
match place.as_ref() {
134+
PlaceRef {
135+
base: &PlaceBase::Local(local),
136+
projection: &[ProjectionElem::Downcast(_, var_idx), ProjectionElem::Field(field, ty)],
137+
} => Some((local, VarField { field, field_ty: ty, var_idx })),
138+
_ => None,
139+
}
140+
}
141+
142+
/// Simplifies `SwitchInt(_) -> [targets]`,
143+
/// where all the `targets` have the same form,
144+
/// into `goto -> target_first`.
145+
pub struct SimplifyBranchSame;
146+
147+
impl<'tcx> MirPass<'tcx> for SimplifyBranchSame {
148+
fn run_pass(&self, _: TyCtxt<'tcx>, _: MirSource<'tcx>, body: &mut Body<'tcx>) {
149+
let mut did_remove_blocks = false;
150+
let bbs = body.basic_blocks_mut();
151+
for bb_idx in bbs.indices() {
152+
let targets = match &bbs[bb_idx].terminator().kind {
153+
TerminatorKind::SwitchInt { targets, .. } => targets,
154+
_ => continue,
155+
};
156+
157+
let mut iter_bbs_reachable = targets
158+
.iter()
159+
.map(|idx| (*idx, &bbs[*idx]))
160+
.filter(|(_, bb)| {
161+
// Reaching `unreachable` is UB so assume it doesn't happen.
162+
bb.terminator().kind != TerminatorKind::Unreachable
163+
// But `asm!(...)` could abort the program,
164+
// so we cannot assume that the `unreachable` terminator itself is reachable.
165+
// FIXME(Centril): use a normalization pass instead of a check.
166+
|| bb.statements.iter().any(|stmt| match stmt.kind {
167+
StatementKind::InlineAsm(..) => true,
168+
_ => false,
169+
})
170+
})
171+
.peekable();
172+
173+
// We want to `goto -> bb_first`.
174+
let bb_first = iter_bbs_reachable
175+
.peek()
176+
.map(|(idx, _)| *idx)
177+
.unwrap_or(targets[0]);
178+
179+
// All successor basic blocks should have the exact same form.
180+
let all_successors_equivalent = iter_bbs_reachable
181+
.map(|(_, bb)| bb)
182+
.tuple_windows()
183+
.all(|(bb_l, bb_r)| {
184+
bb_l.is_cleanup == bb_r.is_cleanup
185+
&& bb_l.terminator().kind == bb_r.terminator().kind
186+
&& bb_l.statements.iter().eq_by(&bb_r.statements, |x, y| x.kind == y.kind)
187+
});
188+
189+
if all_successors_equivalent {
190+
// Replace `SwitchInt(..) -> [bb_first, ..];` with a `goto -> bb_first;`.
191+
bbs[bb_idx].terminator_mut().kind = TerminatorKind::Goto { target: bb_first };
192+
did_remove_blocks = true;
193+
}
194+
}
195+
196+
if did_remove_blocks {
197+
// We have dead blocks now, so remove those.
198+
simplify::remove_dead_blocks(body);
199+
}
200+
}
201+
}

src/test/codegen/match.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,21 @@ pub enum E {
99

1010
// CHECK-LABEL: @exhaustive_match
1111
#[no_mangle]
12-
pub fn exhaustive_match(e: E, unit: ()) {
12+
pub fn exhaustive_match(e: E) -> u8 {
1313
// CHECK: switch{{.*}}, label %[[OTHERWISE:[a-zA-Z0-9_]+]] [
1414
// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]]
1515
// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]]
1616
// CHECK-NEXT: ]
1717
// CHECK: [[B]]:
18+
// CHECK-NEXT: store i8 1, i8* %1, align 1
1819
// CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]]
1920
// CHECK: [[OTHERWISE]]:
2021
// CHECK-NEXT: unreachable
2122
// CHECK: [[A]]:
23+
// CHECK-NEXT: store i8 0, i8* %1, align 1
2224
// CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]]
2325
match e {
24-
E::A => unit,
25-
E::B => unit,
26+
E::A => 0,
27+
E::B => 1,
2628
}
2729
}

src/test/codegen/try_identity.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// compile-flags: -C no-prepopulate-passes -Z mir-opt-level=2
2+
3+
// Ensure that `x?` has no overhead on `Result<T, E>` due to identity `match`es in lowering.
4+
// This requires inlining to trigger the MIR optimizations in `SimplifyArmIdentity`.
5+
6+
#![crate_type = "lib"]
7+
8+
type R = Result<u64, i32>;
9+
10+
#[no_mangle]
11+
fn try_identity(x: R) -> R {
12+
// CHECK: start:
13+
// CHECK-NOT: br {{.*}}
14+
// CHECK ret void
15+
let y = x?;
16+
Ok(y)
17+
}

0 commit comments

Comments
 (0)