From 829b70330e93779575daa35fc4ddc1dd0df15efa Mon Sep 17 00:00:00 2001 From: Djzin Date: Tue, 7 Nov 2017 00:07:32 +0000 Subject: [PATCH 1/4] always add an unreachable branch on matches to give more info to llvm about which values are possible --- src/librustc_mir/build/matches/test.rs | 6 ++- src/test/codegen/match.rs | 9 ++-- src/test/mir-opt/match_false_edges.rs | 58 ++++++++++++++------------ 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index 4792bf2b213e6..439f028ca8add 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -187,7 +187,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let num_enum_variants = self.hir.num_variants(adt_def); let used_variants = variants.count(); let mut otherwise_block = None; - let mut target_blocks = Vec::with_capacity(num_enum_variants); + let mut target_blocks = Vec::with_capacity(num_enum_variants + 1); let mut targets = Vec::with_capacity(used_variants + 1); let mut values = Vec::with_capacity(used_variants); let tcx = self.hir.tcx(); @@ -205,7 +205,9 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if let Some(otherwise_block) = otherwise_block { targets.push(otherwise_block); } else { - values.pop(); + let unreachable_block = self.cfg.start_new_block(); + targets.push(unreachable_block); + target_blocks.push(unreachable_block); } debug!("num_enum_variants: {}, tested variants: {:?}, variants: {:?}", num_enum_variants, values, variants); diff --git a/src/test/codegen/match.rs b/src/test/codegen/match.rs index aa100da60132f..660b6346c57f1 100644 --- a/src/test/codegen/match.rs +++ b/src/test/codegen/match.rs @@ -21,12 +21,15 @@ pub enum E { #[no_mangle] pub fn exhaustive_match(e: E) { // CHECK: switch{{.*}}, label %[[OTHERWISE:[a-zA-Z0-9_]+]] [ -// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[TRUE:[a-zA-Z0-9_]+]] +// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]] +// CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]] // CHECK-NEXT: ] -// CHECK: [[TRUE]]: +// CHECK: [[A]]: // CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]] -// CHECK: [[OTHERWISE]]: +// CHECK: [[B]]: // CHECK-NEXT: br label %[[EXIT:[a-zA-Z0-9_]+]] +// CHECK: [[OTHERWISE]]: +// CHECK-NEXT: unreachable match e { E::A => (), E::B => (), diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs index 02e9d39668de0..318db7a9e3e1a 100644 --- a/src/test/mir-opt/match_false_edges.rs +++ b/src/test/mir-opt/match_false_edges.rs @@ -54,24 +54,24 @@ fn main() { // ... // _2 = std::option::Option::Some(const 42i32,); // _5 = discriminant(_2); -// switchInt(_5) -> [0isize: bb5, otherwise: bb3]; +// switchInt(_5) -> [0isize: bb5, 1isize: bb3, otherwise: bb7]; // } // bb1: { // arm1 // StorageLive(_7); // _7 = _3; // _1 = (const 1i32, _7); // StorageDead(_7); -// goto -> bb11; +// goto -> bb12; // } // bb2: { // binding3(empty) and arm3 // _1 = (const 3i32, const 3i32); -// goto -> bb11; +// goto -> bb12; // } // bb3: { -// falseEdges -> [real: bb7, imaginary: bb4]; //pre_binding1 +// falseEdges -> [real: bb8, imaginary: bb4]; //pre_binding1 // } // bb4: { -// falseEdges -> [real: bb10, imaginary: bb5]; //pre_binding2 +// falseEdges -> [real: bb11, imaginary: bb5]; //pre_binding2 // } // bb5: { // falseEdges -> [real: bb2, imaginary: bb6]; //pre_binding3 @@ -79,28 +79,31 @@ fn main() { // bb6: { // unreachable; // } -// bb7: { // binding1 and guard +// bb7: { +// unreachable; +// } +// bb8: { // binding1 and guard // StorageLive(_3); // _3 = ((_2 as Some).0: i32); // StorageLive(_6); -// _6 = const guard() -> bb8; +// _6 = const guard() -> bb9; // } -// bb8: { // end of guard -// switchInt(_6) -> [0u8: bb9, otherwise: bb1]; +// bb9: { // end of guard +// switchInt(_6) -> [0u8: bb10, otherwise: bb1]; // } -// bb9: { // to pre_binding2 +// bb10: { // to pre_binding2 // falseEdges -> [real: bb4, imaginary: bb4]; // } -// bb10: { // bindingNoLandingPads.before.mir2 and arm2 +// bb11: { // bindingNoLandingPads.before.mir2 and arm2 // StorageLive(_4); // _4 = ((_2 as Some).0: i32); // StorageLive(_8); // _8 = _4; // _1 = (const 2i32, _8); // StorageDead(_8); -// goto -> bb11; +// goto -> bb12; // } -// bb11: { +// bb12: { // ... // return; // } @@ -111,53 +114,56 @@ fn main() { // ... // _2 = std::option::Option::Some(const 42i32,); // _5 = discriminant(_2); -// switchInt(_5) -> [0isize: bb4, otherwise: bb3]; +// switchInt(_5) -> [0isize: bb4, 1isize: bb3, otherwise: bb7]; // } // bb1: { // arm1 // StorageLive(_7); // _7 = _3; // _1 = (const 1i32, _7); // StorageDead(_7); -// goto -> bb11; +// goto -> bb12; // } // bb2: { // binding3(empty) and arm3 // _1 = (const 3i32, const 3i32); -// goto -> bb11; +// goto -> bb12; // } // bb3: { -// falseEdges -> [real: bb7, imaginary: bb4]; //pre_binding1 +// falseEdges -> [real: bb8, imaginary: bb4]; //pre_binding1 // } // bb4: { // falseEdges -> [real: bb2, imaginary: bb5]; //pre_binding2 // } // bb5: { -// falseEdges -> [real: bb10, imaginary: bb6]; //pre_binding3 +// falseEdges -> [real: bb11, imaginary: bb6]; //pre_binding3 // } // bb6: { // unreachable; // } -// bb7: { // binding1 and guard +// bb7: { +// unreachable; +// } +// bb8: { // binding1 and guard // StorageLive(_3); // _3 = ((_2 as Some).0: i32); // StorageLive(_6); -// _6 = const guard() -> bb8; +// _6 = const guard() -> bb9; // } -// bb8: { // end of guard -// switchInt(_6) -> [0u8: bb9, otherwise: bb1]; +// bb9: { // end of guard +// switchInt(_6) -> [0u8: bb10, otherwise: bb1]; // } -// bb9: { // to pre_binding2 +// bb10: { // to pre_binding2 // falseEdges -> [real: bb5, imaginary: bb4]; // } -// bb10: { // binding2 and arm2 +// bb11: { // binding2 and arm2 // StorageLive(_4); // _4 = ((_2 as Some).0: i32); // StorageLive(_8); // _8 = _4; // _1 = (const 2i32, _8); // StorageDead(_8); -// goto -> bb11; +// goto -> bb12; // } -// bb11: { +// bb12: { // ... // return; // } From aed0c9c9c034eed04a9cfdc238e6c50696a308cc Mon Sep 17 00:00:00 2001 From: Djzin Date: Mon, 13 Nov 2017 23:08:12 +0000 Subject: [PATCH 2/4] use lazy cached unreachable block - assign it to the function's closing brace --- src/librustc_mir/build/matches/test.rs | 2 +- src/librustc_mir/build/mod.rs | 21 ++++++++++++++++++++- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index 439f028ca8add..c0b93456d8578 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -205,7 +205,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if let Some(otherwise_block) = otherwise_block { targets.push(otherwise_block); } else { - let unreachable_block = self.cfg.start_new_block(); + let unreachable_block = self.unreachable_block(); targets.push(unreachable_block); target_blocks.push(unreachable_block); } diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 2073d49530061..79be37010412d 100644 --- a/src/librustc_mir/build/mod.rs +++ b/src/librustc_mir/build/mod.rs @@ -306,6 +306,8 @@ struct Builder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { cached_resume_block: Option, /// cached block with the RETURN terminator cached_return_block: Option, + /// cached block with the UNREACHABLE terminator + cached_unreachable_block: Option, } struct CFG<'tcx> { @@ -399,6 +401,11 @@ fn construct_fn<'a, 'gcx, 'tcx, A>(hir: Cx<'a, 'gcx, 'tcx>, TerminatorKind::Goto { target: return_block }); builder.cfg.terminate(return_block, source_info, TerminatorKind::Return); + // Attribute any unreachable codepaths to the function's closing brace + if let Some(unreachable_block) = builder.cached_unreachable_block { + builder.cfg.terminate(unreachable_block, source_info, + TerminatorKind::Unreachable); + } return_block.unit() })); assert_eq!(block, builder.return_block()); @@ -501,7 +508,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { var_indices: NodeMap(), unit_temp: None, cached_resume_block: None, - cached_return_block: None + cached_return_block: None, + cached_unreachable_block: None, }; assert_eq!(builder.cfg.start_new_block(), START_BLOCK); @@ -630,6 +638,17 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } } } + + fn unreachable_block(&mut self) -> BasicBlock { + match self.cached_unreachable_block { + Some(ub) => ub, + None => { + let ub = self.cfg.start_new_block(); + self.cached_unreachable_block = Some(ub); + ub + } + } + } } /////////////////////////////////////////////////////////////////////////// From c503fe113491500bac170c791e71cf02c45ab6e3 Mon Sep 17 00:00:00 2001 From: Djzin Date: Mon, 13 Nov 2017 23:24:30 +0000 Subject: [PATCH 3/4] add optimization codegen tests --- src/test/codegen/match-optimizes-away.rs | 44 ++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 src/test/codegen/match-optimizes-away.rs diff --git a/src/test/codegen/match-optimizes-away.rs b/src/test/codegen/match-optimizes-away.rs new file mode 100644 index 0000000000000..c0f2f64f82c8d --- /dev/null +++ b/src/test/codegen/match-optimizes-away.rs @@ -0,0 +1,44 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +// +// no-system-llvm +// compile-flags: -O +#![crate_type="lib"] + +pub enum Three { First, Second, Third } +use Three::*; + +pub enum Four { First, Second, Third, Fourth } +use Four::*; + +#[no_mangle] +pub fn three_valued(x: Three) -> Three { + // CHECK-LABEL: @three_valued + // CHECK-NEXT: {{^.*:$}} + // CHECK-NEXT: ret i8 %0 + match x { + First => First, + Second => Second, + Third => Third, + } +} + +#[no_mangle] +pub fn four_valued(x: Four) -> Four { + // CHECK-LABEL: @four_valued + // CHECK-NEXT: {{^.*:$}} + // CHECK-NEXT: ret i8 %0 + match x { + First => First, + Second => Second, + Third => Third, + Fourth => Fourth, + } +} From 5b1cc1d81024a28b77d40a33b49b2d396a120b69 Mon Sep 17 00:00:00 2001 From: Djzin Date: Tue, 14 Nov 2017 06:59:56 +0000 Subject: [PATCH 4/4] don't send block back to be marked unreachable twice --- src/librustc_mir/build/matches/test.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/build/matches/test.rs b/src/librustc_mir/build/matches/test.rs index c0b93456d8578..1cf35af3a9e1b 100644 --- a/src/librustc_mir/build/matches/test.rs +++ b/src/librustc_mir/build/matches/test.rs @@ -187,7 +187,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { let num_enum_variants = self.hir.num_variants(adt_def); let used_variants = variants.count(); let mut otherwise_block = None; - let mut target_blocks = Vec::with_capacity(num_enum_variants + 1); + let mut target_blocks = Vec::with_capacity(num_enum_variants); let mut targets = Vec::with_capacity(used_variants + 1); let mut values = Vec::with_capacity(used_variants); let tcx = self.hir.tcx(); @@ -205,9 +205,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { if let Some(otherwise_block) = otherwise_block { targets.push(otherwise_block); } else { - let unreachable_block = self.unreachable_block(); - targets.push(unreachable_block); - target_blocks.push(unreachable_block); + targets.push(self.unreachable_block()); } debug!("num_enum_variants: {}, tested variants: {:?}, variants: {:?}", num_enum_variants, values, variants);