GVN away PtrToPtr-then-Transmute when possible

compiler/rustc_mir_transform/src/gvn.rs

@@ -978,12 +978,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                     // we can't always know exactly what the metadata are.
                     // To allow things like `*mut (?A, ?T)` <-> `*mut (?B, ?T)`,
                     // it's fine to get a projection as the type.
-                    // FIXME: Would it be worth trying to normalize, rather than
-                    // just accepting the projection?  Or are the types in the
-                    // Cast realistically about as normalized as we can get anyway?
                     Value::Cast { kind: CastKind::PtrToPtr, value: inner, from, to }
-                        if from.pointee_metadata_ty_or_projection(self.tcx)
+                        if self.pointers_have_same_metadata(*from, *to) =>
-                            == to.pointee_metadata_ty_or_projection(self.tcx) =>
                     {
                         arg_index = *inner;
                         was_updated = true;
@@ -1054,7 +1050,6 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
         rhs_operand: &mut Operand<'tcx>,
         location: Location,
     ) -> Option<VnIndex> {
-
         let lhs = self.simplify_operand(lhs_operand, location);
         let rhs = self.simplify_operand(rhs_operand, location);
         // Only short-circuit options after we called `simplify_operand`
@@ -1068,13 +1063,14 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
         // types of both casts and the metadata all match.
         if let BinOp::Eq | BinOp::Ne | BinOp::Lt | BinOp::Le | BinOp::Gt | BinOp::Ge = op
             && lhs_ty.is_any_ptr()
-            && let Value::Cast { kind: CastKind::PtrToPtr, value: lhs_value, from: lhs_from, .. } =
+            && let Value::Cast {
-                self.get(lhs)
+                kind: CastKind::PtrToPtr, value: lhs_value, from: lhs_from, ..
-            && let Value::Cast { kind: CastKind::PtrToPtr, value: rhs_value, from: rhs_from, .. } =
+            } = self.get(lhs)
-                self.get(rhs)
+            && let Value::Cast {
+                kind: CastKind::PtrToPtr, value: rhs_value, from: rhs_from, ..
+            } = self.get(rhs)
             && lhs_from == rhs_from
-            && lhs_from.pointee_metadata_ty_or_projection(self.tcx)
+            && self.pointers_have_same_metadata(*lhs_from, lhs_ty)
-                == lhs_ty.pointee_metadata_ty_or_projection(self.tcx)
         {
             lhs = *lhs_value;
             rhs = *rhs_value;
@@ -1254,6 +1250,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             }
         }
 
+        // PtrToPtr-then-PtrToPtr can skip the intermediate step
         if let PtrToPtr = kind
             && let Value::Cast { kind: inner_kind, value: inner_value, from: inner_from, to: _ } =
                 *self.get(value)
@@ -1261,7 +1258,25 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
         {
             from = inner_from;
             value = inner_value;
-            *kind = PtrToPtr;
+            was_updated = true;
+            if inner_from == to {
+                return Some(inner_value);
+            }
+        }
+
+        // PtrToPtr-then-Transmute can just transmute the original, so long as the
+        // PtrToPtr didn't change metadata (and thus the size of the pointer)
+        if let Transmute = kind
+            && let Value::Cast {
+                kind: PtrToPtr,
+                value: inner_value,
+                from: inner_from,
+                to: inner_to,
+            } = *self.get(value)
+            && self.pointers_have_same_metadata(inner_from, inner_to)
+        {
+            from = inner_from;
+            value = inner_value;
             was_updated = true;
             if inner_from == to {
                 return Some(inner_value);
@@ -1315,6 +1330,21 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
         // Fallback: a symbolic `Len`.
         Some(self.insert(Value::Len(inner)))
     }
+
+    fn pointers_have_same_metadata(&self, left_ptr_ty: Ty<'tcx>, right_ptr_ty: Ty<'tcx>) -> bool {
+        let left_meta_ty = left_ptr_ty.pointee_metadata_ty_or_projection(self.tcx);
+        let right_meta_ty = right_ptr_ty.pointee_metadata_ty_or_projection(self.tcx);
+        if left_meta_ty == right_meta_ty {
+            true
+        } else if let Ok(left) =
+            self.tcx.try_normalize_erasing_regions(self.param_env, left_meta_ty)
+            && let Ok(right) = self.tcx.try_normalize_erasing_regions(self.param_env, right_meta_ty)
+        {
+            left == right
+        } else {
+            false
+        }
+    }
 }
 
 fn op_to_prop_const<'tcx>(

