Stop making const None in GVN

compiler/rustc_mir_transform/src/gvn.rs

@@ -1700,6 +1700,21 @@ impl<'tcx> VnState<'_, 'tcx> {
             return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value });
         }
 
+        // When an enum can have a payload, it's not worth turning into a constant.
+        // Making a bunch of EvalCx allocations for `Option::None`s isn't productive
+        // and just makes it harder for subsequent passes to see the Discriminants.
+        // It's still easy for codegen to build the aggregate from the fields, since
+        // for an enum that's just the fields and maybe the constant tag.
+        // (We don't ever make consts for things bigger than a ScalarPair anyway.)
+        if let Value::Aggregate(agg_ty, _, _) = *self.get(index)
+            && let AggregateTy::Def(did, _) = agg_ty
+            && let DefKind::Enum = self.tcx.def_kind(did)
+            && let adt = self.tcx.adt_def(did)
+            && !adt.is_payloadfree()
+        {
+            return None;
+        }
+
         let op = self.evaluated[index].as_ref()?;
         if op.layout.is_unsized() {
             // Do not attempt to propagate unsized locals.

tests/mir-opt/const_debuginfo.main.SingleUseConsts.diff

@@ -32,10 +32,10 @@
                           let _10: (bool, bool, u32);
                           scope 6 {
                               debug f => _10;
-                              let _11: std::option::Option<u16>;
+                              let _11: MyEnum;
                               scope 7 {
 -                                 debug o => _11;
-+                                 debug o => const Option::<u16>::Some(99_u16);
++                                 debug o => const MyEnum::Bar;
                                   let _12: Point;
                                   scope 8 {
 -                                     debug p => _12;
@@ -89,7 +89,7 @@
           StorageLive(_10);
           _10 = (const true, const false, const 123_u32);
           StorageLive(_11);
--         _11 = const Option::<u16>::Some(99_u16);
+-         _11 = const MyEnum::Bar;
 +         nop;
           StorageLive(_12);
 -         _12 = const Point {{ x: 32_u32, y: 32_u32 }};
@@ -121,5 +121,3 @@
 
   ALLOC0 (size: 8, align: 4) { .. }
 
-  ALLOC1 (size: 4, align: 2) { .. }
-

tests/mir-opt/const_debuginfo.rs

@@ -8,6 +8,13 @@ struct Point {
     y: u32,
 }
 
+#[repr(u16)]
+enum MyEnum {
+    Foo = 88,
+    Bar = 99,
+    Baz = 111,
+}
+
 // EMIT_MIR const_debuginfo.main.SingleUseConsts.diff
 fn main() {
     // CHECK-LABEL: fn main(
@@ -17,7 +24,7 @@ fn main() {
     // CHECK: debug sum => const 6_u8;
     // CHECK: debug s => const "hello, world!";
     // CHECK: debug f => {{_.*}};
-    // CHECK: debug o => const Option::<u16>::Some(99_u16);
+    // CHECK: debug o => const MyEnum::Bar;
     // CHECK: debug p => const Point
     // CHECK: debug a => const 64_u32;
     let x = 1u8;
@@ -29,7 +36,7 @@ fn main() {
 
     let f = (true, false, 123u32);
 
-    let o = Some(99u16);
+    let o = MyEnum::Bar;
 
     let p = Point { x: 32, y: 32 };
     let a = p.x + p.y;

tests/mir-opt/const_prop/discriminant.main.GVN.32bit.diff

@@ -17,10 +17,9 @@
           StorageLive(_1);
           StorageLive(_2);
           StorageLive(_3);
--         _3 = Option::<bool>::Some(const true);
+          _3 = Option::<bool>::Some(const true);
 -         _4 = discriminant(_3);
 -         switchInt(move _4) -> [1: bb1, otherwise: bb3];
-+         _3 = const Option::<bool>::Some(true);
 +         _4 = const 1_isize;
 +         switchInt(const 1_isize) -> [1: bb1, otherwise: bb3];
       }

tests/mir-opt/const_prop/discriminant.main.GVN.64bit.diff

@@ -17,10 +17,9 @@
           StorageLive(_1);
           StorageLive(_2);
           StorageLive(_3);
--         _3 = Option::<bool>::Some(const true);
+          _3 = Option::<bool>::Some(const true);
 -         _4 = discriminant(_3);
 -         switchInt(move _4) -> [1: bb1, otherwise: bb3];
-+         _3 = const Option::<bool>::Some(true);
 +         _4 = const 1_isize;
 +         switchInt(const 1_isize) -> [1: bb1, otherwise: bb3];
       }

tests/mir-opt/const_prop/while_let_loops.change_loop_body.GVN.diff

@@ -21,10 +21,9 @@
           StorageLive(_1);
           _1 = const 0_i32;
           StorageLive(_3);
--         _3 = Option::<u32>::None;
+          _3 = Option::<u32>::None;
 -         _4 = discriminant(_3);
 -         switchInt(move _4) -> [1: bb1, otherwise: bb3];
-+         _3 = const Option::<u32>::None;
 +         _4 = const 0_isize;
 +         switchInt(const 0_isize) -> [1: bb1, otherwise: bb3];
       }

tests/mir-opt/funky_arms.float_to_exponential_common.GVN.32bit.panic-abort.diff

@@ -160,7 +160,7 @@
       }
 
       bb11: {
-          _6 = const Option::<usize>::None;
+          _6 = Option::<usize>::None;
           goto -> bb9;
       }
 
@@ -174,7 +174,3 @@
       }
   }
 
-  ALLOC0 (size: 8, align: 4) {
-      00 00 00 00 __ __ __ __                         │ ....░░░░
-  }
-

tests/mir-opt/funky_arms.float_to_exponential_common.GVN.32bit.panic-unwind.diff

@@ -160,7 +160,7 @@
       }
 
       bb11: {
-          _6 = const Option::<usize>::None;
+          _6 = Option::<usize>::None;
           goto -> bb9;
       }
 
