Auto merge of rust-lang#111344 - cjgillot:gvn-simplify, r=<try>

Perform opportunistic simplifications during value numbering

~Based on rust-lang#109597
~Based on rust-lang#119439

Opening mostly for discussion.

compiler/rustc_const_eval/src/interpret/cast.rs

@@ -416,7 +416,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         }
     }
 
-    fn unsize_into(
+    pub fn unsize_into(
         &mut self,
         src: &OpTy<'tcx, M::Provenance>,
         cast_ty: TyAndLayout<'tcx>,

compiler/rustc_mir_transform/src/dataflow_const_prop.rs

@@ -2,7 +2,9 @@
 //!
 //! Currently, this pass only propagates scalar values.
 
-use rustc_const_eval::interpret::{ImmTy, Immediate, InterpCx, OpTy, PlaceTy, Projectable};
+use rustc_const_eval::interpret::{
+    ImmTy, Immediate, InterpCx, OpTy, PlaceTy, Pointer, PointerArithmetic, Projectable,
+};
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def::DefKind;
 use rustc_middle::mir::interpret::{AllocId, ConstAllocation, InterpResult, Scalar};
@@ -935,12 +937,64 @@ impl<'mir, 'tcx: 'mir> rustc_const_eval::interpret::Machine<'mir, 'tcx> for Dumm
     }
 
     fn binary_ptr_op(
-        _ecx: &InterpCx<'mir, 'tcx, Self>,
-        _bin_op: BinOp,
-        _left: &rustc_const_eval::interpret::ImmTy<'tcx, Self::Provenance>,
-        _right: &rustc_const_eval::interpret::ImmTy<'tcx, Self::Provenance>,
+        ecx: &InterpCx<'mir, 'tcx, Self>,
+        bin_op: BinOp,
+        left: &rustc_const_eval::interpret::ImmTy<'tcx, Self::Provenance>,
+        right: &rustc_const_eval::interpret::ImmTy<'tcx, Self::Provenance>,
     ) -> interpret::InterpResult<'tcx, (ImmTy<'tcx, Self::Provenance>, bool)> {
-        throw_machine_stop_str!("can't do pointer arithmetic");
+        use rustc_middle::mir::BinOp::*;
+        Ok(match bin_op {
+            Eq | Ne | Lt | Le | Gt | Ge => {
+                assert_eq!(left.layout.abi, right.layout.abi); // types an differ, e.g. fn ptrs with different `for`
+                let size = ecx.pointer_size();
+                // Just compare the bits. ScalarPairs are compared lexicographically.
+                // We thus always compare pairs and simply fill scalars up with 0.
+                let left = match **left {
+                    Immediate::Scalar(l) => (l.to_bits(size)?, 0),
+                    Immediate::ScalarPair(l1, l2) => (l1.to_bits(size)?, l2.to_bits(size)?),
+                    Immediate::Uninit => panic!("we should never see uninit data here"),
+                };
+                let right = match **right {
+                    Immediate::Scalar(r) => (r.to_bits(size)?, 0),
+                    Immediate::ScalarPair(r1, r2) => (r1.to_bits(size)?, r2.to_bits(size)?),
+                    Immediate::Uninit => panic!("we should never see uninit data here"),
+                };
+                let res = match bin_op {
+                    Eq => left == right,
+                    Ne => left != right,
+                    Lt => left < right,
+                    Le => left <= right,
+                    Gt => left > right,
+                    Ge => left >= right,
+                    _ => bug!(),
+                };
+                (ImmTy::from_bool(res, *ecx.tcx), false)
+            }
+
+            // Some more operations are possible with atomics.
+            // The return value always has the provenance of the *left* operand.
+            Add | Sub | BitOr | BitAnd | BitXor => {
+                assert!(left.layout.ty.is_unsafe_ptr());
+                assert!(right.layout.ty.is_unsafe_ptr());
+                let ptr = left.to_scalar().to_pointer(ecx)?;
+                // We do the actual operation with usize-typed scalars.
+                let usize_layout = ecx.layout_of(ecx.tcx.types.usize).unwrap();
+                let left = ImmTy::from_uint(ptr.addr().bytes(), usize_layout);
+                let right = ImmTy::from_uint(right.to_scalar().to_target_usize(ecx)?, usize_layout);
+                let (result, overflowing) = ecx.overflowing_binary_op(bin_op, &left, &right)?;
+                // Construct a new pointer with the provenance of `ptr` (the LHS).
+                let result_ptr = Pointer::new(
+                    ptr.provenance,
+                    Size::from_bytes(result.to_scalar().to_target_usize(ecx)?),
+                );
+                (
+                    ImmTy::from_scalar(Scalar::from_maybe_pointer(result_ptr, ecx), left.layout),
+                    overflowing,
+                )
+            }
+
+            _ => span_bug!(ecx.cur_span(), "Invalid operator on pointers: {:?}", bin_op),
+        })
     }
 
     fn expose_ptr(

compiler/rustc_mir_transform/src/gvn.rs

@@ -93,7 +93,6 @@ use rustc_index::IndexVec;
 use rustc_middle::mir::interpret::GlobalAlloc;
 use rustc_middle::mir::visit::*;
 use rustc_middle::mir::*;
-use rustc_middle::ty::adjustment::PointerCoercion;
 use rustc_middle::ty::layout::LayoutOf;
 use rustc_middle::ty::{self, Ty, TyCtxt, TypeAndMut};
 use rustc_span::def_id::DefId;
@@ -154,6 +153,9 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
     state.next_opaque = None;
 
     let reverse_postorder = body.basic_blocks.reverse_postorder().to_vec();
+    for dbg in body.var_debug_info.iter_mut() {
+        state.visit_var_debug_info(dbg);
+    }
     for bb in reverse_postorder {
         let data = &mut body.basic_blocks.as_mut_preserves_cfg()[bb];
         state.visit_basic_block_data(bb, data);
@@ -551,6 +553,29 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                     }
                     value.offset(Size::ZERO, to, &self.ecx).ok()?
                 }
