Skip to content

Commit ff0f5de

Browse files
borsbors
committed
Auto merge of #45821 - djzin:unreachable-match-arms, r=nikomatsakis
always add an unreachable branch on matches to give more info to llvm As part of https://github.com/djzin/rustc-optimization I discovered that some simple enum optimizations (src/unary/three_valued_enum.rs and src/unary/four_valued_enum.rs in the repo) are not applied - and the reason for this is that we erase the info that the discriminant of an enum is one of the options by putting the last one in an "otherwise" branch. This patch adds an extra branch so that LLVM can know what the possibilities are for the discriminant, which fixes the three- and four- valued cases. Note that for whatever reason, this doesn't fix the case of 2 variants (most notably `Option` and `Result` have 2 variants) - a pass re-ordering might fix this or we may wish to add "assume" annotations on discriminants to force it to optimize.
2 parents dbeb5bf + 5b1cc1d commit ff0f5de

File tree

5 files changed

+103
-31
lines changed

5 files changed

+103
-31
lines changed

‎src/librustc_mir/build/matches/test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
205205
if let Some(otherwise_block) = otherwise_block {
206206
targets.push(otherwise_block);
207207
} else {
208-
values.pop();
208+
targets.push(self.unreachable_block());
209209
}
210210
debug!("num_enum_variants: {}, tested variants: {:?}, variants: {:?}",
211211
num_enum_variants, values, variants);

‎src/librustc_mir/build/mod.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,8 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
307307
cached_resume_block: Option<BasicBlock>,
308308
/// cached block with the RETURN terminator
309309
cached_return_block: Option<BasicBlock>,
310+
/// cached block with the UNREACHABLE terminator
311+
cached_unreachable_block: Option<BasicBlock>,
310312
}
311313

312314
struct CFG<'tcx> {
@@ -400,6 +402,11 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>,
400402
TerminatorKind::Goto { target: return_block });
401403
builder.cfg.terminate(return_block, source_info,
402404
TerminatorKind::Return);
405+
// Attribute any unreachable codepaths to the function's closing brace
406+
if let Some(unreachable_block) = builder.cached_unreachable_block {
407+
builder.cfg.terminate(unreachable_block, source_info,
408+
TerminatorKind::Unreachable);
409+
}
403410
return_block.unit()
404411
}));
405412
assert_eq!(block, builder.return_block());
@@ -502,7 +509,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
502509
var_indices: NodeMap(),
503510
unit_temp: None,
504511
cached_resume_block: None,
505-
cached_return_block: None
512+
cached_return_block: None,
513+
cached_unreachable_block: None,
506514
};
507515

508516
assert_eq!(builder.cfg.start_new_block(), START_BLOCK);
@@ -631,6 +639,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
631639
}
632640
}
633641
}
642+
643+
fn unreachable_block(&mut self) -> BasicBlock {
644+
match self.cached_unreachable_block {
645+
Some(ub) => ub,
646+
None => {
647+
let ub = self.cfg.start_new_block();
648+
self.cached_unreachable_block = Some(ub);
649+
ub
650+
}
651+
}
652+
}
634653
}
635654

636655
///////////////////////////////////////////////////////////////////////////

‎src/test/codegen/match-optimizes-away.rs

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
//
11+
// no-system-llvm
12+
// compile-flags: -O
13+
#![crate_type="lib"]
14+
15+
pub enum Three { First, Second, Third }
16+
use Three::*;
17+
18+
pub enum Four { First, Second, Third, Fourth }
19+
use Four::*;
20+
21+
#[no_mangle]
22+
pub fn three_valued(x: Three) -> Three {
23+
// CHECK-LABEL: @three_valued
24+
// CHECK-NEXT: {{^.*:$}}
25+
// CHECK-NEXT: ret i8 %0
26+
match x {
27+
First => First,
28+
Second => Second,
29+
Third => Third,
30+
}
31+
}
32+
33+
#[no_mangle]
34+
pub fn four_valued(x: Four) -> Four {
35+
// CHECK-LABEL: @four_valued
36+
// CHECK-NEXT: {{^.*:$}}
37+
// CHECK-NEXT: ret i8 %0
38+
match x {
39+
First => First,
40+
Second => Second,
41+
Third => Third,
42+
Fourth => Fourth,
43+
}
44+
}

