-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Multi-variant layouts for generators #59897
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
Changes from 10 commits
52e4407
9e06f25
70c1b6c
4de2d8a
5a7af54
961ba95
6e2e17d
f772c39
b8f6de4
9ef2c30
f7c2f24
15dbe65
77a6d29
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -604,12 +604,57 @@ impl<'a, 'tcx> LayoutCx<'tcx, TyCtxt<'a, 'tcx, 'tcx>> { | |
tcx.intern_layout(unit) | ||
} | ||
|
||
// Tuples, generators and closures. | ||
ty::Generator(def_id, ref substs, _) => { | ||
let tys = substs.field_tys(def_id, tcx); | ||
univariant(&tys.map(|ty| self.layout_of(ty)).collect::<Result<Vec<_>, _>>()?, | ||
let discr_index = substs.prefix_tys(def_id, tcx).count(); | ||
let prefix_tys = substs.prefix_tys(def_id, tcx) | ||
.chain(iter::once(substs.discr_ty(tcx))); | ||
let prefix = univariant_uninterned( | ||
&prefix_tys.map(|ty| self.layout_of(ty)).collect::<Result<Vec<_>, _>>()?, | ||
&ReprOptions::default(), | ||
StructKind::AlwaysSized)? | ||
StructKind::AlwaysSized)?; | ||
|
||
let mut size = prefix.size; | ||
let mut align = prefix.align; | ||
let variants_tys = substs.state_tys(def_id, tcx); | ||
let variants = variants_tys.enumerate().map(|(i, variant_tys)| { | ||
let mut variant = univariant_uninterned( | ||
&variant_tys.map(|ty| self.layout_of(ty)).collect::<Result<Vec<_>, _>>()?, | ||
&ReprOptions::default(), | ||
StructKind::Prefixed(prefix.size, prefix.align.abi))?; | ||
|
||
variant.variants = Variants::Single { index: VariantIdx::new(i) }; | ||
|
||
size = size.max(variant.size); | ||
align = align.max(variant.align); | ||
|
||
Ok(variant) | ||
}).collect::<Result<IndexVec<VariantIdx, _>, _>>()?; | ||
|
||
let abi = if prefix.abi.is_uninhabited() || | ||
variants.iter().all(|v| v.abi.is_uninhabited()) { | ||
Abi::Uninhabited | ||
} else { | ||
Abi::Aggregate { sized: true } | ||
}; | ||
let discr = match &self.layout_of(substs.discr_ty(tcx))?.abi { | ||
Abi::Scalar(s) => s.clone(), | ||
_ => bug!(), | ||
}; | ||
|
||
let layout = tcx.intern_layout(LayoutDetails { | ||
variants: Variants::Multiple { | ||
discr, | ||
discr_kind: DiscriminantKind::Tag, | ||
discr_index, | ||
variants, | ||
}, | ||
fields: prefix.fields, | ||
abi, | ||
size, | ||
align, | ||
}); | ||
debug!("generator layout: {:#?}", layout); | ||
layout | ||
} | ||
|
||
ty::Closure(def_id, ref substs) => { | ||
|
@@ -1646,6 +1691,14 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx> | |
|
||
fn field(this: TyLayout<'tcx>, cx: &C, i: usize) -> C::TyLayout { | ||
let tcx = cx.tcx(); | ||
let handle_discriminant = |discr: &Scalar| -> C::TyLayout { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd call this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed. Pre-interning feels out of scope, since we are handling all fields more or less the same here? |
||
let layout = LayoutDetails::scalar(cx, discr.clone()); | ||
MaybeResult::from_ok(TyLayout { | ||
details: tcx.intern_layout(layout), | ||
ty: discr.value.to_ty(tcx) | ||
}) | ||
}; | ||
|
||
cx.layout_of(match this.ty.sty { | ||
ty::Bool | | ||
ty::Char | | ||
|
@@ -1720,7 +1773,19 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx> | |
} | ||
|
||
ty::Generator(def_id, ref substs, _) => { | ||
substs.field_tys(def_id, tcx).nth(i).unwrap() | ||
match this.variants { | ||
Variants::Single { index } => { | ||
substs.state_tys(def_id, tcx) | ||
.nth(index.as_usize()).unwrap() | ||
.nth(i).unwrap() | ||
} | ||
Variants::Multiple { ref discr, discr_index, .. } => { | ||
if i == discr_index { | ||
return handle_discriminant(discr); | ||
} | ||
substs.prefix_tys(def_id, tcx).nth(i).unwrap() | ||
} | ||
} | ||
} | ||
|
||
ty::Tuple(tys) => tys[i], | ||
|
@@ -1740,11 +1805,7 @@ impl<'a, 'tcx, C> TyLayoutMethods<'tcx, C> for Ty<'tcx> | |
// Discriminant field for enums (where applicable). | ||
Variants::Multiple { ref discr, .. } => { | ||
assert_eq!(i, 0); | ||
let layout = LayoutDetails::scalar(cx, discr.clone()); | ||
return MaybeResult::from_ok(TyLayout { | ||
details: tcx.intern_layout(layout), | ||
ty: discr.value.to_ty(tcx) | ||
}); | ||
return handle_discriminant(discr); | ||
} | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the inner
Vec
could beIndexVec<Field
.Also, surprised this is
LocalDecl
, is that only for debuginfo?If so, can you add something similar to
__upvar_debuginfo_codegen_only_do_not_use
, to ensure there is no cross-talk between debuginfo and non-debuginfo usecases?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, no, we also use it to get the type of the fields in
GeneratorSubsts::state_tys()
. I suppose I can turn it into a struct with something like__upvar_debuginfo_codegen_only_do_not_use
and whatever struct we should be using here. (What should we be using? Right now we only need the type, I think.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant that
GeneratorLayout
should only haveTy<'tcx>
, and that debuginfo should be separate.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I separated these out and made a
GeneratorField
newtype. That way our layout code will also know when the same field is in two variants so it can lay them out correctly (see the FIXME I added).