From 3aedb85a278b86b254a7c00de0be03864e678d72 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 11 Sep 2023 17:00:51 +0200 Subject: [PATCH 1/4] a bit of cleanup in valtree_to_const_value --- .../src/const_eval/valtrees.rs | 88 ++++++++----------- 1 file changed, 37 insertions(+), 51 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index 572d7f9ac2fec..793a1d21c30ce 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -189,12 +189,11 @@ fn reconstruct_place_meta<'tcx>( } #[instrument(skip(ecx), level = "debug", ret)] -fn create_pointee_place<'tcx>( +fn create_valtree_place<'tcx>( ecx: &mut CompileTimeEvalContext<'tcx, 'tcx>, - ty: Ty<'tcx>, + layout: TyAndLayout<'tcx>, valtree: ty::ValTree<'tcx>, ) -> MPlaceTy<'tcx> { - let layout = ecx.layout_of(ty).unwrap(); let meta = reconstruct_place_meta(layout, valtree, ecx.tcx.tcx); ecx.allocate_dyn(layout, MemoryKind::Stack, meta).unwrap() } @@ -216,11 +215,6 @@ pub fn valtree_to_const_value<'tcx>( // FIXME Does this need an example? let (param_env, ty) = param_env_ty.into_parts(); - let mut ecx: crate::interpret::InterpCx< - '_, - '_, - crate::const_eval::CompileTimeInterpreter<'_, '_>, - > = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No); match ty.kind() { ty::FnDef(..) => { @@ -233,33 +227,29 @@ pub fn valtree_to_const_value<'tcx>( "ValTrees for Bool, Int, Uint, Float or Char should have the form ValTree::Leaf" ), }, - ty::Ref(_, _, _) | ty::Tuple(_) | ty::Array(_, _) | ty::Adt(..) => { - let place = match ty.kind() { - ty::Ref(_, inner_ty, _) => { - // Need to create a place for the pointee (the reference itself will be an immediate) - create_pointee_place(&mut ecx, *inner_ty, valtree) - } - _ => { - // Need to create a place for this valtree. - create_pointee_place(&mut ecx, ty, valtree) - } - }; - debug!(?place); + ty::Ref(_, inner_ty, _) => { + let mut ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No); + let imm = valtree_to_ref(&mut ecx, valtree, *inner_ty); + let imm = ImmTy::from_immediate(imm, tcx.layout_of(param_env_ty).unwrap()); + op_to_const(&ecx, &imm.into()) + } + ty::Tuple(_) | ty::Array(_, _) | ty::Adt(..) => { + let layout = tcx.layout_of(param_env_ty).unwrap(); + if layout.is_zst() { + // Fast path to avoid some allocations. + return ConstValue::ZeroSized; + } + + let mut ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No); + + // Need to create a place for this valtree. + let place = create_valtree_place(&mut ecx, layout, valtree); valtree_into_mplace(&mut ecx, &place, valtree); dump_place(&ecx, &place); intern_const_alloc_recursive(&mut ecx, InternKind::Constant, &place).unwrap(); - match ty.kind() { - ty::Ref(_, _, _) => { - let ref_place = place.to_ref(&tcx); - let imm = - ImmTy::from_immediate(ref_place, tcx.layout_of(param_env_ty).unwrap()); - - op_to_const(&ecx, &imm.into()) - } - _ => op_to_const(&ecx, &place.into()), - } + op_to_const(&ecx, &place.into()) } ty::Never | ty::Error(_) @@ -283,6 +273,22 @@ pub fn valtree_to_const_value<'tcx>( } } +/// Put a valtree into memory and return a reference to that. +fn valtree_to_ref<'tcx>( + ecx: &mut CompileTimeEvalContext<'tcx, 'tcx>, + valtree: ty::ValTree<'tcx>, + pointee_ty: Ty<'tcx>, +) -> Immediate { + let pointee_place = create_valtree_place(ecx, ecx.layout_of(pointee_ty).unwrap(), valtree); + debug!(?pointee_place); + + valtree_into_mplace(ecx, &pointee_place, valtree); + dump_place(ecx, &pointee_place); + intern_const_alloc_recursive(ecx, InternKind::Constant, &pointee_place).unwrap(); + + pointee_place.to_ref(&ecx.tcx) +} + #[instrument(skip(ecx), level = "debug")] fn valtree_into_mplace<'tcx>( ecx: &mut CompileTimeEvalContext<'tcx, 'tcx>, @@ -292,7 +298,6 @@ fn valtree_into_mplace<'tcx>( // This will match on valtree and write the value(s) corresponding to the ValTree // inside the place recursively. - let tcx = ecx.tcx.tcx; let ty = place.layout.ty; match ty.kind() { @@ -305,27 +310,8 @@ fn valtree_into_mplace<'tcx>( ecx.write_immediate(Immediate::Scalar(scalar_int.into()), place).unwrap(); } ty::Ref(_, inner_ty, _) => { - let pointee_place = create_pointee_place(ecx, *inner_ty, valtree); - debug!(?pointee_place); - - valtree_into_mplace(ecx, &pointee_place, valtree); - dump_place(ecx, &pointee_place); - intern_const_alloc_recursive(ecx, InternKind::Constant, &pointee_place).unwrap(); - - let imm = match inner_ty.kind() { - ty::Slice(_) | ty::Str => { - let len = valtree.unwrap_branch().len(); - let len_scalar = Scalar::from_target_usize(len as u64, &tcx); - - Immediate::ScalarPair( - Scalar::from_maybe_pointer(pointee_place.ptr(), &tcx), - len_scalar, - ) - } - _ => pointee_place.to_ref(&tcx), - }; + let imm = valtree_to_ref(ecx, valtree, *inner_ty); debug!(?imm); - ecx.write_immediate(imm, place).unwrap(); } ty::Adt(_, _) | ty::Tuple(_) | ty::Array(_, _) | ty::Str | ty::Slice(_) => { From 06947be1966f088a0dfb496e986bcc09bda0cb31 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Sep 2023 07:40:05 +0200 Subject: [PATCH 2/4] valtree_to_const_value: add fast-path for Scalar tuples/structs --- .../rustc_const_eval/src/const_eval/valtrees.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_const_eval/src/const_eval/valtrees.rs b/compiler/rustc_const_eval/src/const_eval/valtrees.rs index 793a1d21c30ce..1675b824c525c 100644 --- a/compiler/rustc_const_eval/src/const_eval/valtrees.rs +++ b/compiler/rustc_const_eval/src/const_eval/valtrees.rs @@ -7,7 +7,7 @@ use crate::interpret::{ intern_const_alloc_recursive, ConstValue, ImmTy, Immediate, InternKind, MemPlaceMeta, MemoryKind, PlaceTy, Projectable, Scalar, }; -use rustc_middle::ty::layout::{LayoutOf, TyAndLayout}; +use rustc_middle::ty::layout::{LayoutCx, LayoutOf, TyAndLayout}; use rustc_middle::ty::{self, ScalarInt, Ty, TyCtxt}; use rustc_span::source_map::DUMMY_SP; use rustc_target::abi::VariantIdx; @@ -239,6 +239,21 @@ pub fn valtree_to_const_value<'tcx>( // Fast path to avoid some allocations. return ConstValue::ZeroSized; } + if layout.abi.is_scalar() + && (matches!(ty.kind(), ty::Tuple(_)) + || matches!(ty.kind(), ty::Adt(def, _) if def.is_struct())) + { + // A Scalar tuple/struct; we can avoid creating an allocation. + let branches = valtree.unwrap_branch(); + // Find the non-ZST field. (There can be aligned ZST!) + for (i, &inner_valtree) in branches.iter().enumerate() { + let field = layout.field(&LayoutCx { tcx, param_env }, i); + if !field.is_zst() { + return valtree_to_const_value(tcx, param_env.and(field.ty), inner_valtree); + } + } + bug!("could not find non-ZST field during in {layout:#?}"); + } let mut ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No); From 292d5bba86d2d04813be5b887cd86a98876e02ea Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Sep 2023 07:44:49 +0200 Subject: [PATCH 3/4] always evaluate ConstantKind::Ty through valtrees --- compiler/rustc_middle/src/mir/mod.rs | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 3b22ecfbe50c8..1ce7140f361da 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2373,23 +2373,19 @@ impl<'tcx> ConstantKind<'tcx> { param_env: ty::ParamEnv<'tcx>, span: Option, ) -> Result, ErrorHandled> { - let (uneval, param_env) = match self { + match self { ConstantKind::Ty(c) => { - if let ty::ConstKind::Unevaluated(uneval) = c.kind() { - // Avoid the round-trip via valtree, evaluate directly to ConstValue. - let (param_env, uneval) = uneval.prepare_for_eval(tcx, param_env); - (uneval.expand(), param_env) - } else { - // It's already a valtree, or an error. - let val = c.eval(tcx, param_env, span)?; - return Ok(tcx.valtree_to_const_val((self.ty(), val))); - } + // We want to consistently have a "clean" value for type system constants (i.e., no + // data hidden in the padding), so we always go through a valtree here. + let val = c.eval(tcx, param_env, span)?; + Ok(tcx.valtree_to_const_val((self.ty(), val))) } - ConstantKind::Unevaluated(uneval, _) => (uneval, param_env), - ConstantKind::Val(val, _) => return Ok(val), - }; - // FIXME: We might want to have a `try_eval`-like function on `Unevaluated` - tcx.const_eval_resolve(param_env, uneval, span) + ConstantKind::Unevaluated(uneval, _) => { + // FIXME: We might want to have a `try_eval`-like function on `Unevaluated` + tcx.const_eval_resolve(param_env, uneval, span) + } + ConstantKind::Val(val, _) => Ok(val), + } } /// Normalizes the constant to a value or an error if possible. @@ -2605,10 +2601,10 @@ impl<'tcx> ConstantKind<'tcx> { pub fn from_ty_const(c: ty::Const<'tcx>, tcx: TyCtxt<'tcx>) -> Self { match c.kind() { ty::ConstKind::Value(valtree) => { + // Make sure that if `c` is normalized, then the return value is normalized. let const_val = tcx.valtree_to_const_val((c.ty(), valtree)); Self::Val(const_val, c.ty()) } - ty::ConstKind::Unevaluated(uv) => Self::Unevaluated(uv.expand(), c.ty()), _ => Self::Ty(c), } } From 19fb2c7ccd67c8599b5327efd0aab383f77d99a3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 14 Sep 2023 07:53:38 +0200 Subject: [PATCH 4/4] found another place where we can eval() a const, and go through valtrees --- compiler/rustc_middle/src/mir/mod.rs | 6 ++- compiler/rustc_middle/src/ty/consts/kind.rs | 5 --- compiler/rustc_monomorphize/src/collector.rs | 42 +++++--------------- 3 files changed, 14 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 1ce7140f361da..3ebb1038119dc 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -2297,7 +2297,11 @@ pub struct Constant<'tcx> { #[derive(Clone, Copy, PartialEq, Eq, TyEncodable, TyDecodable, Hash, HashStable, Debug)] #[derive(Lift, TypeFoldable, TypeVisitable)] pub enum ConstantKind<'tcx> { - /// This constant came from the type system + /// This constant came from the type system. + /// + /// Any way of turning `ty::Const` into `ConstValue` should go through `valtree_to_const_val`; + /// this ensures that we consistently produce "clean" values without data in the padding or + /// anything like that. Ty(ty::Const<'tcx>), /// An unevaluated mir constant which is not part of the type system. diff --git a/compiler/rustc_middle/src/ty/consts/kind.rs b/compiler/rustc_middle/src/ty/consts/kind.rs index a0a7331a37e6d..e25402fe0c216 100644 --- a/compiler/rustc_middle/src/ty/consts/kind.rs +++ b/compiler/rustc_middle/src/ty/consts/kind.rs @@ -22,11 +22,6 @@ impl rustc_errors::IntoDiagnosticArg for UnevaluatedConst<'_> { } impl<'tcx> UnevaluatedConst<'tcx> { - #[inline] - pub fn expand(self) -> mir::UnevaluatedConst<'tcx> { - mir::UnevaluatedConst { def: self.def, args: self.args, promoted: None } - } - /// FIXME(RalfJung): I cannot explain what this does or why it makes sense, but not doing this /// hurts performance. #[inline] diff --git a/compiler/rustc_monomorphize/src/collector.rs b/compiler/rustc_monomorphize/src/collector.rs index 8cbb68fc8c1b3..de677aaa5f584 100644 --- a/compiler/rustc_monomorphize/src/collector.rs +++ b/compiler/rustc_monomorphize/src/collector.rs @@ -749,39 +749,15 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirUsedCollector<'a, 'tcx> { #[instrument(skip(self), level = "debug")] fn visit_constant(&mut self, constant: &mir::Constant<'tcx>, location: Location) { let literal = self.monomorphize(constant.literal); - let val = match literal { - mir::ConstantKind::Val(val, _) => val, - mir::ConstantKind::Ty(ct) => match ct.kind() { - ty::ConstKind::Value(val) => self.tcx.valtree_to_const_val((ct.ty(), val)), - ty::ConstKind::Unevaluated(ct) => { - debug!(?ct); - let param_env = ty::ParamEnv::reveal_all(); - match self.tcx.const_eval_resolve(param_env, ct.expand(), None) { - // The `monomorphize` call should have evaluated that constant already. - Ok(val) => val, - Err(ErrorHandled::Reported(_)) => return, - Err(ErrorHandled::TooGeneric) => span_bug!( - self.body.source_info(location).span, - "collection encountered polymorphic constant: {:?}", - literal - ), - } - } - _ => return, - }, - mir::ConstantKind::Unevaluated(uv, _) => { - let param_env = ty::ParamEnv::reveal_all(); - match self.tcx.const_eval_resolve(param_env, uv, None) { - // The `monomorphize` call should have evaluated that constant already. - Ok(val) => val, - Err(ErrorHandled::Reported(_)) => return, - Err(ErrorHandled::TooGeneric) => span_bug!( - self.body.source_info(location).span, - "collection encountered polymorphic constant: {:?}", - literal - ), - } - } + let param_env = ty::ParamEnv::reveal_all(); + let val = match literal.eval(self.tcx, param_env, None) { + Ok(v) => v, + Err(ErrorHandled::Reported(_)) => return, + Err(ErrorHandled::TooGeneric) => span_bug!( + self.body.source_info(location).span, + "collection encountered polymorphic constant: {:?}", + literal + ), }; collect_const_value(self.tcx, val, self.output); MirVisitor::visit_ty(self, literal.ty(), TyContext::Location(location));