@@ -174,7 +174,3 @@
       }
   }
 
-  ALLOC0 (size: 8, align: 4) {
-      00 00 00 00 __ __ __ __                         │ ....░░░░
-  }
-

tests/mir-opt/funky_arms.float_to_exponential_common.GVN.64bit.panic-abort.diff

@@ -160,7 +160,7 @@
       }
 
       bb11: {
-          _6 = const Option::<usize>::None;
+          _6 = Option::<usize>::None;
           goto -> bb9;
       }
 
@@ -174,7 +174,3 @@
       }
   }
 
-  ALLOC0 (size: 16, align: 8) {
-      00 00 00 00 00 00 00 00 __ __ __ __ __ __ __ __ │ ........░░░░░░░░
-  }
-

tests/mir-opt/funky_arms.float_to_exponential_common.GVN.64bit.panic-unwind.diff

@@ -160,7 +160,7 @@
       }
 
       bb11: {
-          _6 = const Option::<usize>::None;
+          _6 = Option::<usize>::None;
           goto -> bb9;
       }
 
@@ -174,7 +174,3 @@
       }
   }
 
-  ALLOC0 (size: 16, align: 8) {
-      00 00 00 00 00 00 00 00 __ __ __ __ __ __ __ __ │ ........░░░░░░░░
-  }
-

tests/mir-opt/pre-codegen/checked_ops.checked_shl.PreCodegen.after.panic-abort.mir

@@ -22,7 +22,7 @@ fn checked_shl(_1: u32, _2: u32) -> Option<u32> {
     }
 
     bb1: {
-        _0 = const Option::<u32>::None;
+        _0 = Option::<u32>::None;
         goto -> bb3;
     }
 
@@ -39,7 +39,3 @@ fn checked_shl(_1: u32, _2: u32) -> Option<u32> {
         return;
     }
 }
-
-ALLOC0 (size: 8, align: 4) {
-    00 00 00 00 __ __ __ __                         │ ....░░░░
-}

