From e1b36f5ae24fcae7f987acdc1fe20044451b2e3c Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 13 May 2021 09:28:56 +0200 Subject: [PATCH 1/3] Use DefPathHash instead of HirId to break cycles. --- compiler/rustc_mir_transform/src/inline.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 1b429731d706b..1f8f1fa837167 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -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}; @@ -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, }; @@ -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>, /// Indicates that the caller body has been modified. @@ -179,7 +175,8 @@ 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(); + if callee.def_id() == caller_def_id { return Err("self-recursion"); } @@ -215,22 +212,19 @@ impl<'tcx> Inliner<'tcx> { } 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); // 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) + < self.tcx.def_path_hash(callee_def_id.to_def_id()) { 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)"); } From 2d3d9b26a424dc48b49a17c39159a3caaea9c3bf Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 17 May 2021 21:07:42 +0200 Subject: [PATCH 2/3] Use only local hash. --- compiler/rustc_mir_transform/src/inline.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 1f8f1fa837167..22e16f9231a9c 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -216,8 +216,8 @@ impl<'tcx> Inliner<'tcx> { // 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) - < self.tcx.def_path_hash(callee_def_id.to_def_id()) + if self.tcx.def_path_hash(caller_def_id).local_hash() + < self.tcx.def_path_hash(callee_def_id.to_def_id()).local_hash() { return Ok(()); } From 297dde9b1ad0c28922fac5046f77c2629cebf662 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 2 Apr 2022 23:28:09 +0200 Subject: [PATCH 3/3] Less manipulation of the callee_def_id. --- compiler/rustc_mir_transform/src/inline.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 22e16f9231a9c..2f0673b9a76b2 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -176,7 +176,8 @@ impl<'tcx> Inliner<'tcx> { callee: &Instance<'tcx>, ) -> Result<(), &'static str> { let caller_def_id = caller_body.source.def_id(); - if callee.def_id() == caller_def_id { + let callee_def_id = callee.def_id(); + if callee_def_id == caller_def_id { return Err("self-recursion"); } @@ -185,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"); } } @@ -205,19 +206,19 @@ 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() { + if callee_def_id.is_local() { // Avoid a cycle here by only using `instance_mir` only if we have // 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.to_def_id()).local_hash() + < self.tcx.def_path_hash(callee_def_id).local_hash() { return Ok(()); }