Skip to content

Commit

Permalink
Rollup merge of #100522 - cjgillot:inline-polymorphic-recursion, r=tm…
Browse files Browse the repository at this point in the history
…iasko

Only check the `DefId` for the recursion check in MIR inliner.

The current history check compares `Instance`s, so it cannot detect cases of polymorphic recursion where `Substs` change.
This PR makes it so we only compare `DefId`s, ignoring any change in `Substs`.

According to #100522 (comment), in practice only very few inlining decisions change.

Fixes #100476
  • Loading branch information
Dylan-DPC committed Aug 19, 2022
2 parents 3cebcba + 86645c9 commit d83abe8
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
13 changes: 9 additions & 4 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_middle::mir::*;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_session::config::OptLevel;
use rustc_span::def_id::DefId;
use rustc_span::{hygiene::ExpnKind, ExpnData, LocalExpnId, Span};
use rustc_target::spec::abi::Abi;

Expand Down Expand Up @@ -103,8 +104,12 @@ struct Inliner<'tcx> {
param_env: ParamEnv<'tcx>,
/// Caller codegen attributes.
codegen_fn_attrs: &'tcx CodegenFnAttrs,
/// Stack of inlined Instances.
history: Vec<ty::Instance<'tcx>>,
/// Stack of inlined instances.
/// We only check the `DefId` and not the substs because we want to
/// avoid inlining cases of polymorphic recursion.
/// The number of `DefId`s is finite, so checking history is enough
/// to ensure that we do not loop endlessly while inlining.
history: Vec<DefId>,
/// Indicates that the caller body has been modified.
changed: bool,
}
Expand Down Expand Up @@ -132,7 +137,7 @@ impl<'tcx> Inliner<'tcx> {
Ok(new_blocks) => {
debug!("inlined {}", callsite.callee);
self.changed = true;
self.history.push(callsite.callee);
self.history.push(callsite.callee.def_id());
self.process_blocks(caller_body, new_blocks);
self.history.pop();
}
Expand Down Expand Up @@ -308,7 +313,7 @@ impl<'tcx> Inliner<'tcx> {
return None;
}

if self.history.contains(&callee) {
if self.history.contains(&callee.def_id()) {
return None;
}

Expand Down
25 changes: 25 additions & 0 deletions src/test/mir-opt/inline/polymorphic-recursion.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Make sure that the MIR inliner does not loop indefinitely on polymorphic recursion.
// compile-flags: --crate-type lib

// Randomize `def_path_hash` by defining them under a module with different names
macro_rules! emit {
($($m:ident)*) => {$(
pub mod $m {
pub trait Tr { type Next: Tr; }

pub fn hoge<const N: usize, T: Tr>() {
inner::<N, T>();
}

#[inline(always)]
fn inner<const N: usize, T: Tr>()
{
inner::<N, T::Next>();
inner::<N, T::Next>();
}
}
)*};
}

// Increase the chance of triggering the bug
emit!(m00 m01 m02 m03 m04 m05 m06 m07 m08 m09 m10 m11 m12 m13 m14 m15 m16 m17 m18 m19);

0 comments on commit d83abe8

Please sign in to comment.