WIP: Trying to promote const local array using GVN pass

compiler/rustc_const_eval/src/interpret/place.rs

@@ -442,7 +442,7 @@ where
 
     /// Take an operand, representing a pointer, and dereference it to a place.
     /// Corresponds to the `*` operator in Rust.
-    #[instrument(skip(self), level = "debug")]
+    #[instrument(skip(self), level = "trace")]
     pub fn deref_pointer(
         &self,
         src: &impl Readable<'tcx, M::Provenance>,
@@ -533,7 +533,7 @@ where
 
     /// Computes a place. You should only use this if you intend to write into this
     /// place; for reading, a more efficient alternative is `eval_place_to_op`.
-    #[instrument(skip(self), level = "debug")]
+    #[instrument(skip(self), level = "trace")]
     pub fn eval_place(
         &self,
         mir_place: mir::Place<'tcx>,
@@ -570,7 +570,7 @@ where
 
     /// Write an immediate to a place
     #[inline(always)]
-    #[instrument(skip(self), level = "debug")]
+    #[instrument(skip(self), level = "trace")]
     pub fn write_immediate(
         &mut self,
         src: Immediate<M::Provenance>,
@@ -808,7 +808,7 @@ where
     /// Copies the data from an operand to a place.
     /// `allow_transmute` indicates whether the layouts may disagree.
     #[inline(always)]
-    #[instrument(skip(self), level = "debug")]
+    #[instrument(skip(self), level = "trace")]
     fn copy_op_inner(
         &mut self,
         src: &impl Readable<'tcx, M::Provenance>,
@@ -837,7 +837,7 @@ where
     /// `allow_transmute` indicates whether the layouts may disagree.
     /// Also, if you use this you are responsible for validating that things get copied at the
     /// right type.
-    #[instrument(skip(self), level = "debug")]
+    #[instrument(skip(self), level = "trace")]
     fn copy_op_no_validate(
         &mut self,
         src: &impl Readable<'tcx, M::Provenance>,
@@ -914,7 +914,7 @@ where
     /// If the place currently refers to a local that doesn't yet have a matching allocation,
     /// create such an allocation.
     /// This is essentially `force_to_memplace`.
-    #[instrument(skip(self), level = "debug")]
+    #[instrument(skip(self), level = "trace")]
     pub fn force_allocation(
         &mut self,
         place: &PlaceTy<'tcx, M::Provenance>,

compiler/rustc_mir_transform/src/gvn.rs

@@ -367,7 +367,9 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
     #[instrument(level = "trace", skip(self), ret)]
     fn eval_to_const(&mut self, value: VnIndex) -> Option<OpTy<'tcx>> {
         use Value::*;
-        let op = match *self.get(value) {
+        let vvalue = self.get(value);
+        debug!(?vvalue);
+        let op = match *vvalue {
             Opaque(_) => return None,
             // Do not bother evaluating repeat expressions. This would uselessly consume memory.
             Repeat(..) => return None,
@@ -376,10 +378,11 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 self.ecx.eval_mir_constant(value, DUMMY_SP, None).ok()?
             }
             Aggregate(kind, variant, ref fields) => {
+                debug!(?kind, ?variant, ?fields);
                 let fields = fields
                     .iter()
                     .map(|&f| self.evaluated[f].as_ref())
-                    .collect::<Option<Vec<_>>>()?;
+                    .collect::<Option<Vec<&OpTy<'_>>>>()?;
                 let ty = match kind {
                     AggregateTy::Array => {
                         assert!(fields.len() > 0);
@@ -407,6 +410,32 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                     };
                     let ptr_imm = Immediate::new_pointer_with_meta(data, meta, &self.ecx);
                     ImmTy::from_immediate(ptr_imm, ty).into()
+                } else if matches!(kind, AggregateTy::Array) {
+                    let mut mplace = None;
+                    let alloc_id = self
+                        .ecx
+                        .intern_with_temp_alloc(ty, |ecx, dest| {
+                            for (field_index, op) in fields.iter().copied().enumerate() {
+                                // ignore nested arrays
+                                if let Either::Left(_) = op.as_mplace_or_imm() {
+                                    interpret::throw_inval!(TooGeneric);
+                                }
+                                let field_dest = ecx.project_field(dest, field_index)?;
+                                ecx.copy_op(op, &field_dest)?;
+                            }
+
+                            let dest =
+                                dest.assert_mem_place().map_provenance(|prov| prov.as_immutable());
+                            mplace.replace(dest);
+                            Ok(())
+                        })
+                        .ok()?;
+                    let GlobalAlloc::Memory(_alloc) = self.tcx.global_alloc(alloc_id) else {
+                        bug!()
+                    };
+                    let mplace = mplace.unwrap();
+                    debug!(?mplace);
+                    return Some(mplace.into());
                 } else if matches!(ty.abi, Abi::Scalar(..) | Abi::ScalarPair(..)) {
                     let dest = self.ecx.allocate(ty, MemoryKind::Stack).ok()?;
                     let variant_dest = if let Some(variant) = variant {
@@ -429,6 +458,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             }
 
             Projection(base, elem) => {
+                debug!(?base, ?elem);
                 let value = self.evaluated[base].as_ref()?;
                 let elem = match elem {
                     ProjectionElem::Deref => ProjectionElem::Deref,
@@ -450,6 +480,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
                 self.ecx.project(value, elem).ok()?
             }
             Address { place, kind, provenance: _ } => {
+                debug!(?place, ?kind);
                 if !place.is_indirect_first_projection() {
                     return None;
                 }
@@ -709,7 +740,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
             place.projection = self.tcx.mk_place_elems(&projection);
         }
 
-        trace!(?place);
+        trace!(after_place = ?place);
     }
 
     /// Represent the *value* which would be read from `place`, and point `place` to a preexisting
@@ -1233,6 +1264,7 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
     }
 }
 
+#[instrument(level = "trace", skip(ecx), ret)]
 fn op_to_prop_const<'tcx>(
     ecx: &mut InterpCx<'tcx, DummyMachine>,
     op: &OpTy<'tcx>,
@@ -1248,7 +1280,8 @@ fn op_to_prop_const<'tcx>(
     }
 
     // Do not synthetize too large constants. Codegen will just memcpy them, which we'd like to avoid.
-    if !matches!(op.layout.abi, Abi::Scalar(..) | Abi::ScalarPair(..)) {
+    if !(op.layout.ty.is_array() || matches!(op.layout.abi, Abi::Scalar(..) | Abi::ScalarPair(..)))
+    {
         return None;
     }
 
@@ -1302,21 +1335,42 @@ fn op_to_prop_const<'tcx>(
 
 impl<'tcx> VnState<'_, 'tcx> {
     /// If `index` is a `Value::Constant`, return the `Constant` to be put in the MIR.
+    #[instrument(level = "trace", skip(self, index), ret)]
     fn try_as_constant(&mut self, index: VnIndex) -> Option<ConstOperand<'tcx>> {
         // This was already constant in MIR, do not change it.
-        if let Value::Constant { value, disambiguator: _ } = *self.get(index)
+        let value = self.get(index);
+        debug!(?index, ?value);
+        match value {
             // If the constant is not deterministic, adding an additional mention of it in MIR will
             // not give the same value as the former mention.
-            && value.is_deterministic()
-        {
-            return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: value });
+            Value::Constant { value, disambiguator: _ } if value.is_deterministic() => {
+                return Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_: *value });
+            }
+            // ignore nested arrays
+            Value::Aggregate(AggregateTy::Array, _, fields) => {
+                for f in fields {
+                    if let Value::Constant { value: Const::Val(const_, _), .. } = self.get(*f)
+                        && let ConstValue::Indirect { .. } = const_
+                    {
+                        return None;
+                    }
+                }
+            }
+            // ignore promoted arrays
+            Value::Projection(index, ProjectionElem::Deref) => {
+                if let Value::Constant { value: Const::Val(const_, ty), .. } = self.get(*index)
+                    && let ConstValue::Scalar(Scalar::Ptr(..)) = const_
+                    && let ty::Ref(region, ty, _mutability) = ty.kind()
+                    && region.is_erased()
+                    && ty.is_array()
+                {
+                    return None;
+                }
+            }
+            _ => {}
         }
 
         let op = self.evaluated[index].as_ref()?;
-        if op.layout.is_unsized() {
-            // Do not attempt to propagate unsized locals.
-            return None;
-        }
 
         let value = op_to_prop_const(&mut self.ecx, op)?;
 
@@ -1326,6 +1380,10 @@ impl<'tcx> VnState<'_, 'tcx> {
         assert!(!value.may_have_provenance(self.tcx, op.layout.size));
 
         let const_ = Const::Val(value, op.layout.ty);
+        // cache the propagated const
+        if let Some(new_index) = self.insert_constant(const_) {
+            self.values.swap_indices(index.as_usize(), new_index.as_usize());
+        }
         Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_ })
     }
 
@@ -1353,6 +1411,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
         self.simplify_operand(operand, location);
     }
 
