Skip to content

Commit

Permalink
Auto merge of #88308 - eddyb:cooked-layouts, r=nagisa
Browse files Browse the repository at this point in the history
Morph `layout_raw` query into `layout_of`.

Before this PR, `LayoutCx::layout_of` wrapped the `layout_raw` query, to:
* normalize the type, before attempting to compute the layout
* pass the layout to `record_layout_for_printing`, for `-Zprint-type-sizes`

Moving those two responsibilities into the query may reduce overhead (due to cached calls skipping those steps), but I want to do a perf run to know.

One of the changes I had to make was changing the return type of the query, to be able to both get out the type produced by normalizing inside the query *and* to match the signature of the old `TyCtxt::layout_of`. This change may be worse, perf-wise, so that's another reason I want to check.

r? `@nagisa` cc `@oli-obk`
  • Loading branch information
bors committed Aug 26, 2021
2 parents 20997f6 + edb4b2d commit 4b9f4b2
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 94 deletions.
10 changes: 6 additions & 4 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1100,10 +1100,12 @@ rustc_queries! {
cache_on_disk_if { false }
}

query layout_raw(
env: ty::ParamEnvAnd<'tcx, Ty<'tcx>>
) -> Result<&'tcx rustc_target::abi::Layout, ty::layout::LayoutError<'tcx>> {
desc { "computing layout of `{}`", env.value }
/// Computes the layout of a type. Note that this implicitly
/// executes in "reveal all" mode, and will normalize the input type.
query layout_of(
key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>
) -> Result<ty::layout::TyAndLayout<'tcx>, ty::layout::LayoutError<'tcx>> {
desc { "computing layout of `{}`", key.value }
}

query dylib_dependency_formats(_: CrateNum)
Expand Down
108 changes: 33 additions & 75 deletions compiler/rustc_middle/src/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,10 @@ impl<'tcx> fmt::Display for LayoutError<'tcx> {
}
}

fn layout_raw<'tcx>(
fn layout_of<'tcx>(
tcx: TyCtxt<'tcx>,
query: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> Result<&'tcx Layout, LayoutError<'tcx>> {
) -> Result<TyAndLayout<'tcx>, LayoutError<'tcx>> {
ty::tls::with_related_context(tcx, move |icx| {
let (param_env, ty) = query.into_parts();

Expand All @@ -220,21 +220,33 @@ fn layout_raw<'tcx>(
let icx = ty::tls::ImplicitCtxt { layout_depth: icx.layout_depth + 1, ..icx.clone() };

ty::tls::enter_context(&icx, |_| {
let param_env = param_env.with_reveal_all_normalized(tcx);
let unnormalized_ty = ty;
let ty = tcx.normalize_erasing_regions(param_env, ty);
if ty != unnormalized_ty {
// Ensure this layout is also cached for the normalized type.
return tcx.layout_of(param_env.and(ty));
}

let cx = LayoutCx { tcx, param_env };
let layout = cx.layout_raw_uncached(ty);

let layout = cx.layout_of_uncached(ty)?;
let layout = TyAndLayout { ty, layout };

cx.record_layout_for_printing(layout);

// Type-level uninhabitedness should always imply ABI uninhabitedness.
if let Ok(layout) = layout {
if tcx.conservative_is_privately_uninhabited(param_env.and(ty)) {
assert!(layout.abi.is_uninhabited());
}
if tcx.conservative_is_privately_uninhabited(param_env.and(ty)) {
assert!(layout.abi.is_uninhabited());
}
layout

Ok(layout)
})
})
}

pub fn provide(providers: &mut ty::query::Providers) {
*providers = ty::query::Providers { layout_raw, ..*providers };
*providers = ty::query::Providers { layout_of, ..*providers };
}

pub struct LayoutCx<'tcx, C> {
Expand Down Expand Up @@ -492,7 +504,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
})
}

