Auto merge of rust-lang#137702 - scottmcm:deconstruct-options, r=<try>

[nothing to see here] this is probably a bad idea but I'm curious

r? ghost

Author: bors

compiler/rustc_mir_transform/src/branch_duplicator.rs

@@ -0,0 +1,88 @@
+use std::mem;
+
+use rustc_index::IndexVec;
+use rustc_middle::bug;
+use rustc_middle::mir::*;
+use rustc_middle::ty::TyCtxt;
+use tracing::{debug, instrument, trace};
+
+pub(super) struct BranchDuplicator;
+
+impl<'tcx> crate::MirPass<'tcx> for BranchDuplicator {
+    fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
+        sess.mir_opt_level() >= 2
+    }
+
+    #[instrument(skip_all level = "debug")]
+    fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
+        let def_id = body.source.def_id();
+        debug!(?def_id);
+
+        // Optimizing coroutines creates query cycles.
+        if tcx.is_coroutine(def_id) {
+            trace!("Skipped for coroutine {:?}", def_id);
+            return;
+        }
+
+        let is_branch = |targets: &SwitchTargets| {
+            targets.all_targets().len() == 2
+                || (targets.all_values().len() == 2
+                    && body.basic_blocks[targets.otherwise()].is_empty_unreachable())
+        };
+
+        let mut candidates = Vec::new();
+        for (bb, bbdata) in body.basic_blocks.iter_enumerated() {
+            if let TerminatorKind::SwitchInt { targets, .. } = &bbdata.terminator().kind
+                && is_branch(targets)
+                && let Ok(preds) =
+                    <[BasicBlock; 2]>::try_from(body.basic_blocks.predecessors()[bb].as_slice())
+                && preds.iter().copied().all(|p| {
+                    matches!(body.basic_blocks[p].terminator().kind, TerminatorKind::Goto { .. })
+                })
+                && bbdata.statements.iter().all(|x| is_negligible(&x.kind))
+            {
+                candidates.push((bb, preds));
+            }
+        }
+
+        if candidates.is_empty() {
+            return;
+        }
+
+        let basic_blocks = body.basic_blocks.as_mut();
+        for (bb, [p0, p1]) in candidates {
+            let bbdata = &mut basic_blocks[bb];
+            let statements = mem::take(&mut bbdata.statements);
+            let unreachable = Terminator {
+                source_info: bbdata.terminator().source_info,
+                kind: TerminatorKind::Unreachable,
+            };
+            let terminator = mem::replace(bbdata.terminator_mut(), unreachable);
+
+            let pred0data = &mut basic_blocks[p0];
+            pred0data.statements.extend(statements.iter().cloned());
+            *pred0data.terminator_mut() = terminator.clone();
+
+            let pred1data = &mut basic_blocks[p1];
+            pred1data.statements.extend(statements);
+            *pred1data.terminator_mut() = terminator;
+        }
+    }
+
+    fn is_required(&self) -> bool {
+        false
+    }
+}
+
+fn is_negligible<'tcx>(stmt: &StatementKind<'tcx>) -> bool {
+    use Rvalue::*;
+    use StatementKind::*;
+    match stmt {
+        StorageLive(..) | StorageDead(..) => true,
+        Assign(place_and_rvalue) => match &place_and_rvalue.1 {
+            Ref(..) | RawPtr(..) | Discriminant(..) | NullaryOp(..) => true,
+            _ => false,
+        },
+        _ => false,
+    }
+}

compiler/rustc_mir_transform/src/lib.rs