+                CastKind::PointerCoercion(ty::adjustment::PointerCoercion::Unsize) => {
+                    let src = self.evaluated[value].as_ref()?;
+                    let to = self.ecx.layout_of(to).ok()?;
+                    let dest = self.ecx.allocate(to, MemoryKind::Stack).ok()?;
+                    self.ecx.unsize_into(src, to, &dest.clone().into()).ok()?;
+                    self.ecx
+                        .alloc_mark_immutable(dest.ptr().provenance.unwrap().alloc_id())
+                        .ok()?;
+                    dest.into()
+                }
+                CastKind::FnPtrToPtr
+                | CastKind::PtrToPtr
+                | CastKind::PointerCoercion(
+                    ty::adjustment::PointerCoercion::MutToConstPointer
+                    | ty::adjustment::PointerCoercion::ArrayToPointer
+                    | ty::adjustment::PointerCoercion::UnsafeFnPointer,
+                ) => {
+                    let src = self.evaluated[value].as_ref()?;
+                    let src = self.ecx.read_immediate(src).ok()?;
+                    let to = self.ecx.layout_of(to).ok()?;
+                    let ret = self.ecx.ptr_to_ptr(&src, to).ok()?;
+                    ret.into()
+                }
                 _ => return None,
             },
         };
@@ -777,18 +802,8 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
 
             // Operations.
             Rvalue::Len(ref mut place) => return self.simplify_len(place, location),
-            Rvalue::Cast(kind, ref mut value, to) => {
-                let from = value.ty(self.local_decls, self.tcx);
-                let value = self.simplify_operand(value, location)?;
-                if let CastKind::PointerCoercion(
-                    PointerCoercion::ReifyFnPointer | PointerCoercion::ClosureFnPointer(_),
-                ) = kind
-                {
-                    // Each reification of a generic fn may get a different pointer.
-                    // Do not try to merge them.
-                    return self.new_opaque();
-                }
-                Value::Cast { kind, value, from, to }
+            Rvalue::Cast(ref mut kind, ref mut value, to) => {
+                return self.simplify_cast(kind, value, to, location);
             }
             Rvalue::BinaryOp(op, box (ref mut lhs, ref mut rhs)) => {
                 let ty = lhs.ty(self.local_decls, self.tcx);
@@ -876,6 +891,12 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             }
         }
 
