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

Use DefPathHash instead of HirId to break inlining cycles. #85321

Merged
merged 3 commits into from
Apr 3, 2022
Merged
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
29 changes: 12 additions & 17 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Inlining pass for MIR functions

use rustc_attr::InlineAttr;
use rustc_hir as hir;
use rustc_index::bit_set::BitSet;
use rustc_index::vec::Idx;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
Expand Down Expand Up @@ -88,7 +87,6 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
tcx,
param_env,
codegen_fn_attrs: tcx.codegen_fn_attrs(def_id),
hir_id,
history: Vec::new(),
changed: false,
};
Expand All @@ -102,8 +100,6 @@ struct Inliner<'tcx> {
param_env: ParamEnv<'tcx>,
/// Caller codegen attributes.
codegen_fn_attrs: &'tcx CodegenFnAttrs,
/// Caller HirID.
hir_id: hir::HirId,
/// Stack of inlined Instances.
history: Vec<ty::Instance<'tcx>>,
/// Indicates that the caller body has been modified.
Expand Down Expand Up @@ -179,7 +175,9 @@ impl<'tcx> Inliner<'tcx> {
caller_body: &Body<'tcx>,
callee: &Instance<'tcx>,
) -> Result<(), &'static str> {
if callee.def_id() == caller_body.source.def_id() {
let caller_def_id = caller_body.source.def_id();
let callee_def_id = callee.def_id();
if callee_def_id == caller_def_id {
return Err("self-recursion");
}

Expand All @@ -188,7 +186,7 @@ impl<'tcx> Inliner<'tcx> {
// If there is no MIR available (either because it was not in metadata or
// because it has no MIR because it's an extern function), then the inliner
// won't cause cycles on this.
if !self.tcx.is_mir_available(callee.def_id()) {
if !self.tcx.is_mir_available(callee_def_id) {
return Err("item MIR unavailable");
}
}
Expand All @@ -208,29 +206,26 @@ impl<'tcx> Inliner<'tcx> {
| InstanceDef::CloneShim(..) => return Ok(()),
}

if self.tcx.is_constructor(callee.def_id()) {
if self.tcx.is_constructor(callee_def_id) {
trace!("constructors always have MIR");
// Constructor functions cannot cause a query cycle.
return Ok(());
}

if let Some(callee_def_id) = callee.def_id().as_local() {
let callee_hir_id = self.tcx.hir().local_def_id_to_hir_id(callee_def_id);
if callee_def_id.is_local() {
// Avoid a cycle here by only using `instance_mir` only if we have
// a lower `HirId` than the callee. This ensures that the callee will
// not inline us. This trick only works without incremental compilation.
// So don't do it if that is enabled.
if !self.tcx.dep_graph.is_fully_enabled() && self.hir_id.index() < callee_hir_id.index()
// a lower `DefPathHash` than the callee. This ensures that the callee will
// not inline us. This trick even works with incremental compilation,
// since `DefPathHash` is stable.
if self.tcx.def_path_hash(caller_def_id).local_hash()
< self.tcx.def_path_hash(callee_def_id).local_hash()
{
return Ok(());
}

// If we know for sure that the function we're calling will itself try to
// call us, then we avoid inlining that function.
if self
.tcx
.mir_callgraph_reachable((*callee, caller_body.source.def_id().expect_local()))
{
if self.tcx.mir_callgraph_reachable((*callee, caller_def_id.expect_local())) {
return Err("caller might be reachable from callee (query cycle avoidance)");
}

Expand Down