@@ -118,6 +118,7 @@ declare_passes! {
     mod add_moves_for_packed_drops : AddMovesForPackedDrops;
     mod add_retag : AddRetag;
     mod add_subtyping_projections : Subtyper;
+    mod branch_duplicator: BranchDuplicator;
     mod check_inline : CheckForceInline;
     mod check_call_recursion : CheckCallRecursion, CheckDropRecursion;
     mod check_alignment : CheckAlignment;
@@ -670,6 +671,9 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
             &unreachable_enum_branching::UnreachableEnumBranching,
             &unreachable_prop::UnreachablePropagation,
             &o1(simplify::SimplifyCfg::AfterUnreachableEnumBranching),
+            // If we inlined something returning `Option` or `Result`,
+            // duplicating the discriminant branches on those helps later passes.
+            &branch_duplicator::BranchDuplicator,
             // Inlining may have introduced a lot of redundant code and a large move pattern.
             // Now, we need to shrink the generated MIR.
             &ref_prop::ReferencePropagation,

tests/mir-opt/const_goto_const_eval_fail.f.JumpThreading.diff

@@ -12,36 +12,27 @@
 
       bb1: {
           _1 = const true;
--         goto -> bb3;
-+         goto -> bb7;
+          goto -> bb3;
       }
 
       bb2: {
           _1 = const B;
-          goto -> bb3;
+          switchInt(copy _1) -> [0: bb4, otherwise: bb3];
       }
 
       bb3: {
-          switchInt(copy _1) -> [0: bb5, otherwise: bb4];
-      }
-  
-      bb4: {
           _0 = const 2_u64;
-          goto -> bb6;
+          goto -> bb5;
       }
 
-      bb5: {
+      bb4: {
           _0 = const 1_u64;
-          goto -> bb6;
+          goto -> bb5;
       }
 
-      bb6: {
+      bb5: {
           StorageDead(_1);
           return;
-+     }
-+ 
-+     bb7: {
-+         goto -> bb4;
       }
   }
 

tests/mir-opt/pre-codegen/checked_ops.rs

@@ -1,4 +1,3 @@
-// skip-filecheck
 //@ compile-flags: -O -Zmir-opt-level=2
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
 
@@ -8,10 +7,36 @@
 // EMIT_MIR checked_ops.step_forward.PreCodegen.after.mir
 pub fn step_forward(x: u16, n: usize) -> u16 {
     // This uses `u16` so that the conversion to usize is always widening.
+
+    // CHECK-LABEL: fn step_forward
+    // CHECK: inlined{{.+}}forward
     std::iter::Step::forward(x, n)
 }
 
 // EMIT_MIR checked_ops.checked_shl.PreCodegen.after.mir
 pub fn checked_shl(x: u32, rhs: u32) -> Option<u32> {
+    // CHECK-LABEL: fn checked_shl
+    // CHECK: [[TEMP:_[0-9]+]] = ShlUnchecked(copy _1, copy _2)
+    // CHECK: _0 = Option::<u32>::Some({{move|copy}} [[TEMP]])
     x.checked_shl(rhs)
 }
+
+// EMIT_MIR checked_ops.use_checked_sub.PreCodegen.after.mir
+// EMIT_MIR checked_ops.use_checked_sub.BranchDuplicator.diff
+// EMIT_MIR checked_ops.use_checked_sub.GVN.diff
+pub fn use_checked_sub(x: u32, rhs: u32) {
+    // We want this to be equivalent to open-coding it, leaving no `Option`s around.
+
+    // CHECK-LABEL: fn use_checked_sub
+    // FIXME-CHECK-NOT: let{{.+}}Option
+    // CHECK: inlined{{.+}}u32{{.+}}checked_sub
+    // CHECK: [[DELTA:_[0-9]+]] = SubUnchecked(copy _1, copy _2)
+    // FIXME-CHECK: do_something({{move|copy}} [[DELTA]])
+    if let Some(delta) = x.checked_sub(rhs) {
+        do_something(delta);
+    }
+}
+
+unsafe extern "Rust" {
+    safe fn do_something(_: u32);
+}

tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-abort.diff

@@ -0,0 +1,91 @@
+- // MIR for `use_checked_sub` before BranchDuplicator
++ // MIR for `use_checked_sub` after BranchDuplicator
+  
+  fn use_checked_sub(_1: u32, _2: u32) -> () {
+      debug x => _1;
+      debug rhs => _2;
+      let mut _0: ();
+      let mut _3: std::option::Option<u32>;
+      let mut _4: u32;
+      let mut _5: u32;
+      let mut _6: isize;
+      let _8: ();
+      let mut _9: u32;
+      scope 1 {
+          debug delta => _7;
+          let _7: u32;
+          scope 2 (inlined core::num::<impl u32>::checked_sub) {
+              let mut _10: bool;
+              let mut _11: u32;
+          }
+      }
+  
+      bb0: {
+          StorageLive(_3);
+          StorageLive(_4);
+          _4 = copy _1;
+          StorageLive(_5);
+          _5 = copy _2;
+          StorageLive(_10);
+          _10 = Lt(copy _4, copy _5);
+          switchInt(move _10) -> [0: bb5, otherwise: bb4];
+      }
+  
+      bb1: {
+          StorageLive(_7);
+          _7 = copy ((_3 as Some).0: u32);
+          StorageLive(_9);
+          _9 = copy _7;
+          _8 = do_something(move _9) -> [return: bb2, unwind unreachable];
+      }
+  
+      bb2: {
+          StorageDead(_9);
+          StorageDead(_7);
+          goto -> bb3;
+      }
+  
+      bb3: {
+          StorageDead(_3);
+          return;
+      }
+  
+      bb4: {
+          _3 = const Option::<u32>::None;
+-         goto -> bb6;
++         StorageDead(_10);
++         StorageDead(_5);
++         StorageDead(_4);
++         _6 = discriminant(_3);
++         switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
+      }
+  
+      bb5: {
+          StorageLive(_11);
+          _11 = SubUnchecked(copy _4, copy _5);
+          _3 = Option::<u32>::Some(move _11);
+          StorageDead(_11);
+-         goto -> bb6;
+-     }
+- 
+-     bb6: {
+          StorageDead(_10);
+          StorageDead(_5);
+          StorageDead(_4);
+          _6 = discriminant(_3);
+          switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
++     }
++ 
++     bb6: {
++         unreachable;
+      }
+  
+      bb7: {
+          unreachable;
+      }
+  }
+  
+  ALLOC0 (size: 8, align: 4) {
+      00 00 00 00 __ __ __ __                         │ ....░░░░
+  }
+  

tests/mir-opt/pre-codegen/checked_ops.use_checked_sub.BranchDuplicator.panic-unwind.diff

@@ -0,0 +1,91 @@
+- // MIR for `use_checked_sub` before BranchDuplicator
++ // MIR for `use_checked_sub` after BranchDuplicator
+  
+  fn use_checked_sub(_1: u32, _2: u32) -> () {
+      debug x => _1;
+      debug rhs => _2;
+      let mut _0: ();
+      let mut _3: std::option::Option<u32>;
+      let mut _4: u32;
+      let mut _5: u32;
+      let mut _6: isize;
+      let _8: ();
+      let mut _9: u32;
+      scope 1 {
+          debug delta => _7;
+          let _7: u32;
+          scope 2 (inlined core::num::<impl u32>::checked_sub) {
+              let mut _10: bool;
+              let mut _11: u32;
+          }
+      }
+  
+      bb0: {
+          StorageLive(_3);
+          StorageLive(_4);
+          _4 = copy _1;
+          StorageLive(_5);
+          _5 = copy _2;
+          StorageLive(_10);
+          _10 = Lt(copy _4, copy _5);
+          switchInt(move _10) -> [0: bb5, otherwise: bb4];
+      }
+  
+      bb1: {
+          StorageLive(_7);
+          _7 = copy ((_3 as Some).0: u32);
+          StorageLive(_9);
+          _9 = copy _7;
+          _8 = do_something(move _9) -> [return: bb2, unwind continue];
+      }
+  
+      bb2: {
+          StorageDead(_9);
+          StorageDead(_7);
+          goto -> bb3;
+      }
+  
+      bb3: {
+          StorageDead(_3);
+          return;
+      }
+  
+      bb4: {
+          _3 = const Option::<u32>::None;
+-         goto -> bb6;
++         StorageDead(_10);
++         StorageDead(_5);
++         StorageDead(_4);
++         _6 = discriminant(_3);
++         switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
+      }
+  
+      bb5: {
+          StorageLive(_11);
+          _11 = SubUnchecked(copy _4, copy _5);
+          _3 = Option::<u32>::Some(move _11);
+          StorageDead(_11);
+-         goto -> bb6;
+-     }
+- 
+-     bb6: {
+          StorageDead(_10);
+          StorageDead(_5);
+          StorageDead(_4);
+          _6 = discriminant(_3);
+          switchInt(move _6) -> [1: bb1, 0: bb3, otherwise: bb7];
++     }
++ 
++     bb6: {
++         unreachable;
+      }
+  
+      bb7: {
+          unreachable;
+      }
+  }
+  
+  ALLOC0 (size: 8, align: 4) {
+      00 00 00 00 __ __ __ __                         │ ....░░░░
+  }
+