Skip to content

refactor to remove trans::adt and make rustc::ty::layout authoritative #36151

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

Merged
merged 5 commits into from
Sep 26, 2016
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
4 changes: 4 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,9 @@ pub struct GlobalCtxt<'tcx> {
/// Cache for layouts computed from types.
pub layout_cache: RefCell<FnvHashMap<Ty<'tcx>, &'tcx Layout>>,

//Used to prevent layout from recursing too deeply.
Copy link
Contributor

@arielb1 arielb1 Sep 14, 2016

Choose a reason for hiding this comment

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

nit: don't put random mutable parameters on the tcx. Use a LayoutContext.

Copy link
Member

Choose a reason for hiding this comment

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

You can't, because the recursion doesn't go through a dedicated context. You might be able to put it in TargetDataLayout, I guess, but that's about it.

Copy link
Member

@eddyb eddyb Sep 25, 2016

Choose a reason for hiding this comment

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

Space after //. Also, /// should be used (signifying doc comments).

pub layout_depth: Cell<usize>,

/// Map from function to the `#[derive]` mode that it's defining. Only used
/// by `rustc-macro` crates.
pub derive_macros: RefCell<NodeMap<token::InternedString>>,
Expand Down Expand Up @@ -760,6 +763,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
crate_name: token::intern_and_get_ident(crate_name),
data_layout: data_layout,
layout_cache: RefCell::new(FnvHashMap()),
layout_depth: Cell::new(0),
derive_macros: RefCell::new(NodeMap()),
}, f)
}
Expand Down
87 changes: 71 additions & 16 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,43 @@ pub enum Integer {
}

impl Integer {

Copy link
Member

Choose a reason for hiding this comment

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

Extra newline.

pub fn size(&self) -> Size {
match *self {
I1 => Size::from_bits(1),
I8 => Size::from_bytes(1),
I16 => Size::from_bytes(2),
I32 => Size::from_bytes(4),
I64 => Size::from_bytes(8),
}
}

pub fn align(&self, dl: &TargetDataLayout)-> Align {
match *self {
I1 => dl.i1_align,
I8 => dl.i8_align,
I16 => dl.i16_align,
I32 => dl.i32_align,
I64 => dl.i64_align,
}
}

pub fn to_ty<'a, 'tcx>(&self, tcx: &ty::TyCtxt<'a, 'tcx, 'tcx>,
signed: bool) -> Ty<'tcx> {
Copy link
Member

Choose a reason for hiding this comment

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

signed is aligned 4 spaces to the left of where it should be.

match (*self, signed) {
(I1, false) => tcx.types.u8,
(I8, false) => tcx.types.u8,
(I16, false) => tcx.types.u16,
(I32, false) => tcx.types.u32,
(I64, false) => tcx.types.u64,
(I1, true) => tcx.types.i8,
(I8, true) => tcx.types.i8,
(I16, true) => tcx.types.i16,
(I32, true) => tcx.types.i32,
(I64, true) => tcx.types.i64,
}
}

/// Find the smallest Integer type which can represent the signed value.
pub fn fit_signed(x: i64) -> Integer {
match x {
Expand All @@ -350,6 +387,18 @@ impl Integer {
}
}

//Find the smallest integer with the given alignment.
Copy link
Member

Choose a reason for hiding this comment

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

Space after // and use ///.

pub fn for_abi_align(dl: &TargetDataLayout, align: Align) -> Option<Integer> {
let wanted = align.abi();
for &candidate in &[I8, I16, I32, I64] {
let ty = Int(candidate);
if wanted == ty.align(dl).abi() && wanted == ty.size(dl).bytes() {
return Some(candidate);
}
}
None
}

/// Get the Integer type from an attr::IntType.
pub fn from_attr(dl: &TargetDataLayout, ity: attr::IntType) -> Integer {
match ity {
Expand Down Expand Up @@ -623,6 +672,15 @@ impl<'a, 'gcx, 'tcx> Struct {
}
Ok(None)
}

pub fn offset_of_field(&self, index: usize) -> Size {
assert!(index < self.offset_after_field.len());
if index == 0 {
Size::from_bytes(0)
} else {
self.offset_after_field[index-1]
}
}
}

/// An untagged union.
Expand Down Expand Up @@ -912,7 +970,7 @@ impl<'a, 'gcx, 'tcx> Layout {
Univariant { variant: unit, non_zero: false }
}

// Tuples.
// Tuples and closures.
ty::TyClosure(_, ty::ClosureSubsts { upvar_tys: tys, .. }) |
ty::TyTuple(tys) => {
let mut st = Struct::new(dl, false);
Expand Down Expand Up @@ -975,7 +1033,7 @@ impl<'a, 'gcx, 'tcx> Layout {
if def.variants.len() == 1 {
// Struct, or union, or univariant enum equivalent to a struct.
// (Typechecking will reject discriminant-sizing attrs.)
assert!(!def.is_enum() || hint == attr::ReprAny);

let fields = def.variants[0].fields.iter().map(|field| {
field.ty(tcx, substs).layout(infcx)
});
Expand Down Expand Up @@ -1003,6 +1061,16 @@ impl<'a, 'gcx, 'tcx> Layout {
}
}