+    #[instrument(level = "trace", skip(self, stmt))]
     fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, location: Location) {
         if let StatementKind::Assign(box (ref mut lhs, ref mut rvalue)) = stmt.kind {
             self.simplify_place_projection(lhs, location);
@@ -1366,8 +1425,10 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
                 .as_local()
                 .and_then(|local| self.locals[local])
                 .or_else(|| self.simplify_rvalue(rvalue, location));
+            debug!(?value);
             let Some(value) = value else { return };
 
+            debug!(before_rvalue = ?rvalue);
             if let Some(const_) = self.try_as_constant(value) {
                 *rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
             } else if let Some(local) = self.try_as_local(value, location)
@@ -1376,6 +1437,7 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
                 *rvalue = Rvalue::Use(Operand::Copy(local.into()));
                 self.reused_locals.insert(local);
             }
+            debug!(after_rvalue = ?rvalue);
 
             return;
         }

tests/codegen/issue-73825-gvn-const-local-array.rs

@@ -0,0 +1,25 @@
+// issue: <https://github.com/rust-lang/rust/issues/73825>
+//@ compile-flags: -C opt-level=1
+#![crate_type = "lib"]
+
+// CHECK-LABEL: @foo
+// CHECK-NEXT: {{.*}}:
+// CHECK-NEXT: and
+// CHECK-NEXT: getelementptr inbounds
+// CHECK-NEXT: load i32
+// CHECK-NEXT: ret i32
+#[no_mangle]
+#[rustfmt::skip]
+pub fn foo(x: usize) -> i32 {
+    let base: [i32; 64] = [
+        67, 754, 860, 559, 368, 870, 548, 972,
+        141, 731, 351, 664, 32, 4, 996, 741,
+        203, 292, 237, 480, 151, 940, 777, 540,
+        143, 587, 747, 65, 152, 517, 882, 880,
+        712, 595, 370, 901, 237, 53, 789, 785,
+        912, 650, 896, 367, 316, 392, 62, 473,
+        675, 691, 281, 192, 445, 970, 225, 425,
+        628, 324, 322, 206, 912, 867, 462, 92
+    ];
+    base[x % 64]
+}