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 | 47 +-- .../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, 352 insertions(+), 317 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 2097f38c25d76..80a5bff8cadd9 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); - 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_constant( - true_block, + shortcircuit_block, source_info, destination, Constant { span: expr_span, user_ty: None, - literal: ty::Const::from_bool(this.tcx, true).into(), + 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); - 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(), - }, - ); + let rhs = unpack!(else_block = this.as_local_operand(else_block, rhs)); + this.cfg.push_assign(else_block, source_info, destination, Rvalue::Use(rhs)); + this.cfg.goto(else_block, source_info, join_block); - // 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 0000000000000..b22b7f52402dd --- /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 0aa6fe65686cf..3d1b737ec2d7e 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 @@ 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 358e2e2bbba3c..a6f8d500065fa 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 @@ -
@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 2e128181c5e42..9858b270d7ef9 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 @@ -
@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 637b1c62086c5..bf94e5cbf739b 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 @@ -
@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 45fec46f55ca4..ef431a356c250 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 @@ 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 9b32bcb47f6e5..be4f5efc5dbf1 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 @@ 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