‎src/test/codegen/match.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,15 @@ pub enum E {
2121
#[no_mangle]
2222
pub fn exhaustive_match(e: E) {
2323
// CHECK: switch{{.*}}, label %[[OTHERWISE:[a-zA-Z0-9_]+]] [
24-
// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[TRUE:[a-zA-Z0-9_]+]]
24+
// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]]
25+
// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]]
2526
// CHECK-NEXT: ]
26-
// CHECK: [[TRUE]]:
27+
// CHECK: [[A]]:
2728
// CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]]
28-
// CHECK: [[OTHERWISE]]:
29+
// CHECK: [[B]]:
2930
// CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]]
31+
// CHECK: [[OTHERWISE]]:
32+
// CHECK-NEXT: unreachable
3033
match e {
3134
E::A => (),
3235
E::B => (),

‎src/test/mir-opt/match_false_edges.rs

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -54,53 +54,56 @@ fn main() {
5454
// ...
5555
// _2 = std::option::Option<i32>::Some(const 42i32,);
5656
// _5 = discriminant(_2);
57-
// switchInt(_5) -> [0isize: bb5, otherwise: bb3];
57+
// switchInt(_5) -> [0isize: bb5, 1isize: bb3, otherwise: bb7];
5858
// }
5959
// bb1: { // arm1
6060
// StorageLive(_7);
6161
// _7 = _3;
6262
// _1 = (const 1i32, _7);
6363
// StorageDead(_7);
64-
// goto -> bb11;
64+
// goto -> bb12;
6565
// }
6666
// bb2: { // binding3(empty) and arm3
6767
// _1 = (const 3i32, const 3i32);
68-
// goto -> bb11;
68+
// goto -> bb12;
6969
// }
7070
// bb3: {
71-
// falseEdges -> [real: bb7, imaginary: bb4]; //pre_binding1
71+
// falseEdges -> [real: bb8, imaginary: bb4]; //pre_binding1
7272
// }
7373
// bb4: {
74-
// falseEdges -> [real: bb10, imaginary: bb5]; //pre_binding2
74+
// falseEdges -> [real: bb11, imaginary: bb5]; //pre_binding2
7575
// }
7676
// bb5: {
7777
// falseEdges -> [real: bb2, imaginary: bb6]; //pre_binding3
7878
// }
7979
// bb6: {
8080
// unreachable;
8181
// }
82-
// bb7: { // binding1 and guard
82+
// bb7: {
83+
// unreachable;
84+
// }
85+
// bb8: { // binding1 and guard
8386
// StorageLive(_3);
8487
// _3 = ((_2 as Some).0: i32);
8588
// StorageLive(_6);
86-
// _6 = const guard() -> bb8;
89+
// _6 = const guard() -> bb9;
8790
// }
88-
// bb8: { // end of guard
89-
// switchInt(_6) -> [0u8: bb9, otherwise: bb1];
91+
// bb9: { // end of guard
92+
// switchInt(_6) -> [0u8: bb10, otherwise: bb1];
9093
// }
91-
// bb9: { // to pre_binding2
94+
// bb10: { // to pre_binding2
9295
// falseEdges -> [real: bb4, imaginary: bb4];
9396
// }
94-
// bb10: { // bindingNoLandingPads.before.mir2 and arm2
97+
// bb11: { // bindingNoLandingPads.before.mir2 and arm2
9598
// StorageLive(_4);
9699
// _4 = ((_2 as Some).0: i32);
97100
// StorageLive(_8);
98101
// _8 = _4;
99102
// _1 = (const 2i32, _8);
100103
// StorageDead(_8);
101-
// goto -> bb11;
104+
// goto -> bb12;
102105
// }
103-
// bb11: {
106+
// bb12: {
104107
// ...
105108
// return;
106109
// }
@@ -111,53 +114,56 @@ fn main() {
111114
// ...
112115
// _2 = std::option::Option<i32>::Some(const 42i32,);
113116
// _5 = discriminant(_2);
114-
// switchInt(_5) -> [0isize: bb4, otherwise: bb3];
117+
// switchInt(_5) -> [0isize: bb4, 1isize: bb3, otherwise: bb7];
115118
// }
116119
// bb1: { // arm1
117120
// StorageLive(_7);
118121
// _7 = _3;
119122
// _1 = (const 1i32, _7);
120123
// StorageDead(_7);
121-
// goto -> bb11;
124+
// goto -> bb12;
122125
// }
123126
// bb2: { // binding3(empty) and arm3
124127
// _1 = (const 3i32, const 3i32);
125-
// goto -> bb11;
128+
// goto -> bb12;
126129
// }
127130
// bb3: {
128-
// falseEdges -> [real: bb7, imaginary: bb4]; //pre_binding1
131+
// falseEdges -> [real: bb8, imaginary: bb4]; //pre_binding1
129132
// }
130133
// bb4: {
131134
// falseEdges -> [real: bb2, imaginary: bb5]; //pre_binding2
132135
// }
133136
// bb5: {
134-
// falseEdges -> [real: bb10, imaginary: bb6]; //pre_binding3
137+
// falseEdges -> [real: bb11, imaginary: bb6]; //pre_binding3
135138
// }
136139
// bb6: {
137140
// unreachable;
138141
// }
139-
// bb7: { // binding1 and guard
142+
// bb7: {
143+
// unreachable;
144+
// }
145+
// bb8: { // binding1 and guard
140146
// StorageLive(_3);
141147
// _3 = ((_2 as Some).0: i32);
142148
// StorageLive(_6);
143-
// _6 = const guard() -> bb8;
149+
// _6 = const guard() -> bb9;
144150
// }
145-
// bb8: { // end of guard
146-
// switchInt(_6) -> [0u8: bb9, otherwise: bb1];
151+
// bb9: { // end of guard
152+
// switchInt(_6) -> [0u8: bb10, otherwise: bb1];
147153
// }
148-
// bb9: { // to pre_binding2
154+
// bb10: { // to pre_binding2
149155
// falseEdges -> [real: bb5, imaginary: bb4];
150156
// }
151-
// bb10: { // binding2 and arm2
157+
// bb11: { // binding2 and arm2
152158
// StorageLive(_4);
153159
// _4 = ((_2 as Some).0: i32);
154160
// StorageLive(_8);
155161
// _8 = _4;
156162
// _1 = (const 2i32, _8);
157163
// StorageDead(_8);
158-
// goto -> bb11;
164+
// goto -> bb12;
159165
// }
160-
// bb11: {
166+
// bb12: {
161167
// ...
162168
// return;
163169
// }

0 commit comments

Comments
 (0)