bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Auto merge of #119112 - Nadrieril:remove-target_blocks-hack, r=matthewjasper

Browse source 
match lowering: Remove the `make_target_blocks` hack

This hack was introduced 4 years ago in [`a1d0266` (#60730)](https://github.com/rust-lang/rust/pull/60730/commits/a1d0266878793bc8b2bf50958eb529005ed19da0) to improve LLVM optimization time, specifically noticed in the `encoding` benchmark. Measurements today indicate it is no longer needed.

r? `@matthewjasper`
This commit is contained in:
bors 2023-12-19 21:15:31 +00:00
commit f704f3b93b
4 changed files with 86 additions and 92 deletions
Download patch file Download diff file Expand all files Collapse all files

92
compiler/rustc_mir_build/src/build/matches/mod.rs
View file

@ -1699,59 +1699,51 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
debug!("tested_candidates: {}", total_candidate_count - candidates.len()); debug!("tested_candidates: {}", total_candidate_count - candidates.len());
debug!("untested_candidates: {}", candidates.len()); debug!("untested_candidates: {}", candidates.len());
// HACK(matthewjasper) This is a closure so that we can let the test // The block that we should branch to if none of the
// create its blocks before the rest of the match. This currently // `target_candidates` match. This is either the block where we
// improves the speed of llvm when optimizing long string literal // start matching the untested candidates if there are any,
// matches // otherwise it's the `otherwise_block`.
let make_target_blocks = move |this: &mut Self| -> Vec<BasicBlock> { let remainder_start = &mut None;
// The block that we should branch to if none of the let remainder_start =
// `target_candidates` match. This is either the block where we if candidates.is_empty() { &mut *otherwise_block } else { remainder_start };
// start matching the untested candidates if there are any,
// otherwise it's the `otherwise_block`.
let remainder_start = &mut None;
let remainder_start =
if candidates.is_empty() { &mut *otherwise_block } else { remainder_start };
// For each outcome of test, process the candidates that still // For each outcome of test, process the candidates that still
// apply. Collect a list of blocks where control flow will // apply. Collect a list of blocks where control flow will
// branch if one of the `target_candidate` sets is not // branch if one of the `target_candidate` sets is not
// exhaustive. // exhaustive.
let target_blocks: Vec<_> = target_candidates let target_blocks: Vec<_> = target_candidates
.into_iter() .into_iter()
.map(|mut candidates| { .map(|mut candidates| {
if !candidates.is_empty() { if !candidates.is_empty() {
let candidate_start = this.cfg.start_new_block(); let candidate_start = self.cfg.start_new_block();
this.match_candidates( self.match_candidates(
span, span,
scrutinee_span, scrutinee_span,
candidate_start, candidate_start,
remainder_start, remainder_start,
&mut *candidates, &mut *candidates,
fake_borrows, fake_borrows,
); );
candidate_start candidate_start
} else { } else {
*remainder_start.get_or_insert_with(|| this.cfg.start_new_block()) *remainder_start.get_or_insert_with(|| self.cfg.start_new_block())
} }
}) })
.collect(); .collect();
if !candidates.is_empty() { if !candidates.is_empty() {
let remainder_start = remainder_start.unwrap_or_else(|| this.cfg.start_new_block()); let remainder_start = remainder_start.unwrap_or_else(|| self.cfg.start_new_block());
this.match_candidates( self.match_candidates(
span, span,
scrutinee_span, scrutinee_span,
remainder_start, remainder_start,
otherwise_block, otherwise_block,
candidates, candidates,
fake_borrows, fake_borrows,
); );
}; }
target_blocks self.perform_test(span, scrutinee_span, block, &match_place, &test, target_blocks);
};
self.perform_test(span, scrutinee_span, block, &match_place, &test, make_target_blocks);
} }
/// Determine the fake borrows that are needed from a set of places that /// Determine the fake borrows that are needed from a set of places that

42
compiler/rustc_mir_build/src/build/matches/test.rs
View file