tests/mir-opt/pre-codegen/checked_ops.checked_shl.PreCodegen.after.panic-unwind.mir

@@ -22,7 +22,7 @@ fn checked_shl(_1: u32, _2: u32) -> Option<u32> {
     }
 
     bb1: {
-        _0 = const Option::<u32>::None;
+        _0 = Option::<u32>::None;
         goto -> bb3;
     }
 
@@ -39,7 +39,3 @@ fn checked_shl(_1: u32, _2: u32) -> Option<u32> {
         return;
     }
 }
-
-ALLOC0 (size: 8, align: 4) {
-    00 00 00 00 __ __ __ __                         │ ....░░░░
-}

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,44 @@
 // 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.saturating_sub_at_home.PreCodegen.after.mir
+// EMIT_MIR checked_ops.saturating_sub_at_home.GVN.diff
+pub fn saturating_sub_at_home(lhs: u32, rhs: u32) -> u32 {
+    // CHECK-LABEL: fn saturating_sub_at_home
+    // FIXME-CHECK-NOT: ={{.+}}::Some
+    // FIXME-CHECK-NOT: ={{.+}}::None
+    u32::checked_sub(lhs, rhs).unwrap_or(0)
+}
+
+// EMIT_MIR checked_ops.use_checked_sub.PreCodegen.after.mir
+// 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.saturating_sub_at_home.GVN.panic-abort.diff

@@ -0,0 +1,85 @@
+- // MIR for `saturating_sub_at_home` before GVN
++ // MIR for `saturating_sub_at_home` after GVN
+
+  fn saturating_sub_at_home(_1: u32, _2: u32) -> u32 {
+      debug lhs => _1;
+      debug rhs => _2;
+      let mut _0: u32;
+      let mut _3: std::option::Option<u32>;
+      let mut _4: u32;
+      let mut _5: u32;
+      let mut _8: u32;
+      scope 1 (inlined core::num::<impl u32>::checked_sub) {
+          let mut _6: bool;
+          let mut _7: u32;
+      }
+      scope 2 (inlined Option::<u32>::unwrap_or) {
+          let mut _9: isize;
+          let _10: u32;
+          scope 3 {
+          }
+      }
+
+      bb0: {
+          StorageLive(_3);
+          StorageLive(_4);
+          _4 = copy _1;
+          StorageLive(_5);
+          _5 = copy _2;
+          StorageLive(_6);
+-         _6 = Lt(copy _4, copy _5);
++         _6 = Lt(copy _1, copy _2);
+          switchInt(move _6) -> [0: bb2, otherwise: bb1];
+      }
+
+      bb1: {
+          _3 = Option::<u32>::None;
+          goto -> bb3;
+      }
+
+      bb2: {
+          StorageLive(_7);
+-         _7 = SubUnchecked(copy _4, copy _5);
++         _7 = SubUnchecked(copy _1, copy _2);
+          _3 = Option::<u32>::Some(move _7);
+          StorageDead(_7);
+          goto -> bb3;
+      }
+
+      bb3: {
+          StorageDead(_6);
+          StorageDead(_5);
+          StorageDead(_4);
+          StorageLive(_8);
+          _8 = const 0_u32;
+          StorageLive(_9);
+          _9 = discriminant(_3);
+          switchInt(move _9) -> [0: bb6, 1: bb7, otherwise: bb5];
+      }
+
+      bb4: {
+          StorageDead(_9);
+          StorageDead(_8);
+          StorageDead(_3);
+          return;
+      }
+
+      bb5: {
+          unreachable;
+      }
+
+      bb6: {
+-         _0 = move _8;
++         _0 = const 0_u32;
+          goto -> bb4;
+      }
+
+      bb7: {
+          StorageLive(_10);
+          _10 = move ((_3 as Some).0: u32);
+          _0 = move _10;
+          StorageDead(_10);
+          goto -> bb4;
+      }
+  }
+