fn layout_raw_uncached(&self, ty: Ty<'tcx>) -> Result<&'tcx Layout, LayoutError<'tcx>> {
fn layout_of_uncached(&self, ty: Ty<'tcx>) -> Result<&'tcx Layout, LayoutError<'tcx>> {
let tcx = self.tcx;
let param_env = self.param_env;
let dl = self.data_layout();
Expand Down Expand Up @@ -889,7 +901,9 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let present_first = match present_first {
Some(present_first) => present_first,
// Uninhabited because it has no variants, or only absent ones.
None if def.is_enum() => return tcx.layout_raw(param_env.and(tcx.types.never)),
None if def.is_enum() => {
return Ok(tcx.layout_of(param_env.and(tcx.types.never))?.layout);
}
// If it's a struct, still compute a layout so that we can still compute the
// field offsets.
None => VariantIdx::new(0),
Expand Down Expand Up @@ -1368,11 +1382,9 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {

// Types with no meaningful known layout.
ty::Projection(_) | ty::Opaque(..) => {
let normalized = tcx.normalize_erasing_regions(param_env, ty);
if ty == normalized {
return Err(LayoutError::Unknown(ty));
}
tcx.layout_raw(param_env.and(normalized))?
// NOTE(eddyb) `layout_of` query should've normalized these away,
// if that was possible, so there's no reason to try again here.
return Err(LayoutError::Unknown(ty));
}

ty::Placeholder(..) | ty::GeneratorWitness(..) | ty::Infer(_) => {
Expand Down Expand Up @@ -1712,7 +1724,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
Ok(layout)
}

/// This is invoked by the `layout_raw` query to record the final
/// This is invoked by the `layout_of` query to record the final
/// layout of each type.
#[inline(always)]
fn record_layout_for_printing(&self, layout: TyAndLayout<'tcx>) {
Expand Down Expand Up @@ -2040,22 +2052,9 @@ impl<'tcx> LayoutOf for LayoutCx<'tcx, TyCtxt<'tcx>> {
type TyAndLayout = Result<TyAndLayout<'tcx>, LayoutError<'tcx>>;

/// Computes the layout of a type. Note that this implicitly
/// executes in "reveal all" mode.
/// executes in "reveal all" mode, and will normalize the input type.
fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyAndLayout {
let param_env = self.param_env.with_reveal_all_normalized(self.tcx);
let ty = self.tcx.normalize_erasing_regions(param_env, ty);
let layout = self.tcx.layout_raw(param_env.and(ty))?;
let layout = TyAndLayout { ty, layout };

// N.B., this recording is normally disabled; when enabled, it
// can however trigger recursive invocations of `layout_of`.
// Therefore, we execute it *after* the main query has
// completed, to avoid problems around recursive structures
// and the like. (Admittedly, I wasn't able to reproduce a problem
// here, but it seems like the right thing to do. -nmatsakis)
self.record_layout_for_printing(layout);

Ok(layout)
self.tcx.layout_of(self.param_env.and(ty))
}
}

Expand All @@ -2064,50 +2063,9 @@ impl LayoutOf for LayoutCx<'tcx, ty::query::TyCtxtAt<'tcx>> {
type TyAndLayout = Result<TyAndLayout<'tcx>, LayoutError<'tcx>>;

/// Computes the layout of a type. Note that this implicitly
/// executes in "reveal all" mode.
/// executes in "reveal all" mode, and will normalize the input type.
fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyAndLayout {
let param_env = self.param_env.with_reveal_all_normalized(*self.tcx);
let ty = self.tcx.normalize_erasing_regions(param_env, ty);
let layout = self.tcx.layout_raw(param_env.and(ty))?;
let layout = TyAndLayout { ty, layout };

// N.B., this recording is normally disabled; when enabled, it
// can however trigger recursive invocations of `layout_of`.
// Therefore, we execute it *after* the main query has
// completed, to avoid problems around recursive structures
// and the like. (Admittedly, I wasn't able to reproduce a problem
// here, but it seems like the right thing to do. -nmatsakis)
let cx = LayoutCx { tcx: *self.tcx, param_env: self.param_env };
cx.record_layout_for_printing(layout);

Ok(layout)
}
}

// Helper (inherent) `layout_of` methods to avoid pushing `LayoutCx` to users.
impl TyCtxt<'tcx> {
/// Computes the layout of a type. Note that this implicitly
/// executes in "reveal all" mode.
#[inline]
pub fn layout_of(
self,
param_env_and_ty: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> Result<TyAndLayout<'tcx>, LayoutError<'tcx>> {
let cx = LayoutCx { tcx: self, param_env: param_env_and_ty.param_env };
cx.layout_of(param_env_and_ty.value)
}
}

impl ty::query::TyCtxtAt<'tcx> {
/// Computes the layout of a type. Note that this implicitly
/// executes in "reveal all" mode.
#[inline]
pub fn layout_of(
self,
param_env_and_ty: ty::ParamEnvAnd<'tcx, Ty<'tcx>>,
) -> Result<TyAndLayout<'tcx>, LayoutError<'tcx>> {
let cx = LayoutCx { tcx: self.at(self.span), param_env: param_env_and_ty.param_env };
cx.layout_of(param_env_and_ty.value)
self.tcx.layout_of(self.param_env.and(ty))
}
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir/src/util/alignment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ where
};

let ty = place.ty(local_decls, tcx).ty;
match tcx.layout_raw(param_env.and(ty)) {
match tcx.layout_of(param_env.and(ty)) {
Ok(layout) if layout.align.abi <= pack => {
// If the packed alignment is greater or equal to the field alignment, the type won't be
// further disaligned.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_target/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ impl Layout {
/// to that obtained from `layout_of(ty)`, as we need to produce
/// layouts for which Rust types do not exist, such as enum variants
/// or synthetic fields of enums (i.e., discriminants) and fat pointers.
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable_Generic)]
pub struct TyAndLayout<'a, Ty> {
pub ty: Ty,
pub layout: &'a Layout,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1481,7 +1481,7 @@ fn restrict_repr_packed_field_ref_capture<'tcx>(
match p.kind {
ProjectionKind::Field(..) => match ty.kind() {
ty::Adt(def, _) if def.repr.packed() => {
match tcx.layout_raw(param_env.and(p.ty)) {
match tcx.layout_of(param_env.and(p.ty)) {
Ok(layout) if layout.align.abi.bytes() == 1 => {
// if the alignment is 1, the type can't be further
// disaligned.
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/consts/const-size_of-cycle.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ note: ...which requires const-evaluating + checking `Foo::bytes::{constant#0}`..
LL | bytes: [u8; std::mem::size_of::<Foo>()]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
= note: ...which requires computing layout of `Foo`...
= note: ...which requires computing layout of `[u8; _]`...
= note: ...which requires normalizing `[u8; _]`...
= note: ...which again requires simplifying constant for the type system `Foo::bytes::{constant#0}`, completing the cycle
note: cycle used when checking that `Foo` is well-formed
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/consts/issue-44415.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ note: ...which requires const-evaluating + checking `Foo::bytes::{constant#0}`..
LL | bytes: [u8; unsafe { intrinsics::size_of::<Foo>() }],
| ^^^^^^
= note: ...which requires computing layout of `Foo`...
= note: ...which requires computing layout of `[u8; _]`...
= note: ...which requires normalizing `[u8; _]`...
= note: ...which again requires simplifying constant for the type system `Foo::bytes::{constant#0}`, completing the cycle
note: cycle used when checking that `Foo` is well-formed
Expand Down
9 changes: 5 additions & 4 deletions src/test/ui/recursion/issue-26548-recursion-via-normalize.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//~ ERROR cycle detected when computing layout of
//~| NOTE ...which requires computing layout of
//~| NOTE ...which again requires computing layout of
//~ ERROR cycle detected when computing layout of `S`
//~| NOTE ...which requires computing layout of `std::option::Option<<S as Mirror>::It>`...
//~| NOTE ...which requires computing layout of `std::option::Option<S>`...
//~| NOTE ...which again requires computing layout of `S`, completing the cycle
//~| NOTE cycle used when computing layout of `std::option::Option<S>`

// build-fail

Expand All @@ -13,6 +15,5 @@ impl<T: ?Sized> Mirror for T {
struct S(Option<<S as Mirror>::It>);

fn main() {
//~^ NOTE cycle used when optimizing MIR for `main`
let _s = S(None);
}
13 changes: 5 additions & 8 deletions src/test/ui/recursion/issue-26548-recursion-via-normalize.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
error[E0391]: cycle detected when computing layout of `std::option::Option<S>`
error[E0391]: cycle detected when computing layout of `S`
|
= note: ...which requires computing layout of `S`...
= note: ...which again requires computing layout of `std::option::Option<S>`, completing the cycle
note: cycle used when optimizing MIR for `main`
--> $DIR/issue-26548-recursion-via-normalize.rs:15:1
|
LL | fn main() {
| ^^^^^^^^^
= note: ...which requires computing layout of `std::option::Option<<S as Mirror>::It>`...
= note: ...which requires computing layout of `std::option::Option<S>`...
= note: ...which again requires computing layout of `S`, completing the cycle
= note: cycle used when computing layout of `std::option::Option<S>`

error: aborting due to previous error

Expand Down

0 comments on commit 4b9f4b2

Please sign in to comment.