Skip to content

Commit abd6955

Browse files
committed
Auto merge of #66282 - Centril:simplify-try, r=oli-obk
[mir-opt] asking `?`s in a more optimized fashion This PR works towards #66234 by providing two optimization passes meant to run in sequence: - `SimplifyArmIdentity` which transforms something like: ```rust _LOCAL_TMP = ((_LOCAL_1 as Variant ).FIELD: TY ); ((_LOCAL_0 as Variant).FIELD: TY) = move _LOCAL_TMP; discriminant(_LOCAL_0) = VAR_IDX; ``` into: ```rust _LOCAL_0 = move _LOCAL_1 ``` - `SimplifyBranchSame` which transforms `SwitchInt`s to identical basic blocks into a `goto` to the first reachable target. Together, these are meant to simplify the following into just `res`: ```rust match res { Ok(x) => Ok(x), Err(x) => Err(x), } ``` It should be noted however that the desugaring of `?` includes a function call and so the first pass in this PR relies on inlining to substitute that function call for identity on `x`. Inlining requires `mir-opt-level=2` so this might not have any effect in perf-bot but let's find out. r? @oli-obk -- This is WIP, but I'd appreciate feedback. :)
2 parents f11759d + 2f00e86 commit abd6955

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
@@ -2053,7 +2053,7 @@ pub enum TyKind {
20532053
Err,
20542054
}
20552055

2056-
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
2056+
#[derive(Copy, Clone, RustcEncodable, RustcDecodable, Debug, HashStable, PartialEq)]
20572057
pub struct InlineAsmOutput {
20582058
pub constraint: Symbol,
20592059
pub is_rw: bool,
@@ -2063,7 +2063,7 @@ pub struct InlineAsmOutput {
20632063

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