@ -147,7 +147,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} }
} }
#[instrument(skip(self, make_target_blocks, place_builder), level = "debug")] #[instrument(skip(self, target_blocks, place_builder), level = "debug")]
pub(super) fn perform_test( pub(super) fn perform_test(
&mut self, &mut self,
match_start_span: Span, match_start_span: Span,
@ -155,7 +155,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
block: BasicBlock, block: BasicBlock,
place_builder: &PlaceBuilder<'tcx>, place_builder: &PlaceBuilder<'tcx>,
test: &Test<'tcx>, test: &Test<'tcx>,
make_target_blocks: impl FnOnce(&mut Self) -> Vec<BasicBlock>, target_blocks: Vec<BasicBlock>,
) { ) {
let place = place_builder.to_place(self); let place = place_builder.to_place(self);
let place_ty = place.ty(&self.local_decls, self.tcx); let place_ty = place.ty(&self.local_decls, self.tcx);
@ -164,7 +164,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let source_info = self.source_info(test.span); let source_info = self.source_info(test.span);
match test.kind { match test.kind {
TestKind::Switch { adt_def, ref variants } => { TestKind::Switch { adt_def, ref variants } => {
let target_blocks = make_target_blocks(self);
// Variants is a BitVec of indexes into adt_def.variants. // Variants is a BitVec of indexes into adt_def.variants.
let num_enum_variants = adt_def.variants().len(); let num_enum_variants = adt_def.variants().len();
debug_assert_eq!(target_blocks.len(), num_enum_variants + 1); debug_assert_eq!(target_blocks.len(), num_enum_variants + 1);
@ -210,7 +209,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} }
TestKind::SwitchInt { switch_ty, ref options } => { TestKind::SwitchInt { switch_ty, ref options } => {
let target_blocks = make_target_blocks(self);
let terminator = if *switch_ty.kind() == ty::Bool { let terminator = if *switch_ty.kind() == ty::Bool {
assert!(!options.is_empty() && options.len() <= 2); assert!(!options.is_empty() && options.len() <= 2);
let [first_bb, second_bb] = *target_blocks else { let [first_bb, second_bb] = *target_blocks else {
@ -240,6 +238,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
TestKind::Eq { value, ty } => { TestKind::Eq { value, ty } => {
let tcx = self.tcx; let tcx = self.tcx;
let [success_block, fail_block] = *target_blocks else {
bug!("`TestKind::Eq` should have two target blocks")
};
if let ty::Adt(def, _) = ty.kind() if let ty::Adt(def, _) = ty.kind()
&& Some(def.did()) == tcx.lang_items().string() && Some(def.did()) == tcx.lang_items().string()
{ {
@ -280,38 +281,43 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
); );
self.non_scalar_compare( self.non_scalar_compare(
eq_block, eq_block,
make_target_blocks, success_block,
fail_block,
source_info, source_info,
value, value,
ref_str, ref_str,
ref_str_ty, ref_str_ty,
); );
return; } else if !ty.is_scalar() {
}
if !ty.is_scalar() {
// Use `PartialEq::eq` instead of `BinOp::Eq` // Use `PartialEq::eq` instead of `BinOp::Eq`
// (the binop can only handle primitives) // (the binop can only handle primitives)
self.non_scalar_compare( self.non_scalar_compare(
block, block,
make_target_blocks, success_block,
fail_block,
source_info, source_info,
value, value,
place, place,
ty, ty,
); );
} else if let [success, fail] = *make_target_blocks(self) { } else {
assert_eq!(value.ty(), ty); assert_eq!(value.ty(), ty);
let expect = self.literal_operand(test.span, value); let expect = self.literal_operand(test.span, value);
let val = Operand::Copy(place); let val = Operand::Copy(place);
self.compare(block, success, fail, source_info, BinOp::Eq, expect, val); self.compare(
} else { block,
bug!("`TestKind::Eq` should have two target blocks"); success_block,
fail_block,
source_info,
BinOp::Eq,
expect,
val,
);
} }
} }
TestKind::Range(ref range) => { TestKind::Range(ref range) => {
let lower_bound_success = self.cfg.start_new_block(); let lower_bound_success = self.cfg.start_new_block();
let target_blocks = make_target_blocks(self);
// Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons. // Test `val` by computing `lo <= val && val <= hi`, using primitive comparisons.
// FIXME: skip useless comparison when the range is half-open. // FIXME: skip useless comparison when the range is half-open.
@ -341,8 +347,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
} }
TestKind::Len { len, op } => { TestKind::Len { len, op } => {
let target_blocks = make_target_blocks(self);
let usize_ty = self.tcx.types.usize; let usize_ty = self.tcx.types.usize;
let actual = self.temp(usize_ty, test.span); let actual = self.temp(usize_ty, test.span);
@ -406,7 +410,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn non_scalar_compare( fn non_scalar_compare(
&mut self, &mut self,
block: BasicBlock, block: BasicBlock,
make_target_blocks: impl FnOnce(&mut Self) -> Vec<BasicBlock>, success_block: BasicBlock,
fail_block: BasicBlock,
source_info: SourceInfo, source_info: SourceInfo,
value: Const<'tcx>, value: Const<'tcx>,
mut val: Place<'tcx>, mut val: Place<'tcx>,
@ -531,9 +536,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
); );
self.diverge_from(block); self.diverge_from(block);
let [success_block, fail_block] = *make_target_blocks(self) else {
bug!("`TestKind::Eq` should have two target blocks")
};
// check the result // check the result
self.cfg.terminate( self.cfg.terminate(
eq_block, eq_block,

14
tests/mir-opt/exponential_or.match_tuple.SimplifyCfg-initial.after.mir
View file

@ -38,22 +38,22 @@ fn match_tuple(_1: (u32, bool, Option<i32>, u32)) -> u32 {
bb4: { bb4: {
_5 = Le(const 6_u32, (_1.3: u32)); _5 = Le(const 6_u32, (_1.3: u32));
switchInt(move _5) -> [0: bb6, otherwise: bb5]; switchInt(move _5) -> [0: bb5, otherwise: bb7];
} }
bb5: { bb5: {
_6 = Le((_1.3: u32), const 9_u32); _3 = Le(const 13_u32, (_1.3: u32));
switchInt(move _6) -> [0: bb6, otherwise: bb8]; switchInt(move _3) -> [0: bb1, otherwise: bb6];
} }
bb6: { bb6: {
_3 = Le(const 13_u32, (_1.3: u32)); _4 = Le((_1.3: u32), const 16_u32);
switchInt(move _3) -> [0: bb1, otherwise: bb7]; switchInt(move _4) -> [0: bb1, otherwise: bb8];
} }
bb7: { bb7: {
_4 = Le((_1.3: u32), const 16_u32); _6 = Le((_1.3: u32), const 9_u32);
switchInt(move _4) -> [0: bb1, otherwise: bb8]; switchInt(move _6) -> [0: bb5, otherwise: bb8];
} }
bb8: { bb8: {

30
tests/mir-opt/match_test.main.SimplifyCfg-initial.after.mir
View file

@ -28,43 +28,43 @@ fn main() -> () {
StorageLive(_3); StorageLive(_3);
PlaceMention(_1); PlaceMention(_1);
_6 = Le(const 0_i32, _1); _6 = Le(const 0_i32, _1);
switchInt(move _6) -> [0: bb4, otherwise: bb1]; switchInt(move _6) -> [0: bb3, otherwise: bb8];
} }
bb1: { bb1: {
_7 = Lt(_1, const 10_i32); falseEdge -> [real: bb9, imaginary: bb4];
switchInt(move _7) -> [0: bb4, otherwise: bb2];
} }
bb2: { bb2: {
falseEdge -> [real: bb9, imaginary: bb6];
}
bb3: {
_3 = const 3_i32; _3 = const 3_i32;
goto -> bb14; goto -> bb14;
} }
bb4: { bb3: {
_4 = Le(const 10_i32, _1); _4 = Le(const 10_i32, _1);
switchInt(move _4) -> [0: bb7, otherwise: bb5]; switchInt(move _4) -> [0: bb5, otherwise: bb7];
}
bb4: {
falseEdge -> [real: bb12, imaginary: bb6];
} }
bb5: { bb5: {
_5 = Le(_1, const 20_i32); switchInt(_1) -> [4294967295: bb6, otherwise: bb2];
switchInt(move _5) -> [0: bb7, otherwise: bb6];
} }
bb6: { bb6: {
falseEdge -> [real: bb12, imaginary: bb8]; falseEdge -> [real: bb13, imaginary: bb2];
} }
bb7: { bb7: {
switchInt(_1) -> [4294967295: bb8, otherwise: bb3]; _5 = Le(_1, const 20_i32);
switchInt(move _5) -> [0: bb5, otherwise: bb4];
} }
bb8: { bb8: {
falseEdge -> [real: bb13, imaginary: bb3]; _7 = Lt(_1, const 10_i32);
switchInt(move _7) -> [0: bb3, otherwise: bb1];
} }
bb9: { bb9: {
@ -83,7 +83,7 @@ fn main() -> () {
bb11: { bb11: {
StorageDead(_9); StorageDead(_9);
falseEdge -> [real: bb3, imaginary: bb6]; falseEdge -> [real: bb2, imaginary: bb4];
} }
bb12: { bb12: {