tests/mir-opt/gvn.cast_pointer_then_transmute.GVN.panic-abort.diff

@@ -0,0 +1,47 @@
+- // MIR for `cast_pointer_then_transmute` before GVN
++ // MIR for `cast_pointer_then_transmute` after GVN
+  
+  fn cast_pointer_then_transmute(_1: *mut u32, _2: *mut [u8]) -> () {
+      debug thin => _1;
+      debug fat => _2;
+      let mut _0: ();
+      let _3: usize;
+      let mut _4: *const ();
+      let mut _5: *mut u32;
+      let mut _7: *const ();
+      let mut _8: *mut [u8];
+      scope 1 {
+          debug thin_addr => _3;
+          let _6: usize;
+          scope 2 {
+              debug fat_addr => _6;
+          }
+      }
+  
+      bb0: {
+          StorageLive(_3);
+          StorageLive(_4);
+          StorageLive(_5);
+          _5 = _1;
+-         _4 = move _5 as *const () (PtrToPtr);
++         _4 = _1 as *const () (PtrToPtr);
+          StorageDead(_5);
+-         _3 = move _4 as usize (Transmute);
++         _3 = _1 as usize (Transmute);
+          StorageDead(_4);
+          StorageLive(_6);
+          StorageLive(_7);
+          StorageLive(_8);
+          _8 = _2;
+-         _7 = move _8 as *const () (PtrToPtr);
++         _7 = _2 as *const () (PtrToPtr);
+          StorageDead(_8);
+          _6 = move _7 as usize (Transmute);
+          StorageDead(_7);
+          _0 = const ();
+          StorageDead(_6);
+          StorageDead(_3);
+          return;
+      }
+  }
+  

tests/mir-opt/gvn.cast_pointer_then_transmute.GVN.panic-unwind.diff

@@ -0,0 +1,47 @@
+- // MIR for `cast_pointer_then_transmute` before GVN
++ // MIR for `cast_pointer_then_transmute` after GVN
+  
+  fn cast_pointer_then_transmute(_1: *mut u32, _2: *mut [u8]) -> () {
+      debug thin => _1;
+      debug fat => _2;
+      let mut _0: ();
+      let _3: usize;
+      let mut _4: *const ();
+      let mut _5: *mut u32;
+      let mut _7: *const ();
+      let mut _8: *mut [u8];
+      scope 1 {
+          debug thin_addr => _3;
+          let _6: usize;
+          scope 2 {
+              debug fat_addr => _6;
+          }
+      }
+  
+      bb0: {
+          StorageLive(_3);
+          StorageLive(_4);
+          StorageLive(_5);
+          _5 = _1;
+-         _4 = move _5 as *const () (PtrToPtr);
++         _4 = _1 as *const () (PtrToPtr);
+          StorageDead(_5);
+-         _3 = move _4 as usize (Transmute);
++         _3 = _1 as usize (Transmute);
+          StorageDead(_4);
+          StorageLive(_6);
+          StorageLive(_7);
+          StorageLive(_8);
+          _8 = _2;
+-         _7 = move _8 as *const () (PtrToPtr);
++         _7 = _2 as *const () (PtrToPtr);
+          StorageDead(_8);
+          _6 = move _7 as usize (Transmute);
+          StorageDead(_7);
+          _0 = const ();
+          StorageDead(_6);
+          StorageDead(_3);
+          return;
+      }
+  }
+  

tests/mir-opt/gvn.rs

@@ -913,6 +913,19 @@ fn cast_pointer_eq(p1: *mut u8, p2: *mut u32, p3: *mut u32, p4: *mut [u32]) {
     // CHECK: _0 = const ();
 }
 
