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

Eliminate ConstnessAnd again #91549

Merged
merged 10 commits into from
Dec 13, 2021
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use rustc_middle::ty::fold::TypeFoldable;
use rustc_middle::ty::subst::{GenericArgKind, SubstsRef, UserSubsts};
use rustc_middle::ty::{
self, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations, OpaqueTypeKey, RegionVid,
ToPredicate, Ty, TyCtxt, UserType, UserTypeAnnotationIndex, WithConstness,
ToPredicate, Ty, TyCtxt, UserType, UserTypeAnnotationIndex,
};
use rustc_span::def_id::CRATE_DEF_ID;
use rustc_span::{Span, DUMMY_SP};
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::interpret::{
};

use rustc_errors::ErrorReported;
use rustc_hir as hir;
use rustc_hir::def::DefKind;
use rustc_middle::mir;
use rustc_middle::mir::interpret::ErrorHandled;
Expand Down Expand Up @@ -215,6 +216,7 @@ pub fn eval_to_const_value_raw_provider<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
) -> ::rustc_middle::mir::interpret::EvalToConstValueResult<'tcx> {
assert!(key.param_env.constness() == hir::Constness::Const);
// see comment in eval_to_allocation_raw_provider for what we're doing here
if key.param_env.reveal() == Reveal::All {
let mut key = key;
Expand Down Expand Up @@ -249,6 +251,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
) -> ::rustc_middle::mir::interpret::EvalToAllocationRawResult<'tcx> {
assert!(key.param_env.constness() == hir::Constness::Const);
// Because the constant is computed twice (once per value of `Reveal`), we are at risk of
// reporting the same error twice here. To resolve this, we check whether we can evaluate the
// constant in the more restrictive `Reveal::UserFacing`, which most likely already was
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -929,6 +929,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
} else {
self.param_env
};
let param_env = param_env.with_const();
Copy link
Contributor

Choose a reason for hiding this comment

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

From past experience with similar regressions I would guess that this causes inlining of eval_to_allocation to change, messing up other LLVM optimizations at the caller sites or at other function calls within in this function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I add #[inline] to these functions or should I remove these calls and assertions altogether?

Copy link
Contributor

Choose a reason for hiding this comment

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

Either profile to see what is now not inlined, or add inline (maybe even always or never) to this function, or any of the functions it calls. I'm not sure what exactly is needed, and only profiling can tell for sure

let val = self.tcx.eval_to_allocation_raw(param_env.and(gid))?;
self.raw_const_to_mplace(val)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -817,8 +817,7 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {
);

