Remove duplicate unreachable blocks

Author: saethlin

compiler/rustc_mir_transform/src/simplify.rs

@@ -28,7 +28,7 @@
 //! return.
 
 use crate::MirPass;
-use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
 use rustc_index::vec::{Idx, IndexVec};
 use rustc_middle::mir::coverage::*;
 use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
@@ -48,6 +48,7 @@ impl SimplifyCfg {
 
 pub fn simplify_cfg<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
     CfgSimplifier::new(body).simplify();
+    remove_duplicate_unreachable_blocks(tcx, body);
     remove_dead_blocks(tcx, body);
 
     // FIXME: Should probably be moved into some kind of pass manager
@@ -259,6 +260,47 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
     }
 }
 
+pub fn remove_duplicate_unreachable_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+    struct OptApplier<'tcx> {
+        tcx: TyCtxt<'tcx>,
+        duplicates: FxIndexSet<BasicBlock>,
+    }
+
+    impl<'tcx> MutVisitor<'tcx> for OptApplier<'tcx> {
+        fn tcx(&self) -> TyCtxt<'tcx> {
+            self.tcx
+        }
+
+        fn visit_terminator(&mut self, terminator: &mut Terminator<'tcx>, location: Location) {
+            for target in terminator.successors_mut() {
+                if self.duplicates.contains(target) {
+                    *target = self.duplicates[0];
+                }
+            }
+
+            self.super_terminator(terminator, location);
+        }
+    }
+
+    let unreachable_blocks = body
+        .basic_blocks
+        .iter_enumerated()
+        .filter(|(_, bb)| {
+            // CfgSimplifier::simplify leaves behind some unreachable basic blocks without a
+            // terminator. Those blocks will be deleted by remove_dead_blocks, but we run just
+            // before then so we need to handle missing terminators.
+            // We also need to prevent confusing cleanup and non-cleanup blocks. In practice we
+            // don't emit empty unreachable cleanup blocks, so this simple check suffices.
+            bb.terminator.is_some() && bb.is_empty_unreachable() && !bb.is_cleanup
+        })
+        .map(|(block, _)| block)
+        .collect::<FxIndexSet<_>>();
+
+    if unreachable_blocks.len() > 1 {
+        OptApplier { tcx, duplicates: unreachable_blocks }.visit_body(body);
+    }
+}
+
 pub fn remove_dead_blocks<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
     let reachable = traversal::reachable_as_bitset(body);
     let num_blocks = body.basic_blocks.len();

