Don't drop Rvalue::WrapUnsafeBinder during GVN

Author: compiler-errors

compiler/rustc_mir_transform/src/gvn.rs

@@ -240,6 +240,7 @@ enum Value<'tcx> {
         from: Ty<'tcx>,
         to: Ty<'tcx>,
     },
+    WrapUnsafeBinder(VnIndex, Ty<'tcx>),
 }
 
 struct VnState<'body, 'tcx> {
@@ -580,36 +581,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                     let res = self.ecx.float_to_float_or_int(&value, to).discard_err()?;
                     res.into()
                 }
-                CastKind::Transmute => {
-                    let value = self.evaluated[value].as_ref()?;
-                    let to = self.ecx.layout_of(to).ok()?;
-                    // `offset` for immediates generally only supports projections that match the
-                    // type of the immediate. However, as a HACK, we exploit that it can also do
-                    // limited transmutes: it only works between types with the same layout, and
-                    // cannot transmute pointers to integers.
-                    if value.as_mplace_or_imm().is_right() {
-                        let can_transmute = match (value.layout.backend_repr, to.backend_repr) {
-                            (BackendRepr::Scalar(s1), BackendRepr::Scalar(s2)) => {
-                                s1.size(&self.ecx) == s2.size(&self.ecx)
-                                    && !matches!(s1.primitive(), Primitive::Pointer(..))
-                            }
-                            (BackendRepr::ScalarPair(a1, b1), BackendRepr::ScalarPair(a2, b2)) => {
-                                a1.size(&self.ecx) == a2.size(&self.ecx) &&
-                                b1.size(&self.ecx) == b2.size(&self.ecx) &&
-                                // The alignment of the second component determines its offset, so that also needs to match.
-                                b1.align(&self.ecx) == b2.align(&self.ecx) &&
-                                // None of the inputs may be a pointer.
-                                !matches!(a1.primitive(), Primitive::Pointer(..))
-                                    && !matches!(b1.primitive(), Primitive::Pointer(..))
-                            }
-                            _ => false,
-                        };
-                        if !can_transmute {
-                            return None;
-                        }
-                    }
-                    value.offset(Size::ZERO, to, &self.ecx).discard_err()?
-                }
+                CastKind::Transmute => self.transmute(value, to)?,
                 CastKind::PointerCoercion(ty::adjustment::PointerCoercion::Unsize, _) => {
                     let src = self.evaluated[value].as_ref()?;
                     let to = self.ecx.layout_of(to).ok()?;
@@ -635,10 +607,38 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 }
                 _ => return None,
             },
+            WrapUnsafeBinder(value, ty) => self.transmute(value, ty)?,
         };
         Some(op)
     }
 
