Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

always back ty::Const by an Allocation #58486

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
7c966de
Setup `ty::Const` to allow `ConstValue::Scalar` to be backed by an al…
oli-obk Feb 10, 2019
a5a5ef1
Fiddle the corresponding allocation through all `RawConst` uses
oli-obk Feb 10, 2019
f5fb34c
Make const eval not use the hacky `op_to_const` function
oli-obk Feb 14, 2019
db168bc
Fix equality check for constants
oli-obk Feb 15, 2019
9a63fc8
Promoteds always have a backing allocation
oli-obk Feb 15, 2019
78c36b9
Fixup: statics are always backed by an allocation
oli-obk Feb 15, 2019
785302d
Update comment and make it truer by actually doing what it says
oli-obk Feb 15, 2019
8b0dd6d
Explain the `val` caching in `ty::Const`
oli-obk Feb 15, 2019
6906379
Remove a now-dead function argument
oli-obk Feb 15, 2019
1b93eda
Mark a function private that is unused anywhere else
oli-obk Feb 15, 2019
b3731ee
Fold an if on a match returning a bool directly into the match
oli-obk Feb 15, 2019
b0857a7
Synchronize StableHash with the updated PartialEq impl
oli-obk Feb 15, 2019
1495b6b
Don't create an intermediate place when reading discriminants
oli-obk Feb 15, 2019
92e4471
Note to self
oli-obk Feb 15, 2019
0075868
Insert satisfying `op_to_const` bbqing sound here.
oli-obk Feb 15, 2019
a13cfdb
Make validity checking use `MPlaceTy` instead of `OpTy`
oli-obk Feb 15, 2019
981a79e
Eliminate another use of `lazy_const_to_op`
oli-obk Feb 15, 2019
c07162c
Simplify `raw_const_to_mplace`
oli-obk Feb 15, 2019
defa8d5
Update a comment
oli-obk Feb 15, 2019
b653dc2
Another bad function bites the dust
oli-obk Feb 15, 2019
d3d145e
Update an outdated comment
oli-obk Feb 15, 2019
1f718b0
Keep MPlaceTy internals private
oli-obk Feb 15, 2019
5ef183d
Don't access the stack if not strictly necessary
oli-obk Feb 15, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,15 +300,18 @@ impl_stable_hash_for!(struct ty::FieldDef {
});

impl_stable_hash_for!(
impl<'tcx> for enum mir::interpret::ConstValue<'tcx> [ mir::interpret::ConstValue ] {
impl<'tcx> for enum mir::interpret::ConstValue [ mir::interpret::ConstValue ] {
Scalar(val),
Slice(a, b),
ByRef(id, alloc, offset),
ByRef,
}
);

impl_stable_hash_for!(struct crate::mir::interpret::RawConst<'tcx> {
// FIXME(oli-obk): is ignoring the `alloc_id` for perf reasons ok?
alloc_id,
ty,
alloc,
});

