Skip to content
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

Morph layout_raw query into layout_of. #88308

Merged
merged 1 commit into from
Aug 26, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
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 @@ -886,7 +898,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 @@ -1362,11 +1376,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 @@ -1703,7 +1715,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 @@ -2031,22 +2043,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);
Copy link
Member

Choose a reason for hiding this comment

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

I think the reasoning behind having this outside the query is that it increases the amount of cache hits.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but you're doing 2 cache hits (normalize + layout) instead of 1 (just layout).

This PR only penalizes the initial cache miss (layout gets cached for both normalized and unnormalized versions of the type), but benefits all further hits (since it reduces them to a single lookup).

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 @@ -2055,50 +2054,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 @@ -1063,7 +1063,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 @@ -1485,7 +1485,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