+    fn transmute(&mut self, value: VnIndex, to: Ty<'tcx>) -> Option<OpTy<'tcx>> {
+        let value = self.evaluated[value].as_ref()?;
+        let to = self.ecx.layout_of(to).ok()?;
+        if value.as_mplace_or_imm().is_right() {
+            let can_transmute = match (value.layout.backend_repr, to.backend_repr) {
+                (BackendRepr::Scalar(s1), BackendRepr::Scalar(s2)) => {
+                    s1.size(&self.ecx) == s2.size(&self.ecx)
+                        && !matches!(s1.primitive(), Primitive::Pointer(..))
+                }
+                (BackendRepr::ScalarPair(a1, b1), BackendRepr::ScalarPair(a2, b2)) => {
+                    a1.size(&self.ecx) == a2.size(&self.ecx) &&
+                    b1.size(&self.ecx) == b2.size(&self.ecx) &&
+                    // The alignment of the second component determines its offset, so that also needs to match.
+                    b1.align(&self.ecx) == b2.align(&self.ecx) &&
+                    // None of the inputs may be a pointer.
+                    !matches!(a1.primitive(), Primitive::Pointer(..))
+                        && !matches!(b1.primitive(), Primitive::Pointer(..))
+                }
+                _ => false,
+            };
+            if !can_transmute {
+                return None;
+            }
+        }
+        Some(value.offset(Size::ZERO, to, &self.ecx).discard_err()?)
+    }
+
     fn project(
         &mut self,
         place: PlaceRef<'tcx>,
@@ -872,8 +872,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 self.simplify_place_projection(place, location);
                 return self.new_pointer(*place, AddressKind::Address(mutbl));
             }
-            Rvalue::WrapUnsafeBinder(ref mut op, _) => {
-                return self.simplify_operand(op, location);
+            Rvalue::WrapUnsafeBinder(ref mut op, ty) => {
+                let value = self.simplify_operand(op, location)?;
+                Value::WrapUnsafeBinder(value, ty)
             }
 
             // Operations.

tests/mir-opt/gvn_on_unsafe_binder.propagate.GVN.diff

@@ -0,0 +1,35 @@
+- // MIR for `propagate` before GVN
++ // MIR for `propagate` after GVN
+  
+  fn propagate() -> unsafe<> i32 {
+      let mut _0: unsafe<> i32;
+      let _1: i32;
+      let mut _3: i32;
+      scope 1 {
+          debug x => _1;
+          let _2: unsafe<> i32;
+          scope 2 {
+              debug binder => _2;
+          }
+      }
+  
+      bb0: {
+-         StorageLive(_1);
++         nop;
+          _1 = const 1_i32;
+          StorageLive(_2);
+          StorageLive(_3);
+-         _3 = copy _1;
+-         _2 = wrap_binder!(move _3; unsafe<> i32);
++         _3 = const 1_i32;
++         _2 = const {transmute(0x00000001): unsafe<> i32};
+          StorageDead(_3);
+-         _0 = move _2;
++         _0 = const {transmute(0x00000001): unsafe<> i32};
+          StorageDead(_2);
+-         StorageDead(_1);
++         nop;
+          return;
+      }
+  }
+  

tests/mir-opt/gvn_on_unsafe_binder.rs

@@ -0,0 +1,29 @@
+// skip-filecheck
+//@ test-mir-pass: GVN
+
+// EMIT_MIR gvn_on_unsafe_binder.test.GVN.diff
+// EMIT_MIR gvn_on_unsafe_binder.propagate.GVN.diff
+
+#![feature(unsafe_binders)]
+
+use std::unsafe_binder::wrap_binder;
+
+// Test for ICE <https://github.com/rust-lang/rust/issues/137846>.
+fn test() {
+    unsafe {
+        let x = 1;
+        let binder: unsafe<'a> &'a i32 = wrap_binder!(&x);
+    }
+}
+
+// Test that GVN propagates const values through unsafe binders.
+//
+// The lifetime `'a` is redundant (and doesn't print when we print out the type).
+// However, we need it so that rustfmt doesn't rip out the `unsafe<>` part for now.
+fn propagate() -> unsafe<'a> i32 {
+    unsafe {
+        let x = 1;
+        let binder: unsafe<'a> i32 = wrap_binder!(x);
+        binder
+    }
+}

tests/mir-opt/gvn_on_unsafe_binder.test.GVN.diff

@@ -0,0 +1,30 @@
+- // MIR for `test` before GVN
++ // MIR for `test` after GVN
+  
+  fn test() -> () {
+      let mut _0: ();
+      let _1: i32;
+      let mut _3: &i32;
+      scope 1 {
+          debug x => _1;
+          let _2: unsafe<'a> &'a i32;
+          scope 2 {
+              debug binder => _2;
+          }
+      }
+  
+      bb0: {
+          StorageLive(_1);
+          _1 = const 1_i32;
+          StorageLive(_2);
+          StorageLive(_3);
+          _3 = &_1;
+          _2 = wrap_binder!(move _3; unsafe<'a> &'a i32);
+          StorageDead(_3);
+          _0 = const ();
+          StorageDead(_2);
+          StorageDead(_1);
+          return;
+      }
+  }
+