impl_stable_hash_for! {
Expand Down Expand Up @@ -374,6 +377,7 @@ impl_stable_hash_for!(enum ::syntax::ast::Mutability {

impl_stable_hash_for!(struct ty::Const<'tcx> {
ty,
alloc,
val
});

Expand Down
21 changes: 11 additions & 10 deletions src/librustc/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,24 @@ use std::fmt;

use crate::ty::{Ty, layout::{HasDataLayout, Size}};

use super::{EvalResult, Pointer, PointerArithmetic, Allocation, AllocId, sign_extend, truncate};
use super::{EvalResult, Pointer, PointerArithmetic, AllocId, sign_extend, truncate, Allocation};

/// Represents the result of a raw const operation, pre-validation.
#[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Hash)]
pub struct RawConst<'tcx> {
// the value lives here, at offset 0, and that allocation definitely is a `AllocKind::Memory`
// (so you can use `AllocMap::unwrap_memory`).
/// the value lives here, at offset 0, and the allocation that it refers to is the one in the
/// `alloc` field
pub alloc_id: AllocId,
pub ty: Ty<'tcx>,
/// the allocation that would be returned by using
/// `tcx.alloc_map.lock().unwrap_memory(self.alloc_id)`
pub alloc: &'tcx Allocation,
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
}

/// Represents a constant value in Rust. `Scalar` and `ScalarPair` are optimizations that
/// match the `LocalState` optimizations for easy conversions between `Value` and `ConstValue`.
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, RustcEncodable, RustcDecodable, Hash)]
pub enum ConstValue<'tcx> {
pub enum ConstValue {
/// Used only for types with `layout::abi::Scalar` ABI and ZSTs.
///
/// Not using the enum `Value` to encode that this must not be `Undef`.
Expand All @@ -31,19 +34,17 @@ pub enum ConstValue<'tcx> {
/// it.
Slice(Scalar, u64),

/// An allocation together with an offset into the allocation.
/// Invariant: the `AllocId` matches the allocation.
ByRef(AllocId, &'tcx Allocation, Size),
ByRef,
}

#[cfg(target_arch = "x86_64")]
static_assert!(CONST_SIZE: ::std::mem::size_of::<ConstValue<'static>>() == 40);
static_assert!(CONST_SIZE: ::std::mem::size_of::<ConstValue>() == 40);

impl<'tcx> ConstValue<'tcx> {
impl ConstValue {
#[inline]
pub fn try_to_scalar(&self) -> Option<Scalar> {
match *self {
ConstValue::ByRef(..) |
ConstValue::ByRef |
ConstValue::Slice(..) => None,
ConstValue::Scalar(val) => Some(val),
}
Expand Down
1 change: 1 addition & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1665,6 +1665,7 @@ impl<'tcx> TerminatorKind<'tcx> {
}.into(),
),
ty: switch_ty,
alloc: None,
};
fmt_const_val(&mut s, c).unwrap();
s.into()
Expand Down
46 changes: 16 additions & 30 deletions src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
//! `BraceStructLiftImpl!`) to help with the tedium.

use crate::mir::ProjectionKind;
use crate::mir::interpret::ConstValue;
use crate::ty::{self, Lift, Ty, TyCtxt};
use crate::ty::fold::{TypeFoldable, TypeFolder, TypeVisitor};
use rustc_data_structures::indexed_vec::{IndexVec, Idx};
Expand Down Expand Up @@ -486,23 +485,20 @@ BraceStructLiftImpl! {
}
}

BraceStructLiftImpl! {
impl<'a, 'tcx> Lift<'tcx> for ty::Const<'a> {
type Lifted = ty::Const<'tcx>;
val, ty
}
}

impl<'a, 'tcx> Lift<'tcx> for ConstValue<'a> {
type Lifted = ConstValue<'tcx>;
impl<'a, 'tcx> Lift<'tcx> for ty::Const<'a> {
type Lifted = ty::Const<'tcx>;
fn lift_to_tcx<'b, 'gcx>(&self, tcx: TyCtxt<'b, 'gcx, 'tcx>) -> Option<Self::Lifted> {
match *self {
ConstValue::Scalar(x) => Some(ConstValue::Scalar(x)),
ConstValue::Slice(x, y) => Some(ConstValue::Slice(x, y)),
ConstValue::ByRef(x, alloc, z) => Some(ConstValue::ByRef(
x, alloc.lift_to_tcx(tcx)?, z,
)),
}
let ty = self.ty.lift_to_tcx(tcx)?;
let alloc = match self.alloc {
// can't use `and_then` or `map`, because the inner `lift_to_tcx` needs to early return
Some((a, p)) => Some((a.lift_to_tcx(tcx)?, p)),
None => None,
};
Some(ty::Const {
val: self.val,
ty,
alloc,
})
}
}

Expand Down Expand Up @@ -1064,24 +1060,14 @@ impl<'tcx> TypeFoldable<'tcx> for &'tcx ty::LazyConst<'tcx> {
impl<'tcx> TypeFoldable<'tcx> for ty::Const<'tcx> {
fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, folder: &mut F) -> Self {
let ty = self.ty.fold_with(folder);
let val = self.val.fold_with(folder);
ty::Const {
ty,
val
val: self.val,
alloc: self.alloc,
}
}

fn super_visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> bool {
self.ty.visit_with(visitor) || self.val.visit_with(visitor)
}
}

impl<'tcx> TypeFoldable<'tcx> for ConstValue<'tcx> {
fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, _folder: &mut F) -> Self {
*self
}

fn super_visit_with<V: TypeVisitor<'tcx>>(&self, _visitor: &mut V) -> bool {
false
self.ty.visit_with(visitor)
}
}
63 changes: 58 additions & 5 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ use crate::ty::subst::{Substs, Subst, Kind, UnpackedKind};
use crate::ty::{self, AdtDef, TypeFlags, Ty, TyCtxt, TypeFoldable};
use crate::ty::{List, TyS, ParamEnvAnd, ParamEnv};
use crate::util::captures::Captures;
use crate::mir::interpret::{Scalar, Pointer};
use crate::mir::interpret::{Scalar, Pointer, Allocation};

use std::hash::{Hash, Hasher};
use smallvec::SmallVec;
use std::iter;
use std::cmp::Ordering;
Expand Down Expand Up @@ -2065,7 +2066,7 @@ pub enum LazyConst<'tcx> {
}

