From a7832b14b14e1af2e1e8a2cbf56b5b797892bee3 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Wed, 28 Feb 2024 18:21:24 +1100 Subject: [PATCH] Make the success arms of `if lhs || rhs` meet up in a separate block In the previous code, the success block of `lhs` would jump directly to the success block of `rhs`. However, `rhs_success_block` could already contain statements that are specific to the RHS, and the direct goto causes them to be executed in the LHS success path as well. This patch therefore creates a fresh block that the LHS and RHS success blocks can both jump to. --- .../rustc_mir_build/src/build/matches/mod.rs | 10 ++- ...n_conditional.test_complex.built.after.mir | 78 ++++++++++--------- ..._or_in_conditional.test_or.built.after.mir | 26 ++++--- 3 files changed, 64 insertions(+), 50 deletions(-) diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 641a278c1d3da..6cdb78d1a9413 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -93,8 +93,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { variable_source_info, true, )); - this.cfg.goto(lhs_success_block, variable_source_info, rhs_success_block); - rhs_success_block.unit() + + // Make the LHS and RHS success arms converge to a common block. + // (We can't just make LHS goto RHS, because `rhs_success_block` + // might contain statements that we don't want on the LHS path.) + let success_block = this.cfg.start_new_block(); + this.cfg.goto(lhs_success_block, variable_source_info, success_block); + this.cfg.goto(rhs_success_block, variable_source_info, success_block); + success_block.unit() } ExprKind::Unary { op: UnOp::Not, arg } => { let local_scope = this.local_scope(); diff --git a/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir b/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir index 89572177b1d28..fd8eb370ca958 100644 --- a/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir +++ b/tests/mir-opt/building/logical_or_in_conditional.test_complex.built.after.mir @@ -19,7 +19,7 @@ fn test_complex() -> () { bb0: { StorageLive(_1); StorageLive(_2); - _2 = E::f() -> [return: bb1, unwind: bb37]; + _2 = E::f() -> [return: bb1, unwind: bb38]; } bb1: { @@ -34,7 +34,7 @@ fn test_complex() -> () { } bb3: { - goto -> bb22; + goto -> bb23; } bb4: { @@ -51,7 +51,7 @@ fn test_complex() -> () { bb7: { StorageLive(_4); - _4 = always_true() -> [return: bb8, unwind: bb37]; + _4 = always_true() -> [return: bb8, unwind: bb38]; } bb8: { @@ -73,7 +73,7 @@ fn test_complex() -> () { } bb11: { - drop(_7) -> [return: bb13, unwind: bb37]; + drop(_7) -> [return: bb13, unwind: bb38]; } bb12: { @@ -83,11 +83,11 @@ fn test_complex() -> () { bb13: { StorageDead(_7); StorageDead(_6); - goto -> bb19; + goto -> bb20; } bb14: { - drop(_7) -> [return: bb15, unwind: bb37]; + drop(_7) -> [return: bb15, unwind: bb38]; } bb15: { @@ -107,106 +107,110 @@ fn test_complex() -> () { } bb17: { - drop(_10) -> [return: bb19, unwind: bb37]; + drop(_10) -> [return: bb19, unwind: bb38]; } bb18: { - goto -> bb20; + goto -> bb21; } bb19: { StorageDead(_10); StorageDead(_9); - _1 = const (); - goto -> bb23; + goto -> bb20; } bb20: { - drop(_10) -> [return: bb21, unwind: bb37]; + _1 = const (); + goto -> bb24; } bb21: { - StorageDead(_10); - StorageDead(_9); - goto -> bb22; + drop(_10) -> [return: bb22, unwind: bb38]; } bb22: { - _1 = const (); + StorageDead(_10); + StorageDead(_9); goto -> bb23; } bb23: { + _1 = const (); + goto -> bb24; + } + + bb24: { StorageDead(_8); StorageDead(_5); StorageDead(_4); StorageDead(_2); StorageDead(_1); StorageLive(_11); - _11 = always_true() -> [return: bb24, unwind: bb37]; - } - - bb24: { - switchInt(move _11) -> [0: bb26, otherwise: bb25]; + _11 = always_true() -> [return: bb25, unwind: bb38]; } bb25: { - goto -> bb35; + switchInt(move _11) -> [0: bb27, otherwise: bb26]; } bb26: { - goto -> bb27; + goto -> bb36; } bb27: { - StorageLive(_12); - _12 = E::f() -> [return: bb28, unwind: bb37]; + goto -> bb28; } bb28: { - PlaceMention(_12); - _13 = discriminant(_12); - switchInt(move _13) -> [1: bb32, otherwise: bb30]; + StorageLive(_12); + _12 = E::f() -> [return: bb29, unwind: bb38]; } bb29: { - FakeRead(ForMatchedPlace(None), _12); - unreachable; + PlaceMention(_12); + _13 = discriminant(_12); + switchInt(move _13) -> [1: bb33, otherwise: bb31]; } bb30: { - goto -> bb35; + FakeRead(ForMatchedPlace(None), _12); + unreachable; } bb31: { - goto -> bb29; + goto -> bb36; } bb32: { - falseEdge -> [real: bb34, imaginary: bb30]; + goto -> bb30; } bb33: { - goto -> bb30; + falseEdge -> [real: bb35, imaginary: bb31]; } bb34: { - _0 = const (); - goto -> bb36; + goto -> bb31; } bb35: { _0 = const (); - goto -> bb36; + goto -> bb37; } bb36: { + _0 = const (); + goto -> bb37; + } + + bb37: { StorageDead(_11); StorageDead(_12); return; } - bb37 (cleanup): { + bb38 (cleanup): { resume; } } diff --git a/tests/mir-opt/building/logical_or_in_conditional.test_or.built.after.mir b/tests/mir-opt/building/logical_or_in_conditional.test_or.built.after.mir index b84c17c21886e..3e7c116016cc1 100644 --- a/tests/mir-opt/building/logical_or_in_conditional.test_or.built.after.mir +++ b/tests/mir-opt/building/logical_or_in_conditional.test_or.built.after.mir @@ -20,7 +20,7 @@ fn test_or() -> () { } bb1: { - drop(_3) -> [return: bb3, unwind: bb12]; + drop(_3) -> [return: bb3, unwind: bb13]; } bb2: { @@ -30,11 +30,11 @@ fn test_or() -> () { bb3: { StorageDead(_3); StorageDead(_2); - goto -> bb8; + goto -> bb9; } bb4: { - drop(_3) -> [return: bb5, unwind: bb12]; + drop(_3) -> [return: bb5, unwind: bb13]; } bb5: { @@ -50,38 +50,42 @@ fn test_or() -> () { } bb6: { - drop(_6) -> [return: bb8, unwind: bb12]; + drop(_6) -> [return: bb8, unwind: bb13]; } bb7: { - goto -> bb9; + goto -> bb10; } bb8: { StorageDead(_6); StorageDead(_5); - _0 = const (); - goto -> bb11; + goto -> bb9; } bb9: { - drop(_6) -> [return: bb10, unwind: bb12]; + _0 = const (); + goto -> bb12; } bb10: { + drop(_6) -> [return: bb11, unwind: bb13]; + } + + bb11: { StorageDead(_6); StorageDead(_5); _0 = const (); - goto -> bb11; + goto -> bb12; } - bb11: { + bb12: { StorageDead(_4); StorageDead(_1); return; } - bb12 (cleanup): { + bb13 (cleanup): { resume; } }