if def.variants.len() == 1 && hint == attr::ReprAny{
Copy link
Member

Choose a reason for hiding this comment

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

Missing space before {. Also, this will never be reached, maybe you meant to put the hint == attr::ReprAny in the previous if?

// Equivalent to a struct/tuple/newtype.
let fields = def.variants[0].fields.iter().map(|field| {
field.ty(tcx, substs).layout(infcx)
});
let mut st = Struct::new(dl, false);
st.extend(dl, fields, ty)?;
return success(Univariant { variant: st, non_zero: false });
}

// Cache the substituted and normalized variant field types.
let variants = def.variants.iter().map(|v| {
v.fields.iter().map(|field| field.ty(tcx, substs)).collect::<Vec<_>>()
Expand Down Expand Up @@ -1103,20 +1171,7 @@ impl<'a, 'gcx, 'tcx> Layout {
// won't be so conservative.

// Use the initial field alignment
let wanted = start_align.abi();
let mut ity = min_ity;
for &candidate in &[I16, I32, I64] {
let ty = Int(candidate);
if wanted == ty.align(dl).abi() && wanted == ty.size(dl).bytes() {
ity = candidate;
break;
}
}

// FIXME(eddyb) conservative only to avoid diverging from trans::adt.
if align.abi() != start_align.abi() {
ity = min_ity;
}
let mut ity = Integer::for_abi_align(dl, start_align).unwrap_or(min_ity);

// If the alignment is not larger than the chosen discriminant size,
// don't use the alignment as the final size.
Expand Down
9 changes: 9 additions & 0 deletions src/librustc/ty/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -608,10 +608,19 @@ impl<'a, 'tcx> ty::TyS<'tcx> {
}
}

let rec_limit = tcx.sess.recursion_limit.get();
let depth = tcx.layout_depth.get();
if depth > rec_limit {
tcx.sess.fatal(
&format!("overflow representing the type `{}`", self));
}

tcx.layout_depth.set(depth+1);
let layout = Layout::compute_uncached(self, infcx)?;
if can_cache {
tcx.layout_cache.borrow_mut().insert(self, layout);
}
tcx.layout_depth.set(depth);
Ok(layout)
}

Expand Down
16 changes: 8 additions & 8 deletions src/librustc_trans/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use cabi_s390x;
use cabi_mips;
use cabi_mips64;
use cabi_asmjs;
use machine::{llalign_of_min, llsize_of, llsize_of_real, llsize_of_store};
use machine::{llalign_of_min, llsize_of, llsize_of_alloc};
use type_::Type;
use type_of;

Expand Down Expand Up @@ -102,7 +102,7 @@ impl ArgType {
// Wipe old attributes, likely not valid through indirection.
self.attrs = llvm::Attributes::default();

let llarg_sz = llsize_of_real(ccx, self.ty);
let llarg_sz = llsize_of_alloc(ccx, self.ty);

// For non-immediate arguments the callee gets its own copy of
// the value on the stack, so there are no aliases. It's also
Expand Down Expand Up @@ -200,7 +200,7 @@ impl ArgType {
base::call_memcpy(bcx,
bcx.pointercast(dst, Type::i8p(ccx)),
bcx.pointercast(llscratch, Type::i8p(ccx)),
C_uint(ccx, llsize_of_store(ccx, self.ty)),
C_uint(ccx, llsize_of_alloc(ccx, self.ty)),
cmp::min(llalign_of_min(ccx, self.ty),
llalign_of_min(ccx, ty)) as u32);

Expand Down Expand Up @@ -327,7 +327,7 @@ impl FnType {
if let Layout::CEnum { signed, .. } = *ccx.layout_of(ty) {
arg.signedness = Some(signed);
}
if llsize_of_real(ccx, arg.ty) == 0 {
if llsize_of_alloc(ccx, arg.ty) == 0 {
// For some forsaken reason, x86_64-pc-windows-gnu
// doesn't ignore zero-sized struct arguments.
// The same is true for s390x-unknown-linux-gnu.
Expand Down Expand Up @@ -358,7 +358,7 @@ impl FnType {
ty::TyRef(_, ty::TypeAndMut { ty, .. }) |
ty::TyBox(ty) => {
let llty = type_of::sizing_type_of(ccx, ty);
let llsz = llsize_of_real(ccx, llty);
let llsz = llsize_of_alloc(ccx, llty);
ret.attrs.set_dereferenceable(llsz);
}
_ => {}
Expand Down Expand Up @@ -427,7 +427,7 @@ impl FnType {
} else {
if let Some(inner) = rust_ptr_attrs(ty, &mut arg) {
let llty = type_of::sizing_type_of(ccx, inner);
let llsz = llsize_of_real(ccx, llty);
let llsz = llsize_of_alloc(ccx, llty);
arg.attrs.set_dereferenceable(llsz);
}
args.push(arg);
Expand Down Expand Up @@ -469,8 +469,8 @@ impl FnType {
return;
}

let size = llsize_of_real(ccx, llty);
if size > llsize_of_real(ccx, ccx.int_type()) {
let size = llsize_of_alloc(ccx, llty);
if size > llsize_of_alloc(ccx, ccx.int_type()) {
arg.make_indirect(ccx);
} else if size > 0 {
// We want to pass small aggregates as immediates, but using
Expand Down
Loading