#[cfg(target_arch = "x86_64")]
static_assert!(LAZY_CONST_SIZE: ::std::mem::size_of::<LazyConst<'static>>() == 56);
static_assert!(LAZY_CONST_SIZE: ::std::mem::size_of::<LazyConst<'static>>() == 80);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this isn't a problem? Let's do a perf run.


impl<'tcx> LazyConst<'tcx> {
pub fn map_evaluated<R>(self, f: impl FnOnce(Const<'tcx>) -> Option<R>) -> Option<R> {
Expand All @@ -2086,15 +2087,66 @@ impl<'tcx> LazyConst<'tcx> {
}

/// Typed constant value.
#[derive(Copy, Clone, Debug, Hash, RustcEncodable, RustcDecodable, Eq, PartialEq, Ord, PartialOrd)]
#[derive(Copy, Clone, Debug, RustcEncodable, RustcDecodable, Eq, Ord, PartialOrd)]
pub struct Const<'tcx> {
pub ty: Ty<'tcx>,

pub val: ConstValue<'tcx>,
/// This field is an optimization for caching commonly needed values of constants like `usize`
/// (or other integers for enum discriminants) and slices (e.g. from `b"foo"` and `"foo"`
/// literals)
pub val: ConstValue,

/// The actual backing storage of the constant and a pointer which can be resolved back to the
/// `allocation` field
///
/// Can be `None` for trivial constants created from literals or directly. Is always `Some` for
/// aggregate constants or any named constant that you can actually end up taking a reference
/// to. This will get unwrapped in situations where we do know that it's a referencable
pub alloc: Option<(&'tcx Allocation, Pointer)>,
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
}

impl<'tcx> PartialEq for Const<'tcx> {
fn eq(&self, other: &Self) -> bool {

self.ty == other.ty && match (self.val, other.val) {
(ConstValue::ByRef, ConstValue::ByRef) => {
let (a, pa) = self.alloc.unwrap();
let (b, pb) = other.alloc.unwrap();
// only use the alloc ids to not have to compare the full allocations
// the ids may differ if the allocation is the same
(pa.offset == pb.offset) && (pa.alloc_id == pb.alloc_id || a == b)
},
// ignore the actual allocation, just compare the values
(ConstValue::Scalar(a), ConstValue::Scalar(b)) => a == b,
(ConstValue::Slice(a, an), ConstValue::Slice(b, bn)) => an == bn && a == b,
// if the values don't match, the consts can't be equal and the type equality should
// have already failed, because we make the decision for non-byref solely based on the
// type
_ => bug!("same type but different value kind in constant: {:#?} {:#?}", self, other),
}
}
}

impl<'tcx> Hash for Const<'tcx> {
fn hash<H: Hasher>(&self, hasher: &mut H) {
let Const { ty, val, alloc } = self;
ty.hash(hasher);
val.hash(hasher);
if let ConstValue::ByRef = val {
let (alloc, ptr) = alloc.unwrap();
// type check for future changes
let alloc: &'tcx Allocation = alloc;
alloc.hash(hasher);
ptr.offset.hash(hasher);
// do not hash the alloc id in the pointer. It does not add anything new to the hash.
// If the hash of the alloc id is the same, then the hash of the allocation would also
// be the same.
}
}
}

#[cfg(target_arch = "x86_64")]
static_assert!(CONST_SIZE: ::std::mem::size_of::<Const<'static>>() == 48);
static_assert!(CONST_SIZE: ::std::mem::size_of::<Const<'static>>() == 72);

impl<'tcx> Const<'tcx> {
#[inline]
Expand All @@ -2105,6 +2157,7 @@ impl<'tcx> Const<'tcx> {
Self {
val: ConstValue::Scalar(val),
ty,
alloc: None,
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/librustc_codegen_llvm/consts.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use libc::c_uint;
use llvm::{self, SetUnnamedAddr, True};
use rustc::hir::def_id::DefId;
use rustc::mir::interpret::{ConstValue, Allocation, read_target_uint,
use rustc::mir::interpret::{Allocation, read_target_uint,
Pointer, ErrorHandled, GlobalId};
use rustc::hir::Node;
use debuginfo;
Expand Down Expand Up @@ -69,11 +69,11 @@ pub fn codegen_static_initializer(
};
let param_env = ty::ParamEnv::reveal_all();
let static_ = cx.tcx.const_eval(param_env.and(cid))?;

let alloc = match static_.val {
ConstValue::ByRef(_, alloc, n) if n.bytes() == 0 => alloc,
let (alloc, ptr) = match static_.alloc {
Some(alloc) => alloc,
_ => bug!("static const eval returned {:#?}", static_),
};
assert_eq!(ptr.offset.bytes(), 0);
Ok((const_alloc_to_llvm(cx, alloc), alloc))
}

Expand Down
5 changes: 3 additions & 2 deletions src/librustc_codegen_ssa/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,9 @@ impl<'a, 'tcx: 'a, V: CodegenObject> OperandRef<'tcx, V> {
let b_llval = bx.cx().const_usize(b);
OperandValue::Pair(a_llval, b_llval)
},
ConstValue::ByRef(_, alloc, offset) => {
return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, offset)));
ConstValue::ByRef => {
let (alloc, ptr) = val.alloc.unwrap();
return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, ptr.offset)));
},
};

Expand Down
6 changes: 2 additions & 4 deletions src/librustc_codegen_ssa/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,8 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
};
let layout = cx.layout_of(self.monomorphize(&ty));
match bx.tcx().const_eval(param_env.and(cid)) {
Ok(val) => match val.val {
mir::interpret::ConstValue::ByRef(_, alloc, offset) => {
bx.cx().from_const_alloc(layout, alloc, offset)
}
Ok(val) => match val.alloc {
Some((alloc, ptr)) => bx.cx().from_const_alloc(layout, alloc, ptr.offset),
_ => bug!("promoteds should have an allocation: {:?}", val),
},
Err(_) => {
Expand Down
Loading