From 4464cc2256b2ccf5fbdaaf60135db11537d5bff9 Mon Sep 17 00:00:00 2001 From: AngelicosPhosphoros Date: Tue, 30 Mar 2021 00:19:10 +0300 Subject: [PATCH] Simplify logical operations CFG This is basically same commit as e38e954a0d249f88d0a55504f70d6055e865a931 which was reverted later in 676953fde9120cda62e4ef2f75a804af7481d6af In both cases, this changes weren't benchmarked. e38e954a0d249f88d0a55504f70d6055e865a931 leads to missed optimization described in [this issue](https://github.com/rust-lang/rust/issues/62993) 676953fde9120cda62e4ef2f75a804af7481d6af leads to missed optimization described in [this issue](https://github.com/rust-lang/rust/issues/83623) Also it changes some src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump* files automatically. --- .../rustc_mir_build/src/build/expr/into.rs | 63 ++- .../codegen/issue-83623-SIMD-PartialEq.rs | 46 +++ ...ons.main.-------.InstrumentCoverage.0.html | 382 +++++++++--------- ...ean.main.-------.InstrumentCoverage.0.html | 170 ++++---- ...pl#6}-eq.-------.InstrumentCoverage.0.html | 2 +- ...pl#6}-ne.-------.InstrumentCoverage.0.html | 2 +- ...used.foo.-------.InstrumentCoverage.0.html | 10 +- ...ate_func.-------.InstrumentCoverage.0.html | 10 +- 8 files changed, 360 insertions(+), 325 deletions(-) create mode 100644 src/test/codegen/issue-83623-SIMD-PartialEq.rs diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index 2097f38c25d..80a5bff8cad 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -111,18 +111,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { ExprKind::LogicalOp { op, lhs, rhs } => { // And: // - // [block: If(lhs)] -true-> [else_block: If(rhs)] -true-> [true_block] - // | | (false) - // +----------false-----------+------------------> [false_block] + // [block: If(lhs)] -true-> [else_block: dest = (rhs)] + // | (false) + // [shortcurcuit_block: dest = false] // // Or: // - // [block: If(lhs)] -false-> [else_block: If(rhs)] -true-> [true_block] - // | (true) | (false) - // [true_block] [false_block] + // [block: If(lhs)] -false-> [else_block: dest = (rhs)] + // | (true) + // [shortcurcuit_block: dest = true] - let (true_block, false_block, mut else_block, join_block) = ( - this.cfg.start_new_block(), + let (shortcircuit_block, mut else_block, join_block) = ( this.cfg.start_new_block(), this.cfg.start_new_block(), this.cfg.start_new_block(), @@ -130,41 +129,31 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let lhs = unpack!(block = this.as_local_operand(block, lhs)); let blocks = match op { - LogicalOp::And => (else_block, false_block), - LogicalOp::Or => (true_block, else_block), + LogicalOp::And => (else_block, shortcircuit_block), + LogicalOp::Or => (shortcircuit_block, else_block), }; let term = TerminatorKind::if_(this.tcx, lhs, blocks.0, blocks.1); this.cfg.terminate(block, source_info, term); + this.cfg.push_assign_constant( + shortcircuit_block, + source_info, + destination, + Constant { + span: expr_span, + user_ty: None, + literal: match op { + LogicalOp::And => ty::Const::from_bool(this.tcx, false).into(), + LogicalOp::Or => ty::Const::from_bool(this.tcx, true).into(), + }, + }, + ); + this.cfg.goto(shortcircuit_block, source_info, join_block); + let rhs = unpack!(else_block = this.as_local_operand(else_block, rhs)); - let term = TerminatorKind::if_(this.tcx, rhs, true_block, false_block); - this.cfg.terminate(else_block, source_info, term); + this.cfg.push_assign(else_block, source_info, destination, Rvalue::Use(rhs)); + this.cfg.goto(else_block, source_info, join_block); - this.cfg.push_assign_constant( - true_block, - source_info, - destination, - Constant { - span: expr_span, - user_ty: None, - literal: ty::Const::from_bool(this.tcx, true).into(), - }, - ); - - this.cfg.push_assign_constant( - false_block, - source_info, - destination, - Constant { - span: expr_span, - user_ty: None, - literal: ty::Const::from_bool(this.tcx, false).into(), - }, - ); - - // Link up both branches: - this.cfg.goto(true_block, source_info, join_block); - this.cfg.goto(false_block, source_info, join_block); join_block.unit() } ExprKind::Loop { body } => { diff --git a/src/test/codegen/issue-83623-SIMD-PartialEq.rs b/src/test/codegen/issue-83623-SIMD-PartialEq.rs new file mode 100644 index 00000000000..b22b7f52402 --- /dev/null +++ b/src/test/codegen/issue-83623-SIMD-PartialEq.rs @@ -0,0 +1,46 @@ +// This test checks that jumps generated by logical operators can be optimized away + +// compile-flags: -Copt-level=3 +// only-64bit + +#![crate_type="lib"] + +pub struct Blueprint { + pub fuel_tank_size: u32, + pub payload: u32, + pub wheel_diameter: u32, + pub wheel_width: u32, + pub storage: u32, +} + +// && chains should not prevent SIMD optimizations for primitives +impl PartialEq for Blueprint{ + fn eq(&self, other: &Self)->bool{ + // CHECK-NOT: call{{.*}}bcmp + // CHECK-NOT: call{{.*}}memcmp + // CHECK-NOT: br {{.*}} + self.fuel_tank_size == other.fuel_tank_size + && self.payload == other.payload + && self.wheel_diameter == other.wheel_diameter + && self.wheel_width == other.wheel_width + && self.storage == other.storage + } +} + +#[derive(PartialEq)] +pub struct Blueprint2 { + pub fuel_tank_size: u32, + pub payload: u32, + pub wheel_diameter: u32, + pub wheel_width: u32, + pub storage: u32, +} + +// Derived PartialEq should not generate jumps and should use SIMD +#[no_mangle] +pub fn partial_eq_should_not_jump(a: &Blueprint2, b:&Blueprint2)->bool{ + // CHECK-NOT: call{{.*}}bcmp + // CHECK-NOT: call{{.*}}memcmp + // CHECK-NOT: br {{.*}} + a==b +} diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.conditions/conditions.main.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.conditions/conditions.main.-------.InstrumentCoverage.0.html index 0aa6fe65686..3d1b737ec2d 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.conditions/conditions.main.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.conditions/conditions.main.-------.InstrumentCoverage.0.html @@ -83,7 +83,7 @@ For revisions in Pull Requests (PR): 5:13-7:6: @1[1]: _2 = const ()"> }⦉@1@2⦊‸⦉@2 const B: u32 = 100; - let @21⦊x⦉@21 = if let @19⦊x⦉@19 = if @3⦊countdown > 7⦉@3 { } else if @5⦊countdown > 2⦉@5 { if @7⦊countdown < 1⦉@7 || @15⦊countdown > 5⦉@15 || @11⦊countdown != 9⦉@11 @17⦊{ - countdown = 0; - }⦉@17@18⦊‸⦉@18 - @19,20⦊countdown -= 5; - countdown⦉@19,20 +14:12-14:25: @7[6]: _13 = Lt(move _14, const 1_u32)">@7⦊countdown < 1⦉@7 || @13⦊countdown > 5⦉@13 || @10⦊countdown != 9⦉@10 @15⦊{ + countdown = 0; + }⦉@15@16⦊‸⦉@16 + @17,18⦊countdown -= 5; + countdown⦉@17,18 } else { @8⦊return⦉@8; }; - let @21⦊mut countdown = 0; - if true⦉@21 @22⦊{ - countdown = 10; - }⦉@22@23⦊‸⦉@23 + let @19⦊mut countdown = 0; + if true⦉@19 @20⦊{ + countdown = 10; + }⦉@20@21⦊‸⦉@21 - if @24⦊countdown > 7⦉@24 @25,27⦊{ - countdown -= 4; - }⦉@25,27 else if @26⦊countdown > 2⦉@26 { - if @28⦊countdown < 1⦉@28 || @36⦊countdown > 5⦉@36 || @32⦊countdown != 9⦉@32 @38⦊{ - countdown = 0; - }⦉@38@39⦊‸⦉@39 - @40,41⦊countdown -= 5⦉@40,41; + if @22⦊countdown > 7⦉@22 @23,25⦊{ + countdown -= 4; + }⦉@23,25 else if @24⦊countdown > 2⦉@24 { + if @26⦊countdown < 1⦉@26 || @32⦊countdown > 5⦉@32 || @29⦊countdown != 9⦉@29 @34⦊{ + countdown = 0; + }⦉@34@35⦊‸⦉@35 + @36,37⦊countdown -= 5⦉@36,37; } else { - @29⦊return⦉@29; + @27⦊return⦉@27; } - if @42⦊true⦉@42 { - let @43⦊mut countdown = 0; - if true⦉@43 @45⦊{ - countdown = 10; - }⦉@45@46⦊‸⦉@46 + if @38⦊true⦉@38 { + let @39⦊mut countdown = 0; + if true⦉@39 @41⦊{ + countdown = 10; + }⦉@41@42⦊‸⦉@42 - if @47⦊countdown > 7⦉@47 @48,50⦊{ - countdown -= 4; - }⦉@48,50 - else if @49⦊countdown > 2⦉@49 { - if @51⦊countdown < 1⦉@51 || @59⦊countdown > 5⦉@59 || @55⦊countdown != 9⦉@55 @61⦊{ - countdown = 0; - }⦉@61@62⦊‸⦉@62 - @63,64⦊countdown -= 5⦉@63,64; + if @43⦊countdown > 7⦉@43 @44,46⦊{ + countdown -= 4; + }⦉@44,46 + else if @45⦊countdown > 2⦉@45 { + if @47⦊countdown < 1⦉@47 || @53⦊countdown > 5⦉@53 || @50⦊countdown != 9⦉@50 @55⦊{ + countdown = 0; + }⦉@55@56⦊‸⦉@56 + @57,58⦊countdown -= 5⦉@57,58; } else { - @52⦊return⦉@52; + @48⦊return⦉@48; } - }@44⦊‸⦉@44 // Note: closing brace shows uncovered (vs. `0` for implicit else) because condition literal + }@40⦊‸⦉@40 // Note: closing brace shows uncovered (vs. `0` for implicit else) because condition literal // `true` was const-evaluated. The compiler knows the `if` block will be executed. - let @66⦊mut countdown = 0; - if true⦉@66 @67⦊{ - countdown = 1; - }⦉@67@68⦊‸⦉@68 + let @60⦊mut countdown = 0; + if true⦉@60 @61⦊{ + countdown = 1; + }⦉@61@62⦊‸⦉@62 - let @89⦊z⦉@89 = if @69⦊countdown > 7⦉@69 @70,72⦊{ - countdown -= 4; - }⦉@70,72 else if @71⦊countdown > 2⦉@71 { - if @73⦊countdown < 1⦉@73 || @81⦊countdown > 5⦉@81 || @77⦊countdown != 9⦉@77 @83⦊{ - countdown = 0; - }⦉@83@84⦊‸⦉@84 - @85,86⦊countdown -= 5⦉@85,86; + let @81⦊z⦉@81 = if @63⦊countdown > 7⦉@63 @64,66⦊{ + countdown -= 4; + }⦉@64,66 else if @65⦊countdown > 2⦉@65 { + if @67⦊countdown < 1⦉@67 || @73⦊countdown > 5⦉@73 || @70⦊countdown != 9⦉@70 @75⦊{ + countdown = 0; + }⦉@75@76⦊‸⦉@76 + @77,78⦊countdown -= 5⦉@77,78; } else { - let @74,87,88⦊should_be_reachable = countdown; - println!("reached"); - return⦉@74,87,88; + let @68,79,80⦊should_be_reachable = countdown; + println!("reached"); + return⦉@68,79,80; }; - let @107⦊w⦉@107 = if @89⦊countdown > 7⦉@89 @90,92⦊{ - countdown -= 4; - }⦉@90,92 else if @91⦊countdown > 2⦉@91 { - if @93⦊countdown < 1⦉@93 || @101⦊countdown > 5⦉@101 || @97⦊countdown != 9⦉@97 @103⦊{ - countdown = 0; - }⦉@103@104⦊‸⦉@104 - @105,106⦊countdown -= 5⦉@105,106; + let @97⦊w⦉@97 = if @81⦊countdown > 7⦉@81 @82,84⦊{ + countdown -= 4; + }⦉@82,84 else if @83⦊countdown > 2⦉@83 { + if @85⦊countdown < 1⦉@85 || @91⦊countdown > 5⦉@91 || @88⦊countdown != 9⦉@88 @93⦊{ + countdown = 0; + }⦉@93@94⦊‸⦉@94 + @95,96⦊countdown -= 5⦉@95,96; } else { - @94⦊return⦉@94; + @86⦊return⦉@86; }; -}@111⦊‸⦉@111 +}@101⦊‸⦉@101 diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.lazy_boolean/lazy_boolean.main.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.lazy_boolean/lazy_boolean.main.-------.InstrumentCoverage.0.html index 358e2e2bbba..a6f8d500065 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.lazy_boolean/lazy_boolean.main.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.lazy_boolean/lazy_boolean.main.-------.InstrumentCoverage.0.html @@ -69,9 +69,9 @@ For revisions in Pull Requests (PR): -
@0,1,2,3⦊fn main() { - // Initialize test constants in a way that cannot be determined at compile time, to ensure - // rustc and LLVM cannot optimize out statements (or coverage counters) downstream from - // dependent conditions. - let is_true = std::env::args().len() == 1; - - let (mut a, mut b, mut c) = (0, 0, 0); - }⦉@4@5⦊‸⦉@5 let - @10⦊somebool⦉@10 + @9⦊somebool⦉@9 = @6⦊a < b⦉@6 || - @9⦊b < c⦉@9 + @8⦊b < c⦉@8 ; let - @14⦊somebool⦉@14 + @12⦊somebool⦉@12 = - @10⦊b < a⦉@10 + @9⦊b < a⦉@9 || - @13⦊b < c⦉@13 + @11⦊b < c⦉@11 ; - let @18⦊somebool⦉@18 = @14⦊a < b⦉@14 && @17⦊b < c⦉@17; - let @22⦊somebool⦉@22 = @18⦊b < a⦉@18 && @21⦊b < c⦉@21; + let @15⦊somebool⦉@15 = @12⦊a < b⦉@12 && @14⦊b < c⦉@14; + let @18⦊somebool⦉@18 = @15⦊b < a⦉@15 && @17⦊b < c⦉@17; if - @22⦊! - is_true⦉@22 - @23⦊{ - a = 2 - ; - }⦉@23@24⦊‸⦉@24 + @18⦊! + is_true⦉@18 + @19⦊{ + a = 2 + ; + }⦉@19@20⦊‸⦉@20 if - @25⦊is_true⦉@25 - @26⦊{ - b = 30 - ; - }⦉@26 + @21⦊is_true⦉@21 + @22⦊{ + b = 30 + ; + }⦉@22 else - @27⦊{ - c = 400 - ; - }⦉@27 + @23⦊{ + c = 400 + ; + }⦉@23 - if @28⦊!is_true⦉@28 @29⦊{ - a = 2; - }⦉@29@30⦊‸⦉@30 + if @24⦊!is_true⦉@24 @25⦊{ + a = 2; + }⦉@25@26⦊‸⦉@26 - if @31⦊is_true⦉@31 @32⦊{ - b = 30; - }⦉@32 else @33⦊{ - c = 400; - }⦉@33 -}@34⦊‸⦉@34
+ if @27⦊is_true⦉@27 @28⦊{ + b = 30; + }⦉@28 else @29⦊{ + c = 400; + }⦉@29 +}@30⦊‸⦉@30 diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-eq.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-eq.-------.InstrumentCoverage.0.html index 2e128181c5e..9858b270d7e 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-eq.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-eq.-------.InstrumentCoverage.0.html @@ -69,6 +69,6 @@ For revisions in Pull Requests (PR): -
@0⦊‸⦉@0PartialEq@4⦊‸⦉@4
+
@0⦊‸⦉@0PartialEq@3⦊‸⦉@3
diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-ne.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-ne.-------.InstrumentCoverage.0.html index 637b1c62086..bf94e5cbf73 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-ne.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.partial_eq/partial_eq.{impl#6}-ne.-------.InstrumentCoverage.0.html @@ -69,6 +69,6 @@ For revisions in Pull Requests (PR): -
@0⦊‸⦉@0PartialEq@4⦊‸⦉@4
+
@0⦊‸⦉@0PartialEq@3⦊‸⦉@3
diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html index 45fec46f55c..ef431a356c2 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.foo.-------.InstrumentCoverage.0.html @@ -77,11 +77,11 @@ For revisions in Pull Requests (PR): 3:11-3:17: @2[3]: _4 = Lt(move _5, const 10_i32) 3:11-3:17: @2[5]: FakeRead(ForMatchedPlace, _4)">@1,2⦊i < 10⦉@1,2 { @3,5⦊i != 0⦉@3,5 || @8⦊i != 0⦉@8; - @9,10⦊i += 1⦉@9,10; +4:9-4:15: @5[4]: _7 = Ne(move _8, const 0_i32)">@3,5⦊i != 0⦉@3,5 || @7⦊i != 0⦉@7; + @8,9⦊i += 1⦉@8,9; } -}@4,11⦊‸⦉@4,11 +}@4,10⦊‸⦉@4,10 diff --git a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html index 9b32bcb47f6..be4f5efc5db 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html +++ b/src/test/run-make-fulldeps/coverage-spanview/expected_mir_dump.unused/unused.unused_template_func.-------.InstrumentCoverage.0.html @@ -77,11 +77,11 @@ For revisions in Pull Requests (PR): 11:11-11:17: @2[3]: _4 = Lt(move _5, const 10_i32) 11:11-11:17: @2[5]: FakeRead(ForMatchedPlace, _4)">@1,2⦊i < 10⦉@1,2 { @3,5⦊i != 0⦉@3,5 || @8⦊i != 0⦉@8; - @9,10⦊i += 1⦉@9,10; +12:9-12:15: @5[4]: _7 = Ne(move _8, const 0_i32)">@3,5⦊i != 0⦉@3,5 || @7⦊i != 0⦉@7; + @8,9⦊i += 1⦉@8,9; } -}@4,11⦊‸⦉@4,11 +}@4,10⦊‸⦉@4,10