+        let fields: Option<Vec<_>> = fields
+            .iter_mut()
+            .map(|op| self.simplify_operand(op, location).or_else(|| self.new_opaque()))
+            .collect();
+        let fields = fields?;
+
         let (ty, variant_index) = match *kind {
             AggregateKind::Array(..) => {
                 assert!(!fields.is_empty());
@@ -895,12 +916,6 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             AggregateKind::Adt(_, _, _, _, Some(_)) => return None,
         };
 
-        let fields: Option<Vec<_>> = fields
-            .iter_mut()
-            .map(|op| self.simplify_operand(op, location).or_else(|| self.new_opaque()))
-            .collect();
-        let fields = fields?;
-
         if let AggregateTy::Array = ty
             && fields.len() > 4
         {
@@ -1031,6 +1046,50 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
         }
     }
 
+    fn simplify_cast(
+        &mut self,
+        kind: &mut CastKind,
+        operand: &mut Operand<'tcx>,
+        to: Ty<'tcx>,
+        location: Location,
+    ) -> Option<VnIndex> {
+        use rustc_middle::ty::adjustment::PointerCoercion::*;
+        use CastKind::*;
+
+        let mut from = operand.ty(self.local_decls, self.tcx);
+        let mut value = self.simplify_operand(operand, location)?;
+        if from == to {
+            return Some(value);
+        }
+
+        if let CastKind::PointerCoercion(ReifyFnPointer | ClosureFnPointer(_)) = kind {
+            // Each reification of a generic fn may get a different pointer.
+            // Do not try to merge them.
+            return self.new_opaque();
+        }
+
+        if let PtrToPtr | PointerCoercion(MutToConstPointer) = kind
+            && let Value::Cast { kind: inner_kind, value: inner_value, from: inner_from, to: _ } =
+                *self.get(value)
+            && let PtrToPtr | PointerCoercion(MutToConstPointer) = inner_kind
+        {
+            from = inner_from;
+            value = inner_value;
+            *kind = PtrToPtr;
+            if inner_from == to {
+                return Some(inner_value);
+            }
+            if let Some(const_) = self.try_as_constant(value) {
+                *operand = Operand::Constant(Box::new(const_));
+            } else if let Some(local) = self.try_as_local(value, location) {
+                *operand = Operand::Copy(local.into());
+                self.reused_locals.insert(local);
+            }
+        }
+
+        Some(self.insert(Value::Cast { kind: *kind, value, from, to }))
+    }
+
     fn simplify_len(&mut self, place: &mut Place<'tcx>, location: Location) -> Option<VnIndex> {
         // Trivial case: we are fetching a statically known length.
         let place_ty = place.ty(self.local_decls, self.tcx).ty;
@@ -1178,6 +1237,51 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
         self.tcx
     }
 
+    fn visit_var_debug_info(&mut self, var_debug_info: &mut VarDebugInfo<'tcx>) {
+        let mut replace_dereffed = |place: &mut Place<'tcx>| -> Option<!> {
+            let last_deref = place.projection.iter().rposition(|e| e == PlaceElem::Deref)?;
+
+            // Another place that holds the same value.
+            let mut place_ref = place.as_ref();
+            let mut value = self.locals[place.local]?;
+
+            for (index, &proj) in place.projection[..last_deref].iter().enumerate() {
+                if let Some(candidates) = self.rev_locals.get(value)
+                    && let Some(&local) = candidates.first()
+                {
+                    place_ref = PlaceRef { local, projection: &place.projection[index..] };
+                }
+
+                let place_upto =
+                    PlaceRef { local: place.local, projection: &place.projection[..index] };
+                if let Some(projected) = self.project(place_upto, value, proj) {
+                    value = projected;
+                } else {
+                    if place_ref.projection.len() < place.projection.len() {
+                        *place = place_ref.project_deeper(&[], self.tcx);
+                    }
+                    return None;
+                }
+            }
+
+            if let Some(candidates) = self.rev_locals.get(value)
+                && let Some(&local) = candidates.first()
+            {
+                let place_ref = PlaceRef { local, projection: &place.projection[last_deref..] };
+                *place = place_ref.project_deeper(&[], self.tcx);
+            }
+
+            return None;
+        };
+
+        match &mut var_debug_info.value {
+            VarDebugInfoContents::Const(_) => {}
+            VarDebugInfoContents::Place(place) => {
+                replace_dereffed(place);
+            }
+        }
+    }
+
     fn visit_place(&mut self, place: &mut Place<'tcx>, _: PlaceContext, location: Location) {
         self.simplify_place_projection(place, location);
     }

tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-abort.diff