tests/mir-opt/building/async_await.b-{closure#0}.generator_resume.0.mir

@@ -90,7 +90,7 @@ fn b::{closure#0}(_1: Pin<&mut [async fn body@$DIR/async_await.rs:14:18: 17:2]>,
 
     bb0: {
         _39 = discriminant((*(_1.0: &mut [async fn body@$DIR/async_await.rs:14:18: 17:2]))); // scope 0 at $DIR/async_await.rs:+0:18: +3:2
-        switchInt(move _39) -> [0: bb1, 1: bb29, 3: bb27, 4: bb28, otherwise: bb30]; // scope 0 at $DIR/async_await.rs:+0:18: +3:2
+        switchInt(move _39) -> [0: bb1, 1: bb28, 3: bb26, 4: bb27, otherwise: bb29]; // scope 0 at $DIR/async_await.rs:+0:18: +3:2
     }
 
     bb1: {
@@ -263,7 +263,7 @@ fn b::{closure#0}(_1: Pin<&mut [async fn body@$DIR/async_await.rs:14:18: 17:2]>,
         StorageDead(_29);                // scope 5 at $DIR/async_await.rs:+2:13: +2:14
         StorageDead(_26);                // scope 5 at $DIR/async_await.rs:+2:13: +2:14
         _32 = discriminant(_25);         // scope 4 at $DIR/async_await.rs:+2:8: +2:14
-        switchInt(move _32) -> [0: bb22, 1: bb20, otherwise: bb21]; // scope 4 at $DIR/async_await.rs:+2:8: +2:14
+        switchInt(move _32) -> [0: bb21, 1: bb20, otherwise: bb9]; // scope 4 at $DIR/async_await.rs:+2:8: +2:14
     }
 
     bb20: {
@@ -281,10 +281,6 @@ fn b::{closure#0}(_1: Pin<&mut [async fn body@$DIR/async_await.rs:14:18: 17:2]>,
     }
 
     bb21: {
-        unreachable;                     // scope 4 at $DIR/async_await.rs:+2:8: +2:14
-    }
-
-    bb22: {
         StorageLive(_33);                // scope 4 at $DIR/async_await.rs:+2:5: +2:14
         _33 = ((_25 as Ready).0: ());    // scope 4 at $DIR/async_await.rs:+2:5: +2:14
         _37 = _33;                       // scope 6 at $DIR/async_await.rs:+2:5: +2:14
@@ -293,34 +289,34 @@ fn b::{closure#0}(_1: Pin<&mut [async fn body@$DIR/async_await.rs:14:18: 17:2]>,
         StorageDead(_28);                // scope 4 at $DIR/async_await.rs:+2:13: +2:14
         StorageDead(_25);                // scope 4 at $DIR/async_await.rs:+2:13: +2:14
         StorageDead(_24);                // scope 4 at $DIR/async_await.rs:+2:13: +2:14
-        goto -> bb24;                    // scope 0 at $DIR/async_await.rs:+2:13: +2:14
+        goto -> bb23;                    // scope 0 at $DIR/async_await.rs:+2:13: +2:14
     }
 
-    bb23: {
+    bb22: {
         StorageDead(_36);                // scope 4 at $DIR/async_await.rs:+2:13: +2:14
         _38 = move _35;                  // scope 4 at $DIR/async_await.rs:+2:8: +2:14
         StorageDead(_35);                // scope 4 at $DIR/async_await.rs:+2:13: +2:14
         _7 = const ();                   // scope 4 at $DIR/async_await.rs:+2:8: +2:14
         goto -> bb16;                    // scope 4 at $DIR/async_await.rs:+2:8: +2:14
     }
 
-    bb24: {
+    bb23: {
         nop;                             // scope 0 at $DIR/async_await.rs:+2:13: +2:14
-        goto -> bb25;                    // scope 0 at $DIR/async_await.rs:+3:1: +3:2
+        goto -> bb24;                    // scope 0 at $DIR/async_await.rs:+3:1: +3:2
     }
 
-    bb25: {
+    bb24: {
         StorageDead(_21);                // scope 0 at $DIR/async_await.rs:+3:1: +3:2
-        goto -> bb26;                    // scope 0 at $DIR/async_await.rs:+3:1: +3:2
+        goto -> bb25;                    // scope 0 at $DIR/async_await.rs:+3:1: +3:2
     }
 
-    bb26: {
+    bb25: {
         _0 = Poll::<()>::Ready(move _37); // scope 0 at $DIR/async_await.rs:+3:2: +3:2
         discriminant((*(_1.0: &mut [async fn body@$DIR/async_await.rs:14:18: 17:2]))) = 1; // scope 0 at $DIR/async_await.rs:+3:2: +3:2
         return;                          // scope 0 at $DIR/async_await.rs:+3:2: +3:2
     }
 
-    bb27: {
+    bb26: {
         StorageLive(_3);                 // scope 0 at $DIR/async_await.rs:+0:18: +3:2
         StorageLive(_4);                 // scope 0 at $DIR/async_await.rs:+0:18: +3:2
         StorageLive(_19);                // scope 0 at $DIR/async_await.rs:+0:18: +3:2
@@ -329,19 +325,19 @@ fn b::{closure#0}(_1: Pin<&mut [async fn body@$DIR/async_await.rs:14:18: 17:2]>,
         goto -> bb11;                    // scope 0 at $DIR/async_await.rs:+0:18: +3:2
     }
 
-    bb28: {
+    bb27: {
         StorageLive(_21);                // scope 0 at $DIR/async_await.rs:+0:18: +3:2
         StorageLive(_35);                // scope 0 at $DIR/async_await.rs:+0:18: +3:2
         StorageLive(_36);                // scope 0 at $DIR/async_await.rs:+0:18: +3:2
         _35 = move _2;                   // scope 0 at $DIR/async_await.rs:+0:18: +3:2
-        goto -> bb23;                    // scope 0 at $DIR/async_await.rs:+0:18: +3:2
+        goto -> bb22;                    // scope 0 at $DIR/async_await.rs:+0:18: +3:2
     }
 
-    bb29: {
-        assert(const false, "`async fn` resumed after completion") -> bb29; // scope 0 at $DIR/async_await.rs:+0:18: +3:2
+    bb28: {
+        assert(const false, "`async fn` resumed after completion") -> bb28; // scope 0 at $DIR/async_await.rs:+0:18: +3:2
     }
 
-    bb30: {
+    bb29: {
         unreachable;                     // scope 0 at $DIR/async_await.rs:+0:18: +3:2
     }
 }

tests/mir-opt/separate_const_switch.too_complex.SeparateConstSwitch.diff

@@ -61,33 +61,29 @@
 
       bb4: {
           _8 = discriminant(_2);           // scope 0 at $DIR/separate_const_switch.rs:+5:11: +10:6
-          switchInt(move _8) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 0 at $DIR/separate_const_switch.rs:+5:5: +10:6
+          switchInt(move _8) -> [0: bb6, 1: bb5, otherwise: bb2]; // scope 0 at $DIR/separate_const_switch.rs:+5:5: +10:6
       }
 
       bb5: {
           StorageLive(_11);                // scope 0 at $DIR/separate_const_switch.rs:+12:28: +12:29
           _11 = ((_2 as Break).0: usize);  // scope 0 at $DIR/separate_const_switch.rs:+12:28: +12:29
           _0 = Option::<i32>::None;        // scope 4 at $DIR/separate_const_switch.rs:+12:34: +12:38
           StorageDead(_11);                // scope 0 at $DIR/separate_const_switch.rs:+12:37: +12:38
-          goto -> bb8;                     // scope 0 at $DIR/separate_const_switch.rs:+12:37: +12:38
+          goto -> bb7;                     // scope 0 at $DIR/separate_const_switch.rs:+12:37: +12:38
       }
 
       bb6: {
-          unreachable;                     // scope 0 at $DIR/separate_const_switch.rs:+5:11: +10:6
-      }
-  
-      bb7: {
           StorageLive(_9);                 // scope 0 at $DIR/separate_const_switch.rs:+11:31: +11:32
           _9 = ((_2 as Continue).0: i32);  // scope 0 at $DIR/separate_const_switch.rs:+11:31: +11:32
           StorageLive(_10);                // scope 3 at $DIR/separate_const_switch.rs:+11:42: +11:43
           _10 = _9;                        // scope 3 at $DIR/separate_const_switch.rs:+11:42: +11:43
           _0 = Option::<i32>::Some(move _10); // scope 3 at $DIR/separate_const_switch.rs:+11:37: +11:44
           StorageDead(_10);                // scope 3 at $DIR/separate_const_switch.rs:+11:43: +11:44
           StorageDead(_9);                 // scope 0 at $DIR/separate_const_switch.rs:+11:43: +11:44
-          goto -> bb8;                     // scope 0 at $DIR/separate_const_switch.rs:+11:43: +11:44
+          goto -> bb7;                     // scope 0 at $DIR/separate_const_switch.rs:+11:43: +11:44
       }
 
-      bb8: {
+      bb7: {
           StorageDead(_2);                 // scope 0 at $DIR/separate_const_switch.rs:+14:1: +14:2
           return;                          // scope 0 at $DIR/separate_const_switch.rs:+14:2: +14:2
       }

tests/mir-opt/simplify_duplicate_unreachable_blocks.assert_nonzero_nonmax.SimplifyCfg-after-uninhabited-enum-branching.diff

@@ -0,0 +1,25 @@
+- // MIR for `assert_nonzero_nonmax` before SimplifyCfg-after-uninhabited-enum-branching
++ // MIR for `assert_nonzero_nonmax` after SimplifyCfg-after-uninhabited-enum-branching
+  
+  fn assert_nonzero_nonmax(_1: u8) -> u8 {
+      let mut _0: u8;                      // return place in scope 0 at $DIR/simplify_duplicate_unreachable_blocks.rs:+0:47: +0:49
+  
+      bb0: {
+-         switchInt(_1) -> [0: bb1, 255: bb2, otherwise: bb3]; // scope 0 at $DIR/simplify_duplicate_unreachable_blocks.rs:+3:13: +7:14
++         switchInt(_1) -> [0: bb1, 255: bb1, otherwise: bb2]; // scope 0 at $DIR/simplify_duplicate_unreachable_blocks.rs:+3:13: +7:14
+      }
+  
+      bb1: {
+          unreachable;                     // scope 0 at $DIR/simplify_duplicate_unreachable_blocks.rs:+10:13: +10:26
+      }
+  
+      bb2: {
+-         unreachable;                     // scope 0 at $DIR/simplify_duplicate_unreachable_blocks.rs:+13:13: +13:26
+-     }
+- 
+-     bb3: {
+          _0 = _1;                         // scope 0 at $DIR/simplify_duplicate_unreachable_blocks.rs:+16:13: +16:20
+          return;                          // scope 0 at $DIR/simplify_duplicate_unreachable_blocks.rs:+17:13: +17:21
+      }
+  }
+  

tests/mir-opt/simplify_duplicate_unreachable_blocks.rs

@@ -0,0 +1,30 @@
+#![feature(custom_mir, core_intrinsics)]
+#![crate_type = "lib"]
+
+use std::intrinsics::mir::*;
+
+// unit-test: SimplifyCfg-after-uninhabited-enum-branching
+
+// EMIT_MIR simplify_duplicate_unreachable_blocks.assert_nonzero_nonmax.SimplifyCfg-after-uninhabited-enum-branching.diff
+#[custom_mir(dialect = "runtime", phase = "post-cleanup")]
+pub unsafe fn assert_nonzero_nonmax(x: u8) -> u8 {
+    mir!(
+        {
+            match x {
+                0 => unreachable1,
+                u8::MAX => unreachable2,
+                _ => retblock,
+            }
+        }
+        unreachable1 = {
+            Unreachable()
+        }
+        unreachable2 = {
+            Unreachable()
+        }
+        retblock = {
+            RET = x;
+            Return()
+        }
+    )
+}

tests/mir-opt/try_identity_e2e.new.PreCodegen.after.mir

@@ -26,37 +26,37 @@ fn new(_1: Result<T, E>) -> Result<T, E> {
     bb0: {
         StorageLive(_2);                 // scope 0 at $DIR/try_identity_e2e.rs:+2:15: +7:10
         _3 = discriminant(_1);           // scope 0 at $DIR/try_identity_e2e.rs:+3:19: +3:20
-        switchInt(move _3) -> [0: bb2, 1: bb1, otherwise: bb5]; // scope 0 at $DIR/try_identity_e2e.rs:+3:13: +3:20
+        switchInt(move _3) -> [0: bb3, 1: bb1, otherwise: bb2]; // scope 0 at $DIR/try_identity_e2e.rs:+3:13: +3:20
     }
 
     bb1: {
         _5 = move ((_1 as Err).0: E);    // scope 0 at $DIR/try_identity_e2e.rs:+5:21: +5:22
         _2 = ControlFlow::<E, T>::Break(move _5); // scope 2 at $DIR/try_identity_e2e.rs:+5:27: +5:48
-        goto -> bb3;                     // scope 0 at $DIR/try_identity_e2e.rs:+5:47: +5:48
+        goto -> bb4;                     // scope 0 at $DIR/try_identity_e2e.rs:+5:47: +5:48
     }
 
     bb2: {
+        unreachable;                     // scope 0 at $DIR/try_identity_e2e.rs:+3:19: +3:20
+    }
+
+    bb3: {
         _4 = move ((_1 as Ok).0: T);     // scope 0 at $DIR/try_identity_e2e.rs:+4:20: +4:21
         _2 = ControlFlow::<E, T>::Continue(move _4); // scope 1 at $DIR/try_identity_e2e.rs:+4:26: +4:50
-        goto -> bb3;                     // scope 0 at $DIR/try_identity_e2e.rs:+4:49: +4:50
+        goto -> bb4;                     // scope 0 at $DIR/try_identity_e2e.rs:+4:49: +4:50
     }
 
-    bb3: {
+    bb4: {
         _6 = discriminant(_2);           // scope 0 at $DIR/try_identity_e2e.rs:+2:15: +7:10
-        switchInt(move _6) -> [0: bb6, 1: bb4, otherwise: bb5]; // scope 0 at $DIR/try_identity_e2e.rs:+2:9: +7:10
+        switchInt(move _6) -> [0: bb6, 1: bb5, otherwise: bb2]; // scope 0 at $DIR/try_identity_e2e.rs:+2:9: +7:10
     }
 
-    bb4: {
+    bb5: {
         _8 = move ((_2 as Break).0: E);  // scope 0 at $DIR/try_identity_e2e.rs:+9:32: +9:33
         _0 = Result::<T, E>::Err(move _8); // scope 4 at $DIR/try_identity_e2e.rs:+9:45: +9:51
         StorageDead(_2);                 // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2
         return;                          // scope 0 at $DIR/try_identity_e2e.rs:+12:1: +12:2
     }
 
-    bb5: {
-        unreachable;                     // scope 0 at $DIR/try_identity_e2e.rs:+2:15: +7:10
-    }
-
     bb6: {
         _7 = move ((_2 as Continue).0: T); // scope 0 at $DIR/try_identity_e2e.rs:+8:35: +8:36
         _0 = Result::<T, E>::Ok(move _7); // scope 0 at $DIR/try_identity_e2e.rs:+1:5: +11:6

tests/mir-opt/uninhabited_enum_branching.main.SimplifyCfg-after-uninhabited-enum-branching.after.mir

@@ -34,7 +34,7 @@ fn main() -> () {
         StorageLive(_7);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
         _7 = Test2::D;                   // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
         _8 = discriminant(_7);           // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
-        switchInt(move _8) -> [4: bb5, 5: bb3, otherwise: bb4]; // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:5: +7:19
+        switchInt(move _8) -> [4: bb4, 5: bb3, otherwise: bb2]; // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:5: +7:19
     }
 
     bb2: {
@@ -49,22 +49,18 @@ fn main() -> () {
                                          // + literal: Const { ty: &str, val: Value(Slice(..)) }
         _6 = &(*_9);                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:21: +9:24
         StorageDead(_9);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:23: +9:24
-        goto -> bb6;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:23: +9:24
+        goto -> bb5;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:23: +9:24
     }
 
     bb4: {
-        unreachable;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
-    }
-
-    bb5: {
         _6 = const "D";                  // scope 0 at $DIR/uninhabited_enum_branching.rs:+8:21: +8:24
                                          // mir::Constant
                                          // + span: $DIR/uninhabited_enum_branching.rs:27:21: 27:24
                                          // + literal: Const { ty: &str, val: Value(Slice(..)) }
-        goto -> bb6;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+8:21: +8:24
+        goto -> bb5;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+8:21: +8:24
     }
 
-    bb6: {
+    bb5: {
         StorageDead(_7);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+10:6: +10:7
         StorageDead(_6);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+10:6: +10:7
         _0 = const ();                   // scope 0 at $DIR/uninhabited_enum_branching.rs:+0:11: +11:2

tests/mir-opt/uninhabited_enum_branching.main.UninhabitedEnumBranching.diff

@@ -63,7 +63,7 @@
           StorageLive(_7);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
           _7 = Test2::D;                   // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
           _8 = discriminant(_7);           // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
-          switchInt(move _8) -> [4: bb8, 5: bb6, otherwise: bb7]; // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:5: +7:19
+          switchInt(move _8) -> [4: bb7, 5: bb6, otherwise: bb2]; // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:5: +7:19
       }
 
       bb6: {
@@ -74,22 +74,18 @@
                                            // + literal: Const { ty: &str, val: Value(Slice(..)) }
           _6 = &(*_9);                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:21: +9:24
           StorageDead(_9);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:23: +9:24
-          goto -> bb9;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:23: +9:24
+          goto -> bb8;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+9:23: +9:24
       }
 
       bb7: {
-          unreachable;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+7:11: +7:19
-      }
-  
-      bb8: {
           _6 = const "D";                  // scope 0 at $DIR/uninhabited_enum_branching.rs:+8:21: +8:24
                                            // mir::Constant
                                            // + span: $DIR/uninhabited_enum_branching.rs:27:21: 27:24
                                            // + literal: Const { ty: &str, val: Value(Slice(..)) }
-          goto -> bb9;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+8:21: +8:24
+          goto -> bb8;                     // scope 0 at $DIR/uninhabited_enum_branching.rs:+8:21: +8:24
       }
 
-      bb9: {
+      bb8: {
           StorageDead(_7);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+10:6: +10:7
           StorageDead(_6);                 // scope 0 at $DIR/uninhabited_enum_branching.rs:+10:6: +10:7
           _0 = const ();                   // scope 0 at $DIR/uninhabited_enum_branching.rs:+0:11: +11:2