Skip to content

Issue #5977 - Generalizing RawNullablePointer to RawForbiddenValue #31215

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Issue #5977 - Generalizing RawNullablePointer to RawForbiddenValue #31215

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
71817da
Issue #5977 - Generalizing RawNullablePointer to RawForbiddenValue
Jan 26, 2016
f16bfd7
Issue #5977 - RawNullablePointer can now optimize Option<char> and Op…
Jan 27, 2016
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
2 changes: 1 addition & 1 deletion src/liblibc
Open in desktop
Submodule liblibc updated from 30f70b to 91ff43
115 changes: 71 additions & 44 deletions src/librustc_trans/trans/adt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,25 +349,28 @@ fn represent_type_uncached<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,

// For the moment, we can only apply these
// optimizations to safe pointers.
match cases[discr].find_ptr(cx) {
Some(ref df) if df.len() == 1 && st.fields.len() == 1 => {
match cases[discr].find_forbidden_value(cx) {
Some((ref df, forbidden_value))
if df.len() == 1 && st.fields.len() == 1 => {
let payload_ty = st.fields[0];
return RawForbiddenValue {
payload_discr: Disr::from(discr),
payload_ty: payload_ty,
forbidden_value: C_null(type_of::sizing_type_of(cx, payload_ty)),
forbidden_value: forbidden_value,
unit_fields: cases[1 - discr].tys.clone()
};
}
Some(mut discrfield) => {
discrfield.push(0);
discrfield.reverse();
return StructWrappedNullablePointer {
nndiscr: Disr::from(discr),
nonnull: st,
discrfield: discrfield,
nullfields: cases[1 - discr].tys.clone()
};
Some((mut discrfield, forbidden)) => {
if is_null(forbidden) {
discrfield.push(0);
discrfield.reverse();
return StructWrappedNullablePointer {
nndiscr: Disr::from(discr),
nonnull: st,
discrfield: discrfield,
nullfields: cases[1 - discr].tys.clone()
};
}
}
None => {}
}
Expand Down Expand Up @@ -466,60 +469,73 @@ struct Case<'tcx> {
/// This represents the (GEP) indices to follow to get to the discriminant field
pub type DiscrField = Vec<usize>;

fn find_discr_field_candidate<'tcx>(tcx: &ty::ctxt<'tcx>,
fn find_discr_field_candidate<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
Copy link
Member

Choose a reason for hiding this comment

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

Please either add a doc comment explaining what the two parts of the returned tuple are and/or switch from using a tuple to using a struct with self-explanatory named fields

ty: Ty<'tcx>,
mut path: DiscrField) -> Option<DiscrField> {
mut path: DiscrField) -> Option<(DiscrField, ValueRef)> {
let ref tcx = cx.tcx();
match ty.sty {
// Fat &T/&mut T/Box<T> i.e. T is [T], str, or Trait
ty::TyRef(_, ty::TypeAndMut { ty, .. }) | ty::TyBox(ty) if !type_is_sized(tcx, ty) => {
path.push(FAT_PTR_ADDR);
Some(path)
},

// Regular thin pointer: &T/&mut T/Box<T>
ty::TyRef(..) | ty::TyBox(..) => Some(path),

// Functions are just pointers
ty::TyBareFn(..) => Some(path),
// Fat &T/&mut T/Box<T> i.e. T is [T], str, or Trait: null is forbidden
ty::TyRef(_, ty::TypeAndMut { ty: ty2, .. }) | ty::TyBox(ty2)
if !type_is_sized(tcx, ty2) => {
path.push(FAT_PTR_ADDR);
Some((path, C_null(type_of::sizing_type_of(cx, ty))))
Copy link
Member

Choose a reason for hiding this comment

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

I agree with the other comments provided regarding this line: as I understand it, you are trying to construct an instance of the forbidden value that will be used to tag whether we are in the unit-case or in the payload case.

For these fat &Unsized things, the code you have here will construct a null pointer of type *struct { data: *i8, metadata: *i8 }. (This type description can be derived by inspecting the code for fn sizing_type_of, here.)

There are other places in the code, namely fn type_of here where unsized pointers do have that doubly-indirect pointer shape (see comment above fn type_of). But in this case, if I understand correctly, the null in question that is being used to mark the unit case is just a simple *i8 that will be stored in the first slot of that two word fat pointer.

},

// Regular thin pointer: &T/&mut T/Box<T>: null is forbidden
ty::TyRef(..) | ty::TyBox(..) => Some((path, C_null(type_of::sizing_type_of(cx, ty)))),

// Functions are just pointers: null is forbidden
ty::TyBareFn(..) => Some((path, C_null(type_of::sizing_type_of(cx, ty)))),

// Is this a char or a bool? If so, std::uXX:MAX is forbidden.
ty::TyChar => Some((path,
C_integral(type_of::sizing_type_of(cx, ty),
std::u32::MAX as u64, false))),
ty::TyBool => Some((path,
C_integral(type_of::sizing_type_of(cx, ty),
std::u8::MAX as u64, false))),

// Is this the NonZero lang item wrapping a pointer or integer type?
// If so, null is forbidden.
ty::TyStruct(def, substs) if Some(def.did) == tcx.lang_items.non_zero() => {
let nonzero_fields = &def.struct_variant().fields;
assert_eq!(nonzero_fields.len(), 1);
let field_ty = monomorphize::field_ty(tcx, substs, &nonzero_fields[0]);
match field_ty.sty {
ty::TyRawPtr(ty::TypeAndMut { ty, .. }) if !type_is_sized(tcx, ty) => {
path.extend_from_slice(&[0, FAT_PTR_ADDR]);
Some(path)
Some((path, C_null(Type::i8p(cx))))
},
ty::TyRawPtr(..) | ty::TyInt(..) | ty::TyUint(..) => {
path.push(0);
Some(path)
Some((path, C_null(type_of::sizing_type_of(cx, field_ty))))
},
_ => None
}
},

// Perhaps one of the fields of this struct is non-zero
// Perhaps one of the fields of this struct has a forbidden value.
// let's recurse and find out
ty::TyStruct(def, substs) => {
for (j, field) in def.struct_variant().fields.iter().enumerate() {
let field_ty = monomorphize::field_ty(tcx, substs, field);
if let Some(mut fpath) = find_discr_field_candidate(tcx, field_ty, path.clone()) {
if let Some((mut fpath, forbidden)) =
find_discr_field_candidate(cx, field_ty, path.clone()) {
fpath.push(j);
return Some(fpath);
return Some((fpath, forbidden));
}
}
None
},

// Perhaps one of the upvars of this struct is non-zero
// Perhaps one of the upvars of this struct has a forbidden value.
// Let's recurse and find out!
ty::TyClosure(_, ref substs) => {
for (j, &ty) in substs.upvar_tys.iter().enumerate() {
if let Some(mut fpath) = find_discr_field_candidate(tcx, ty, path.clone()) {
if let Some((mut fpath, forbidden)) =
find_discr_field_candidate(cx, ty, path.clone()) {
fpath.push(j);
return Some(fpath);
return Some((fpath, forbidden));
}
}
None
Expand All @@ -528,26 +544,27 @@ fn find_discr_field_candidate<'tcx>(tcx: &ty::ctxt<'tcx>,
// Can we use one of the fields in this tuple?
ty::TyTuple(ref tys) => {
for (j, &ty) in tys.iter().enumerate() {
if let Some(mut fpath) = find_discr_field_candidate(tcx, ty, path.clone()) {
if let Some((mut fpath, forbidden)) =
find_discr_field_candidate(cx, ty, path.clone()) {
fpath.push(j);
return Some(fpath);
return Some((fpath, forbidden));
}
}
None
},

// Is this a fixed-size array of something non-zero
// Is this a fixed-size array of something with a forbidden value
// with at least one element?
ty::TyArray(ety, d) if d > 0 => {
if let Some(mut vpath) = find_discr_field_candidate(tcx, ety, path) {
if let Some((mut vpath, forbidden)) = find_discr_field_candidate(cx, ety, path) {
vpath.push(0);
Some(vpath)
Some((vpath, forbidden))
} else {
None
}
},

// Anything else is not a pointer
// Anything else doesn't have a known-to-be-safe forbidden value.
_ => None
}
}
Expand All @@ -557,13 +574,22 @@ impl<'tcx> Case<'tcx> {
mk_struct(cx, &self.tys, false, scapegoat).size == 0
}

/// Find a safe pointer that may be used to discriminate in a
/// Find a forbidden value that may be used to discriminate in a
/// RawForbiddenValue or StructWrappedNullablePointer.
fn find_ptr<'a>(&self, cx: &CrateContext<'a, 'tcx>) -> Option<DiscrField> {
///
/// Example: In `Option<&T>`, since `&T` has a forbidden value 0,
/// this method will return the path to `&T`, with a value of 0.
///
/// Example: In `Option<(u64, char)>`, since `char` has a
/// forbidden value 2^32 - 1, this method will return the path to
/// the `char` field in the tuple, with a value of 2^32 - 1.
fn find_forbidden_value<'a>(&self, cx: &CrateContext<'a, 'tcx>) ->
Option<(DiscrField, ValueRef)> {
for (i, &ty) in self.tys.iter().enumerate() {
if let Some(mut path) = find_discr_field_candidate(cx.tcx(), ty, vec![]) {
if let Some((mut path, forbidden)) =
find_discr_field_candidate(cx, ty, vec![]) {
path.push(i);
return Some(path);
return Some((path, forbidden));
}
}
None
Expand Down Expand Up @@ -1129,7 +1155,8 @@ pub fn trans_field_ptr<'blk, 'tcx>(bcx: Block<'blk, 'tcx>, r: &Repr<'tcx>,
PointerCast(bcx, val.value, ty.ptr_to())
}
RawForbiddenValue { payload_discr, payload_ty, .. } => {
assert_eq!(ix, 0); // By definition, the payload of RawForbiddenValue has a single field.
// By definition, the payload of RawForbiddenValue has a single field.
assert_eq!(ix, 0);
assert_eq!(discr, payload_discr);
let ty = type_of::type_of(bcx.ccx(), payload_ty);
PointerCast(bcx, val.value, ty.ptr_to())
Expand Down
22 changes: 17 additions & 5 deletions src/test/run-pass/enum-null-pointer-opt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,21 +38,33 @@ fn main() {
assert_eq!(size_of::<&Trait>(), size_of::<Option<&Trait>>());
assert_eq!(size_of::<&mut Trait>(), size_of::<Option<&mut Trait>>());

// Chars
assert_eq!(size_of::<char>(), size_of::<Option<char>>());
assert_eq!(size_of::<char>(), size_of::<Result<char, ()>>());

// Bools
assert_eq!(size_of::<bool>(), size_of::<Option<bool>>());
assert_eq!(size_of::<bool>(), size_of::<Result<bool, ()>>());

// Pointers - Box<T>
assert_eq!(size_of::<Box<isize>>(), size_of::<Option<Box<isize>>>());

// The optimization can't apply to raw pointers
assert!(size_of::<Option<*const isize>>() != size_of::<*const isize>());
assert!(Some(0 as *const isize).is_some()); // Can't collapse None to null

struct Foo {
_a: Box<isize>
// The optimization can't apply to raw u32
assert!(size_of::<Option<u32>>() != size_of::<u32>());

struct Foo<T> {
_a: T
}
struct Bar(Box<isize>);
struct Bar<T>(T);

// Should apply through structs
assert_eq!(size_of::<Foo>(), size_of::<Option<Foo>>());
assert_eq!(size_of::<Bar>(), size_of::<Option<Bar>>());
assert_eq!(size_of::<Foo<Box<isize>>>(), size_of::<Option<Foo<Box<isize>>>>());
assert_eq!(size_of::<Bar<Box<isize>>>(), size_of::<Option<Bar<Box<isize>>>>());

// and tuples
assert_eq!(size_of::<(u8, Box<isize>)>(), size_of::<Option<(u8, Box<isize>)>>());
// and fixed-size arrays
Expand Down