let implsrc = tcx.infer_ctxt().enter(|infcx| {
let mut selcx =
SelectionContext::with_constness(&infcx, hir::Constness::Const);
let mut selcx = SelectionContext::new(&infcx);
selcx.select(&obligation)
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
//! See the `Qualif` trait for more info.

use rustc_errors::ErrorReported;
use rustc_hir as hir;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, subst::SubstsRef, AdtDef, Ty};
Expand Down Expand Up @@ -167,7 +166,7 @@ impl Qualif for NeedsNonConstDrop {
);

let implsrc = cx.tcx.infer_ctxt().enter(|infcx| {
let mut selcx = SelectionContext::with_constness(&infcx, hir::Constness::Const);
let mut selcx = SelectionContext::new(&infcx);
selcx.select(&obligation)
});
!matches!(
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_data_structures/src/tagged_ptr/copy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,11 @@ where
// SAFETY: pointer_raw returns the original pointer
unsafe { std::mem::transmute_copy(&self.pointer_raw()) }
}
#[inline]
pub fn tag(&self) -> T {
unsafe { T::from_usize(self.packed.get() >> Self::TAG_BIT_SHIFT) }
}
#[inline]
pub fn set_tag(&mut self, tag: T) {
let mut packed = self.packed.get();
let new_tag = T::into_usize(tag) << Self::TAG_BIT_SHIFT;
Expand Down
25 changes: 0 additions & 25 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3210,31 +3210,6 @@ impl<'hir> Node<'hir> {
}
}

/// Returns `Constness::Const` when this node is a const fn/impl/item.
pub fn constness_for_typeck(&self) -> Constness {
match self {
Node::Item(Item {
kind: ItemKind::Fn(FnSig { header: FnHeader { constness, .. }, .. }, ..),
..
})
| Node::TraitItem(TraitItem {
kind: TraitItemKind::Fn(FnSig { header: FnHeader { constness, .. }, .. }, ..),
..
})
| Node::ImplItem(ImplItem {
kind: ImplItemKind::Fn(FnSig { header: FnHeader { constness, .. }, .. }, ..),
..
})
| Node::Item(Item { kind: ItemKind::Impl(Impl { constness, .. }), .. }) => *constness,

Node::Item(Item { kind: ItemKind::Const(..), .. })
| Node::TraitItem(TraitItem { kind: TraitItemKind::Const(..), .. })
| Node::ImplItem(ImplItem { kind: ImplItemKind::Const(..), .. }) => Constness::Const,

_ => Constness::NotConst,
}
}

pub fn as_owner(self) -> Option<OwnerNode<'hir>> {
match self {
Node::Item(i) => Some(OwnerNode::Item(i)),
Expand Down
20 changes: 1 addition & 19 deletions compiler/rustc_infer/src/traits/engine.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use crate::infer::InferCtxt;
use crate::traits::Obligation;
use rustc_data_structures::fx::FxHashMap;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::{self, ToPredicate, Ty, WithConstness};
use rustc_middle::ty::{self, ToPredicate, Ty};

use super::FulfillmentError;
use super::{ObligationCause, PredicateObligation};
Expand Down Expand Up @@ -48,26 +47,9 @@ pub trait TraitEngine<'tcx>: 'tcx {

fn select_all_or_error(&mut self, infcx: &InferCtxt<'_, 'tcx>) -> Vec<FulfillmentError<'tcx>>;

fn select_all_with_constness_or_error(
&mut self,
infcx: &InferCtxt<'_, 'tcx>,
_constness: hir::Constness,
) -> Vec<FulfillmentError<'tcx>> {
self.select_all_or_error(infcx)
}

fn select_where_possible(&mut self, infcx: &InferCtxt<'_, 'tcx>)
-> Vec<FulfillmentError<'tcx>>;

// FIXME(fee1-dead) this should not provide a default body for chalk as chalk should be updated
fn select_with_constness_where_possible(
&mut self,
infcx: &InferCtxt<'_, 'tcx>,
_constness: hir::Constness,
) -> Vec<FulfillmentError<'tcx>> {
self.select_where_possible(infcx)
}

fn pending_obligations(&self) -> Vec<PredicateObligation<'tcx>>;

fn relationships(&mut self) -> &mut FxHashMap<ty::TyVid, ty::FoundRelationships>;
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_infer/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ impl PredicateObligation<'tcx> {
}
}

impl TraitObligation<'tcx> {
/// Returns `true` if the trait predicate is considered `const` in its ParamEnv.
pub fn is_const(&self) -> bool {
match (self.predicate.skip_binder().constness, self.param_env.constness()) {
(ty::BoundConstness::ConstIfConst, hir::Constness::Const) => true,
_ => false,
}
}
}

