From e6b5e00ea215c64731360b45d5867ee0332a93c0 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sun, 10 Mar 2013 22:58:44 -0700 Subject: [PATCH 1/2] Simplify struct representation. Out goes the extra layer of struct wrapping; the destructedness flag is added to the end of the struct. This means that, if the struct previously had alignment padding at the end, the flag will live there instead of increasing the struct size. --- src/librustc/middle/trans/adt.rs | 79 +++++++++++++------------------- 1 file changed, 31 insertions(+), 48 deletions(-) diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index e39de62cd297c..d816aefeb2fd8 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -81,8 +81,14 @@ pub enum Repr { Unit(int), /// C-like enums; basically an int. CEnum(int, int), // discriminant range - /// Single-case variants, and structs/tuples/records. - Univariant(Struct, Destructor), + /** + * Single-case variants, and structs/tuples/records. + * + * Structs with destructors need a dynamic destroyedness flag to + * avoid running the destructor too many times; this is included + * in the `Struct` if present. + */ + Univariant(Struct, bool), /** * General-case enums: discriminant as int, followed by fields. * The fields start immediately after the discriminant, meaning @@ -92,18 +98,6 @@ pub enum Repr { General(~[Struct]) } -/** - * Structs without destructors have historically had an extra layer of - * LLVM-struct to make accessing them work the same as structs with - * destructors. This could probably be flattened to a boolean now - * that this module exists. - */ -enum Destructor { - StructWithDtor, - StructWithoutDtor, - NonStruct -} - /// For structs, and struct-like parts of anything fancier. struct Struct { size: u64, @@ -129,14 +123,17 @@ pub fn represent_type(cx: @CrateContext, t: ty::t) -> @Repr { } let repr = @match ty::get(t).sty { ty::ty_tup(ref elems) => { - Univariant(mk_struct(cx, *elems), NonStruct) + Univariant(mk_struct(cx, *elems), false) } ty::ty_struct(def_id, ref substs) => { let fields = ty::lookup_struct_fields(cx.tcx, def_id); - let dt = ty::ty_dtor(cx.tcx, def_id).is_present(); - Univariant(mk_struct(cx, fields.map(|field| { + let ftys = do fields.map |field| { ty::lookup_field_type(cx.tcx, def_id, field.id, substs) - })), if dt { StructWithDtor } else { StructWithoutDtor }) + }; + let dtor = ty::ty_dtor(cx.tcx, def_id).is_present(); + let ftys = + if dtor { ftys + [ty::mk_bool(cx.tcx)] } else { ftys }; + Univariant(mk_struct(cx, ftys), dtor) } ty::ty_enum(def_id, ref substs) => { struct Case { discr: int, tys: ~[ty::t] }; @@ -156,7 +153,7 @@ pub fn represent_type(cx: @CrateContext, t: ty::t) -> @Repr { } else if cases.len() == 1 { // Equivalent to a struct/tuple/newtype. fail_unless!(cases[0].discr == 0); - Univariant(mk_struct(cx, cases[0].tys), NonStruct) + Univariant(mk_struct(cx, cases[0].tys), false) } else if cases.all(|c| c.tys.len() == 0) { // All bodies empty -> intlike let discrs = cases.map(|c| c.discr); @@ -206,16 +203,11 @@ fn generic_fields_of(cx: @CrateContext, r: &Repr, sizing: bool) match *r { Unit(*) => ~[], CEnum(*) => ~[T_enum_discrim(cx)], - Univariant(ref st, dt) => { - let f = if sizing { + Univariant(ref st, _dtor) => { + if sizing { st.fields.map(|&ty| type_of::sizing_type_of(cx, ty)) } else { st.fields.map(|&ty| type_of::type_of(cx, ty)) - }; - match dt { - NonStruct => f, - StructWithoutDtor => ~[T_struct(f)], - StructWithDtor => ~[T_struct(f), T_i8()] } } General(ref sts) => { @@ -308,9 +300,10 @@ pub fn trans_start_init(bcx: block, r: &Repr, val: ValueRef, discr: int) { fail_unless!(min <= discr && discr <= max); Store(bcx, C_int(bcx.ccx(), discr), GEPi(bcx, val, [0, 0])) } - Univariant(_, StructWithDtor) => { + Univariant(ref st, true) => { fail_unless!(discr == 0); - Store(bcx, C_u8(1), GEPi(bcx, val, [0, 1])) + Store(bcx, C_bool(true), + GEPi(bcx, val, [0, st.fields.len() - 1])) } Univariant(*) => { fail_unless!(discr == 0); @@ -328,7 +321,10 @@ pub fn trans_start_init(bcx: block, r: &Repr, val: ValueRef, discr: int) { pub fn num_args(r: &Repr, discr: int) -> uint { match *r { Unit(*) | CEnum(*) => 0, - Univariant(ref st, _) => { fail_unless!(discr == 0); st.fields.len() } + Univariant(ref st, dtor) => { + fail_unless!(discr == 0); + st.fields.len() - (if dtor { 1 } else { 0 }) + } General(ref cases) => cases[discr as uint].fields.len() } } @@ -343,12 +339,8 @@ pub fn trans_field_ptr(bcx: block, r: &Repr, val: ValueRef, discr: int, Unit(*) | CEnum(*) => { bcx.ccx().sess.bug(~"element access in C-like enum") } - Univariant(ref st, dt) => { + Univariant(ref st, _dtor) => { fail_unless!(discr == 0); - let val = match dt { - NonStruct => val, - StructWithDtor | StructWithoutDtor => GEPi(bcx, val, [0, 0]) - }; struct_field_ptr(bcx, st, val, ix, false) } General(ref cases) => { @@ -376,7 +368,7 @@ fn struct_field_ptr(bcx: block, st: &Struct, val: ValueRef, ix: uint, /// Access the struct drop flag, if present. pub fn trans_drop_flag_ptr(bcx: block, r: &Repr, val: ValueRef) -> ValueRef { match *r { - Univariant(_, StructWithDtor) => GEPi(bcx, val, [0, 1]), + Univariant(ref st, true) => GEPi(bcx, val, [0, st.fields.len() - 1]), _ => bcx.ccx().sess.bug(~"tried to get drop flag of non-droppable \ type") } @@ -415,15 +407,9 @@ pub fn trans_const(ccx: @CrateContext, r: &Repr, discr: int, fail_unless!(min <= discr && discr <= max); C_int(ccx, discr) } - Univariant(ref st, dt) => { + Univariant(ref st, _dro) => { fail_unless!(discr == 0); - let s = C_struct(build_const_struct(ccx, st, vals)); - match dt { - NonStruct => s, - // The actual destructor flag doesn't need to be present. - // But add an extra struct layer for compatibility. - StructWithDtor | StructWithoutDtor => C_struct(~[s]) - } + C_struct(build_const_struct(ccx, st, vals)) } General(ref cases) => { let case = &cases[discr as uint]; @@ -508,9 +494,7 @@ pub fn const_get_field(ccx: @CrateContext, r: &Repr, val: ValueRef, match *r { Unit(*) | CEnum(*) => ccx.sess.bug(~"element access in C-like enum \ const"), - Univariant(_, NonStruct) => const_struct_field(ccx, val, ix), - Univariant(*) => const_struct_field(ccx, const_get_elt(ccx, val, - [0]), ix), + Univariant(*) => const_struct_field(ccx, val, ix), General(*) => const_struct_field(ccx, const_get_elt(ccx, val, [1, 0]), ix) } @@ -542,8 +526,7 @@ fn const_struct_field(ccx: @CrateContext, val: ValueRef, ix: uint) /// Is it safe to bitcast a value to the one field of its one variant? pub fn is_newtypeish(r: &Repr) -> bool { match *r { - Univariant(ref st, StructWithoutDtor) - | Univariant(ref st, NonStruct) => st.fields.len() == 1, + Univariant(ref st, false) => st.fields.len() == 1, _ => false } } From 9eaa608b041165df6ddfc064561f31315c5cb21b Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Sun, 10 Mar 2013 23:16:08 -0700 Subject: [PATCH 2/2] Get rid of the `Unit` enum representation. The only thing we really lose is that C-like enums with one variant and a non-zero discriminant now take up space, but I do not think this is a common usage. As previously noted, that was mostly there for transitional compatibility with the pre-adt.rs codebase. --- src/librustc/middle/trans/adt.rs | 40 ++++++++------------------------ 1 file changed, 10 insertions(+), 30 deletions(-) diff --git a/src/librustc/middle/trans/adt.rs b/src/librustc/middle/trans/adt.rs index d816aefeb2fd8..66a32335020ac 100644 --- a/src/librustc/middle/trans/adt.rs +++ b/src/librustc/middle/trans/adt.rs @@ -71,14 +71,6 @@ use util::ppaux::ty_to_str; /// Representations. pub enum Repr { - /** - * `Unit` exists only so that an enum with a single C-like variant - * can occupy no space, for ABI compatibility with rustc from - * before (and during) the creation of this module. It may not be - * worth keeping around; `CEnum` and `Univariant` cover it - * overwise. - */ - Unit(int), /// C-like enums; basically an int. CEnum(int, int), // discriminant range /** @@ -146,18 +138,15 @@ pub fn represent_type(cx: @CrateContext, t: ty::t) -> @Repr { }; if cases.len() == 0 { // Uninhabitable; represent as unit - Unit(0) - } else if cases.len() == 1 && cases[0].tys.len() == 0 { - // `()`-like; see comment on definition of `Unit`. - Unit(cases[0].discr) - } else if cases.len() == 1 { - // Equivalent to a struct/tuple/newtype. - fail_unless!(cases[0].discr == 0); - Univariant(mk_struct(cx, cases[0].tys), false) + Univariant(mk_struct(cx, ~[]), false) } else if cases.all(|c| c.tys.len() == 0) { // All bodies empty -> intlike let discrs = cases.map(|c| c.discr); CEnum(discrs.min(), discrs.max()) + } else if cases.len() == 1 { + // Equivalent to a struct/tuple/newtype. + fail_unless!(cases[0].discr == 0); + Univariant(mk_struct(cx, cases[0].tys), false) } else { // The general case. Since there's at least one // non-empty body, explicit discriminants should have @@ -201,7 +190,6 @@ pub fn sizing_fields_of(cx: @CrateContext, r: &Repr) -> ~[TypeRef] { fn generic_fields_of(cx: @CrateContext, r: &Repr, sizing: bool) -> ~[TypeRef] { match *r { - Unit(*) => ~[], CEnum(*) => ~[T_enum_discrim(cx)], Univariant(ref st, _dtor) => { if sizing { @@ -229,7 +217,7 @@ pub fn trans_switch(bcx: block, r: &Repr, scrutinee: ValueRef) CEnum(*) | General(*) => { (_match::switch, Some(trans_get_discr(bcx, r, scrutinee))) } - Unit(*) | Univariant(*) => { + Univariant(*) => { (_match::single, None) } } @@ -239,7 +227,6 @@ pub fn trans_switch(bcx: block, r: &Repr, scrutinee: ValueRef) pub fn trans_get_discr(bcx: block, r: &Repr, scrutinee: ValueRef) -> ValueRef { match *r { - Unit(the_disc) => C_int(bcx.ccx(), the_disc), CEnum(min, max) => load_discr(bcx, scrutinee, min, max), Univariant(*) => C_int(bcx.ccx(), 0), General(ref cases) => load_discr(bcx, scrutinee, 0, @@ -277,7 +264,7 @@ pub fn trans_case(bcx: block, r: &Repr, discr: int) -> _match::opt_result { CEnum(*) => { _match::single_result(rslt(bcx, C_int(bcx.ccx(), discr))) } - Unit(*) | Univariant(*)=> { + Univariant(*)=> { bcx.ccx().sess.bug(~"no cases for univariants or structs") } General(*) => { @@ -293,9 +280,6 @@ pub fn trans_case(bcx: block, r: &Repr, discr: int) -> _match::opt_result { */ pub fn trans_start_init(bcx: block, r: &Repr, val: ValueRef, discr: int) { match *r { - Unit(the_discr) => { - fail_unless!(discr == the_discr); - } CEnum(min, max) => { fail_unless!(min <= discr && discr <= max); Store(bcx, C_int(bcx.ccx(), discr), GEPi(bcx, val, [0, 0])) @@ -320,7 +304,7 @@ pub fn trans_start_init(bcx: block, r: &Repr, val: ValueRef, discr: int) { */ pub fn num_args(r: &Repr, discr: int) -> uint { match *r { - Unit(*) | CEnum(*) => 0, + CEnum(*) => 0, Univariant(ref st, dtor) => { fail_unless!(discr == 0); st.fields.len() - (if dtor { 1 } else { 0 }) @@ -336,7 +320,7 @@ pub fn trans_field_ptr(bcx: block, r: &Repr, val: ValueRef, discr: int, // decide to do some kind of cdr-coding-like non-unique repr // someday), it will need to return a possibly-new bcx as well. match *r { - Unit(*) | CEnum(*) => { + CEnum(*) => { bcx.ccx().sess.bug(~"element access in C-like enum") } Univariant(ref st, _dtor) => { @@ -399,9 +383,6 @@ pub fn trans_drop_flag_ptr(bcx: block, r: &Repr, val: ValueRef) -> ValueRef { pub fn trans_const(ccx: @CrateContext, r: &Repr, discr: int, vals: &[ValueRef]) -> ValueRef { match *r { - Unit(*) => { - C_struct(~[]) - } CEnum(min, max) => { fail_unless!(vals.len() == 0); fail_unless!(min <= discr && discr <= max); @@ -475,7 +456,6 @@ fn roundup(x: u64, a: u64) -> u64 { ((x + (a - 1)) / a) * a } pub fn const_get_discrim(ccx: @CrateContext, r: &Repr, val: ValueRef) -> int { match *r { - Unit(discr) => discr, CEnum(*) => const_to_int(val) as int, Univariant(*) => 0, General(*) => const_to_int(const_get_elt(ccx, val, [0])) as int, @@ -492,7 +472,7 @@ pub fn const_get_discrim(ccx: @CrateContext, r: &Repr, val: ValueRef) pub fn const_get_field(ccx: @CrateContext, r: &Repr, val: ValueRef, _discr: int, ix: uint) -> ValueRef { match *r { - Unit(*) | CEnum(*) => ccx.sess.bug(~"element access in C-like enum \ + CEnum(*) => ccx.sess.bug(~"element access in C-like enum \ const"), Univariant(*) => const_struct_field(ccx, val, ix), General(*) => const_struct_field(ccx, const_get_elt(ccx, val,