Auto merge of rust-lang#138526 - scottmcm:no-const-none, r=<try>

Stop making `const None` in GVN

Having `ALLOC`s for these seems unhelpful, since it's extra stuff to keep track of, makes it harder for other passes to know what's in it (reversing a tag constant to a `VariantIdx` isn't trivial), and even in codegen it's arguably harder to get a `None` from a valtree than from seeing "oh, aggregate nothing with this `VariantIdx`".

Making constants for the `discriminant(_n)` is useful for putting in `switchInt`s, and still happens with this PR.  It's just for the full enum that we don't make consts (except for might-as-well-be-an-integer enums like `Ordering`, or ones that are actually ZSTs like `const Option::<Infallible>::None`).

This also adds a couple tests I've been using as I look at how we're doing enums in MIR.  They're not really *improved* by this change materially, but they help motivate why I'm trying to tweak how the enums come out.

Author: bors

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 __ __ __ __ __ __ __ __ │ ........░░░░░░░░
-  }
-