// `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger.
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
static_assert_size!(PredicateObligation<'_>, 32);
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_infer/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use smallvec::smallvec;
use crate::infer::outlives::components::{push_outlives_components, Component};
use crate::traits::{Obligation, ObligationCause, PredicateObligation};
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_middle::ty::{self, ToPredicate, TyCtxt, WithConstness};
use rustc_middle::ty::{self, ToPredicate, TyCtxt};
use rustc_span::symbol::Ident;
use rustc_span::Span;

Expand Down Expand Up @@ -328,8 +328,8 @@ pub fn transitive_bounds_that_define_assoc_type<'tcx>(
));
for (super_predicate, _) in super_predicates.predicates {
let subst_predicate = super_predicate.subst_supertrait(tcx, &trait_ref);
if let Some(binder) = subst_predicate.to_opt_poly_trait_ref() {
stack.push(binder.value);
if let Some(binder) = subst_predicate.to_opt_poly_trait_pred() {
stack.push(binder.map_bound(|t| t.trait_ref));
}
}

Expand Down Expand Up @@ -362,8 +362,8 @@ impl<'tcx, I: Iterator<Item = PredicateObligation<'tcx>>> Iterator for FilterToT

fn next(&mut self) -> Option<ty::PolyTraitRef<'tcx>> {
while let Some(obligation) = self.base_iterator.next() {
if let Some(data) = obligation.predicate.to_opt_poly_trait_ref() {
return Some(data.value);
if let Some(data) = obligation.predicate.to_opt_poly_trait_pred() {
return Some(data.map_bound(|t| t.trait_ref));
}
}
None
Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_macros/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ enum QueryModifier {

/// Use a separate query provider for local and extern crates
SeparateProvideExtern(Ident),

/// Always remap the ParamEnv's constness before hashing and passing to the query provider
RemapEnvConstness(Ident),
}

impl Parse for QueryModifier {
Expand Down Expand Up @@ -123,6 +126,8 @@ impl Parse for QueryModifier {
Ok(QueryModifier::EvalAlways(modifier))
} else if modifier == "separate_provide_extern" {
Ok(QueryModifier::SeparateProvideExtern(modifier))
} else if modifier == "remap_env_constness" {
Ok(QueryModifier::RemapEnvConstness(modifier))
} else {
Err(Error::new(modifier.span(), "unknown query modifier"))
}
Expand Down Expand Up @@ -222,6 +227,9 @@ struct QueryModifiers {

/// Use a separate query provider for local and extern crates
separate_provide_extern: Option<Ident>,

/// Always remap the ParamEnv's constness before hashing.
remap_env_constness: Option<Ident>,
}

/// Process query modifiers into a struct, erroring on duplicates
Expand All @@ -236,6 +244,7 @@ fn process_modifiers(query: &mut Query) -> QueryModifiers {
let mut anon = None;
let mut eval_always = None;
let mut separate_provide_extern = None;
let mut remap_env_constness = None;
for modifier in query.modifiers.0.drain(..) {
match modifier {
QueryModifier::LoadCached(tcx, id, block) => {
Expand Down Expand Up @@ -335,6 +344,12 @@ fn process_modifiers(query: &mut Query) -> QueryModifiers {
}
separate_provide_extern = Some(ident);
}
QueryModifier::RemapEnvConstness(ident) => {
if remap_env_constness.is_some() {
panic!("duplicate modifier `remap_env_constness` for query `{}`", query.name);
}
remap_env_constness = Some(ident)
}
}
}
let desc = desc.unwrap_or_else(|| {
Expand All @@ -351,6 +366,7 @@ fn process_modifiers(query: &mut Query) -> QueryModifiers {
anon,
eval_always,
separate_provide_extern,
remap_env_constness,
}
}

Expand Down Expand Up @@ -485,6 +501,10 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
if let Some(separate_provide_extern) = &modifiers.separate_provide_extern {
attributes.push(quote! { (#separate_provide_extern) });
}
// Pass on the remap_env_constness modifier
if let Some(remap_env_constness) = &modifiers.remap_env_constness {
attributes.push(quote! { (#remap_env_constness) });
}

// This uses the span of the query definition for the commas,
// which can be important if we later encounter any ambiguity
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,8 @@ impl<'hir> Map<'hir> {
/// Panics if `LocalDefId` does not have an associated body.
///
/// This should only be used for determining the context of a body, a return
/// value of `Some` does not always suggest that the owner of the body is `const`.
/// value of `Some` does not always suggest that the owner of the body is `const`,
/// just that it has to be checked as if it were.
pub fn body_const_context(&self, did: LocalDefId) -> Option<ConstContext> {
let hir_id = self.local_def_id_to_hir_id(did);
let ccx = match self.body_owner_kind(hir_id) {
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/infer/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ impl<'tcx, R> Canonical<'tcx, QueryResponse<'tcx, R>> {
}
}

impl<'tcx, R> Canonical<'tcx, ty::ParamEnvAnd<'tcx, R>> {
#[inline]
pub fn without_const(mut self) -> Self {
self.value = self.value.without_const();
self
}
}

impl<'tcx, V> Canonical<'tcx, V> {
/// Allows you to map the `value` of a canonical while keeping the
/// same set of bound variables.
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl<'tcx> TyCtxt<'tcx> {
cid: GlobalId<'tcx>,
span: Option<Span>,
) -> EvalToConstValueResult<'tcx> {
let param_env = param_env.with_const();
// Const-eval shouldn't depend on lifetimes at all, so we can erase them, which should
// improve caching of queries.
let inputs = self.erase_regions(param_env.and(cid));
Expand Down Expand Up @@ -92,6 +93,7 @@ impl<'tcx> TyCtxt<'tcx> {
gid: GlobalId<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> Result<&'tcx mir::Allocation, ErrorHandled> {
let param_env = param_env.with_const();
trace!("eval_to_allocation: Need to compute {:?}", gid);
let raw_const = self.eval_to_allocation_raw(param_env.and(gid))?;
Ok(self.global_alloc(raw_const.alloc_id).unwrap_memory())
Expand Down
Loading