-
Notifications
You must be signed in to change notification settings - Fork 13.3k
instance: polymorphize shims #75414
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
instance: polymorphize shims #75414
Changes from all commits
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 | ||
---|---|---|---|---|
|
@@ -16,7 +16,7 @@ use rustc_middle::ty::{ | |||
fold::{TypeFoldable, TypeVisitor}, | ||||
query::Providers, | ||||
subst::SubstsRef, | ||||
Const, Ty, TyCtxt, | ||||
Const, InstanceDef, Ty, TyCtxt, | ||||
}; | ||||
use rustc_span::symbol::sym; | ||||
use std::convert::TryInto; | ||||
|
@@ -27,21 +27,25 @@ pub fn provide(providers: &mut Providers) { | |||
providers.unused_generic_params = unused_generic_params; | ||||
} | ||||
|
||||
/// Determine which generic parameters are used by the function/method/closure represented by | ||||
/// `def_id`. Returns a bitset where bits representing unused parameters are set (`is_empty` | ||||
/// Determine which generic parameters are used by the `instance`. | ||||
/// | ||||
/// Returns a bitset where bits representing unused parameters are set (`is_empty` | ||||
/// indicates all parameters are used). | ||||
fn unused_generic_params(tcx: TyCtxt<'_>, def_id: DefId) -> FiniteBitSet<u32> { | ||||
debug!("unused_generic_params({:?})", def_id); | ||||
fn unused_generic_params<'tcx>( | ||||
tcx: TyCtxt<'tcx>, | ||||
instance: InstanceDef<'tcx>, | ||||
) -> FiniteBitSet<u32> { | ||||
debug!("unused_generic_params({:?})", instance); | ||||
|
||||
// If polymorphization disabled, then all parameters are used. | ||||
if !tcx.sess.opts.debugging_opts.polymorphize { | ||||
// If polymorphization disabled, then all parameters are used. | ||||
return FiniteBitSet::new_empty(); | ||||
} | ||||
|
||||
// Polymorphization results are stored in cross-crate metadata only when there are unused | ||||
// parameters, so assume that non-local items must have only used parameters (else this query | ||||
// would not be invoked, and the cross-crate metadata used instead). | ||||
if !def_id.is_local() { | ||||
// Exit early if this instance should not be polymorphized. | ||||
let def_id = instance.def_id(); | ||||
if !should_polymorphize(tcx, def_id, instance) { | ||||
debug!("unused_generic_params: skipping"); | ||||
return FiniteBitSet::new_empty(); | ||||
} | ||||
|
||||
|
@@ -53,36 +57,24 @@ fn unused_generic_params(tcx: TyCtxt<'_>, def_id: DefId) -> FiniteBitSet<u32> { | |||
return FiniteBitSet::new_empty(); | ||||
} | ||||
|
||||
// Exit early when there is no MIR available. | ||||
let context = tcx.hir().body_const_context(def_id.expect_local()); | ||||
match context { | ||||
Some(ConstContext::ConstFn) | None if !tcx.is_mir_available(def_id) => { | ||||
debug!("unused_generic_params: (no mir available) def_id={:?}", def_id); | ||||
return FiniteBitSet::new_empty(); | ||||
} | ||||
Some(_) if !tcx.is_ctfe_mir_available(def_id) => { | ||||
debug!("unused_generic_params: (no ctfe mir available) def_id={:?}", def_id); | ||||
return FiniteBitSet::new_empty(); | ||||
} | ||||
_ => {} | ||||
} | ||||
|
||||
// Create a bitset with N rightmost ones for each parameter. | ||||
let generics_count: u32 = | ||||
generics.count().try_into().expect("more generic parameters than can fit into a `u32`"); | ||||
let mut unused_parameters = FiniteBitSet::<u32>::new_empty(); | ||||
unused_parameters.set_range(0..generics_count); | ||||
debug!("unused_generic_params: (start) unused_parameters={:?}", unused_parameters); | ||||
|
||||
mark_used_by_default_parameters(tcx, def_id, generics, &mut unused_parameters); | ||||
debug!("unused_generic_params: (after default) unused_parameters={:?}", unused_parameters); | ||||
|
||||
// Visit MIR and accumululate used generic parameters. | ||||
let body = match context { | ||||
let body = match tcx.hir().body_const_context(def_id.expect_local()) { | ||||
// Const functions are actually called and should thus be considered for polymorphization | ||||
// via their runtime MIR | ||||
// via their runtime MIR. | ||||
Some(ConstContext::ConstFn) | None => tcx.optimized_mir(def_id), | ||||
Some(_) => tcx.mir_for_ctfe(def_id), | ||||
}; | ||||
|
||||
// Visit MIR and accumululate used generic parameters. | ||||
let mut vis = MarkUsedGenericParams { tcx, def_id, unused_parameters: &mut unused_parameters }; | ||||
vis.visit_body(body); | ||||
debug!("unused_generic_params: (after visitor) unused_parameters={:?}", unused_parameters); | ||||
|
@@ -98,6 +90,48 @@ fn unused_generic_params(tcx: TyCtxt<'_>, def_id: DefId) -> FiniteBitSet<u32> { | |||
unused_parameters | ||||
} | ||||
|
||||
/// Returns `true` if the `InstanceDef` should be polymorphized. | ||||
fn should_polymorphize<'tcx>( | ||||
tcx: TyCtxt<'tcx>, | ||||
def_id: DefId, | ||||
instance: ty::InstanceDef<'tcx>, | ||||
) -> bool { | ||||
// If a instance's MIR body is not polymorphic then the modified substitutions that are derived | ||||
// from polymorphization's result won't make any difference. | ||||
if !instance.has_polymorphic_mir_body() { | ||||
return false; | ||||
} | ||||
|
||||
// Don't polymorphize intrinsics or virtual calls - calling `instance_mir` will panic. | ||||
if matches!(instance, ty::InstanceDef::Intrinsic(..) | ty::InstanceDef::Virtual(..)) { | ||||
return false; | ||||
} | ||||
|
||||
// Polymorphization results are stored in cross-crate metadata only when there are unused | ||||
// parameters, so assume that non-local items must have only used parameters (else this query | ||||
// would not be invoked, and the cross-crate metadata used instead). | ||||
if !def_id.is_local() { | ||||
return false; | ||||
} | ||||
|
||||
// Foreign items don't have a body to analyze. | ||||
if tcx.is_foreign_item(def_id) { | ||||
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. After rebasing, this line was necessary to avoid |
||||
return false; | ||||
} | ||||
|
||||
// Without available MIR, polymorphization has nothing to analyze. | ||||
match tcx.hir().body_const_context(def_id.expect_local()) { | ||||
// FIXME(davidtwco): Disable polymorphization for any constant functions which, at the time | ||||
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. @oli-obk: I've had to change this to avoid polymorphizing constant functions (which was changed in #78407 prompting the most recent rebase). When changed, it results in a cycle error from one test:
And in another, an ICE:
I'll spend some time looking into these but given that they were essentially regressions in polymorphization introduced from #78407 (unfortunately, polymorphization has a habit of producing the strangest errors so any change to polymorphization really needs to be tested with polymorphization enabled), I've just disabled checking constants for now so that this PR can proceed :) 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. oh... I did touch the 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 did some digging. The cycle error with polymorphization occurs since this test was created 5 months ago, so it's not caused by my PR. Since it works otherwise, polymorphization will need duplicate the cycle prevention logic we have for wf-checks of 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. The ICE is also unrelated to my PR, but in contrast to the cycle, I know exactly how to fix it:
should be a match on 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. And since I already had a worktree open for this: #82765 |
||||
// of writing, can result in an ICE from typeck in one test and a cycle error in another. | ||||
Some(_) => false, | ||||
None if !tcx.is_mir_available(def_id) => { | ||||
debug!("should_polymorphize: (no mir available) def_id={:?}", def_id); | ||||
false | ||||
} | ||||
None => true, | ||||
} | ||||
} | ||||
|
||||
/// Some parameters are considered used-by-default, such as non-generic parameters and the dummy | ||||
/// generic parameters from closures, this function marks them as used. `leaf_is_closure` should | ||||
/// be `true` if the item that `unused_generic_params` was invoked on is a closure. | ||||
|
@@ -220,7 +254,9 @@ impl<'a, 'tcx> MarkUsedGenericParams<'a, 'tcx> { | |||
/// Invoke `unused_generic_params` on a body contained within the current item (e.g. | ||||
/// a closure, generator or constant). | ||||
fn visit_child_body(&mut self, def_id: DefId, substs: SubstsRef<'tcx>) { | ||||
let unused = self.tcx.unused_generic_params(def_id); | ||||
let unused = self | ||||
.tcx | ||||
.unused_generic_params(ty::InstanceDef::Item(ty::WithOptConstParam::unknown(def_id))); | ||||
debug!( | ||||
"visit_child_body: unused_parameters={:?} unused={:?}", | ||||
self.unused_parameters, unused | ||||
|
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.
Before rebasing,
should_polymorphize
invokeddef_id_if_not_guaranteed_local_codegen
instead of using this value. It returnedtrue
wheneverdef_id_if_not_guaranteed_local_codegen
returnedNone
, which meant that aClosureOnceShim
(particularly, the shim forFnOnce::fn_once
) wasn't being checked for available MIR, which was causing an ICE after I rebased.