@@ -31,16 +31,17 @@
           StorageDead(_3);
           StorageLive(_6);
           _6 = const 1_usize;
-          _7 = Len((*_2));
+-         _7 = Len((*_2));
 -         _8 = Lt(_6, _7);
 -         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind unreachable];
-+         _8 = Lt(const 1_usize, _7);
-+         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind unreachable];
++         _7 = const 3_usize;
++         _8 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind unreachable];
       }
 
       bb1: {
 -         _1 = (*_2)[_6];
-+         _1 = (*_2)[1 of 2];
++         _1 = const 2_u32;
           StorageDead(_6);
           StorageDead(_4);
           StorageDead(_2);

tests/mir-opt/const_prop/slice_len.main.GVN.32bit.panic-unwind.diff

@@ -31,16 +31,17 @@
           StorageDead(_3);
           StorageLive(_6);
           _6 = const 1_usize;
-          _7 = Len((*_2));
+-         _7 = Len((*_2));
 -         _8 = Lt(_6, _7);
 -         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind continue];
-+         _8 = Lt(const 1_usize, _7);
-+         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind continue];
++         _7 = const 3_usize;
++         _8 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind continue];
       }
 
       bb1: {
 -         _1 = (*_2)[_6];
-+         _1 = (*_2)[1 of 2];
++         _1 = const 2_u32;
           StorageDead(_6);
           StorageDead(_4);
           StorageDead(_2);

tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-abort.diff

@@ -31,16 +31,17 @@
           StorageDead(_3);
           StorageLive(_6);
           _6 = const 1_usize;
-          _7 = Len((*_2));
+-         _7 = Len((*_2));
 -         _8 = Lt(_6, _7);
 -         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind unreachable];
-+         _8 = Lt(const 1_usize, _7);
-+         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind unreachable];
++         _7 = const 3_usize;
++         _8 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind unreachable];
       }
 
       bb1: {
 -         _1 = (*_2)[_6];
-+         _1 = (*_2)[1 of 2];
++         _1 = const 2_u32;
           StorageDead(_6);
           StorageDead(_4);
           StorageDead(_2);

tests/mir-opt/const_prop/slice_len.main.GVN.64bit.panic-unwind.diff

@@ -31,16 +31,17 @@
           StorageDead(_3);
           StorageLive(_6);
           _6 = const 1_usize;
-          _7 = Len((*_2));
+-         _7 = Len((*_2));
 -         _8 = Lt(_6, _7);
 -         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, _6) -> [success: bb1, unwind continue];
-+         _8 = Lt(const 1_usize, _7);
-+         assert(move _8, "index out of bounds: the length is {} but the index is {}", move _7, const 1_usize) -> [success: bb1, unwind continue];
++         _7 = const 3_usize;
++         _8 = const true;
++         assert(const true, "index out of bounds: the length is {} but the index is {}", const 3_usize, const 1_usize) -> [success: bb1, unwind continue];
       }
 
       bb1: {
 -         _1 = (*_2)[_6];
-+         _1 = (*_2)[1 of 2];
++         _1 = const 2_u32;
           StorageDead(_6);
           StorageDead(_4);
           StorageDead(_2);

tests/mir-opt/const_prop/slice_len.rs

@@ -8,9 +8,7 @@ fn main() {
     // CHECK-LABEL: fn main(
     // CHECK: debug a => [[a:_.*]];
     // CHECK: [[slice:_.*]] = const {{.*}} as &[u32] (PointerCoercion(Unsize));
-    // FIXME(cjgillot) simplify Len and projection into unsized slice.
-    // CHECK-NOT: assert(const true,
-    // CHECK: [[a]] = (*[[slice]])[1 of 2];
-    // CHECK-NOT: [[a]] = const 2_u32;
+    // CHECK: assert(const true,
+    // CHECK: [[a]] = const 2_u32;
     let a = (&[1u32, 2, 3] as &[u32])[1];
 }

tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.32bit.diff

@@ -12,8 +12,9 @@
           StorageLive(_1);
           StorageLive(_2);
 -         _2 = ();
+-         _1 = Union32 { value: move _2 };
 +         _2 = const ();
-          _1 = Union32 { value: move _2 };
++         _1 = Union32 { value: const () };
           StorageDead(_2);
           _0 = move _1 as u32 (Transmute);
           StorageDead(_1);