+// Transmuting can skip a pointer cast so long as it wasn't a fat-to-thin cast.
+unsafe fn cast_pointer_then_transmute(thin: *mut u32, fat: *mut [u8]) {
+    // CHECK-LABEL: fn cast_pointer_then_transmute
+
+    // CHECK: [[UNUSED:_.+]] = _1 as *const () (PtrToPtr);
+    // CHECK: = _1 as usize (Transmute);
+    let thin_addr: usize = std::intrinsics::transmute(thin as *const ());
+
+    // CHECK: [[TEMP2:_.+]] = _2 as *const () (PtrToPtr);
+    // CHECK: = move [[TEMP2]] as usize (Transmute);
+    let fat_addr: usize = std::intrinsics::transmute(fat as *const ());
+}
+
 fn main() {
     subexpression_elimination(2, 4, 5);
     wrap_unwrap(5);
@@ -981,3 +994,4 @@ fn identity<T>(x: T) -> T {
 // EMIT_MIR gvn.array_len.GVN.diff
 // EMIT_MIR gvn.generic_cast_metadata.GVN.diff
 // EMIT_MIR gvn.cast_pointer_eq.GVN.diff
+// EMIT_MIR gvn.cast_pointer_then_transmute.GVN.diff

tests/mir-opt/pre-codegen/slice_iter.slice_iter_generic_is_empty.PreCodegen.after.panic-abort.mir

@@ -9,7 +9,7 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
         let mut _8: *const T;
         scope 2 {
             let _4: std::ptr::NonNull<T>;
-            let _10: usize;
+            let _9: usize;
             scope 3 {
             }
             scope 4 {
@@ -24,7 +24,6 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
                 }
             }
             scope 5 (inlined std::ptr::const_ptr::<impl *const T>::addr) {
-                let mut _9: *const ();
                 scope 6 (inlined std::ptr::const_ptr::<impl *const T>::cast::<()>) {
                 }
             }
@@ -34,7 +33,8 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
     }
 
     bb0: {
-        StorageLive(_10);
+        StorageLive(_9);
+        StorageLive(_8);
         StorageLive(_4);
         switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
     }
@@ -61,20 +61,16 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
     }
 
     bb2: {
-        StorageLive(_8);
         _8 = ((*_1).1: *const T);
-        StorageLive(_9);
+        _9 = _8 as usize (Transmute);
-        _9 = _8 as *const () (PtrToPtr);
+        _0 = Eq(_9, const 0_usize);
-        _10 = move _9 as usize (Transmute);
-        StorageDead(_9);
-        StorageDead(_8);
-        _0 = Eq(_10, const 0_usize);
         goto -> bb3;
     }
 
     bb3: {
         StorageDead(_4);
-        StorageDead(_10);
+        StorageDead(_8);
+        StorageDead(_9);
         return;
     }
 }

tests/mir-opt/pre-codegen/slice_iter.slice_iter_generic_is_empty.PreCodegen.after.panic-unwind.mir

@@ -9,7 +9,7 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
         let mut _8: *const T;
         scope 2 {
             let _4: std::ptr::NonNull<T>;
-            let _10: usize;
+            let _9: usize;
             scope 3 {
             }
             scope 4 {
@@ -24,7 +24,6 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
                 }
             }
             scope 5 (inlined std::ptr::const_ptr::<impl *const T>::addr) {
-                let mut _9: *const ();
                 scope 6 (inlined std::ptr::const_ptr::<impl *const T>::cast::<()>) {
                 }
             }
@@ -34,7 +33,8 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
     }
 
     bb0: {
-        StorageLive(_10);
+        StorageLive(_9);
+        StorageLive(_8);
         StorageLive(_4);
         switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
     }
@@ -61,20 +61,16 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
     }
 
     bb2: {
-        StorageLive(_8);
         _8 = ((*_1).1: *const T);
-        StorageLive(_9);
+        _9 = _8 as usize (Transmute);
-        _9 = _8 as *const () (PtrToPtr);
+        _0 = Eq(_9, const 0_usize);
-        _10 = move _9 as usize (Transmute);
-        StorageDead(_9);
-        StorageDead(_8);
-        _0 = Eq(_10, const 0_usize);
         goto -> bb3;
     }
 
     bb3: {
         StorageDead(_4);
-        StorageDead(_10);
+        StorageDead(_8);
+        StorageDead(_9);
         return;
     }
 }