Conversation
This comment has been minimized.
This comment has been minimized.
Detect cycles instead of treating every nested always-inline call as recursive. Cache results and keep the traversal on the heap. Add an -O0 assembly regression for acyclic chains. Retain the existing always_inline runtime test for self-recursion and mutual recursion.
b3a2235 to
935c4f4
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
| pub functions: RefCell<FxHashMap<String, Function<'gcc>>>, | ||
| pub intrinsics: RefCell<FxHashMap<String, Function<'gcc>>>, | ||
| #[cfg(feature = "master")] | ||
| pub inline_recursion: RefCell<FxHashMap<DefId, bool>>, |
There was a problem hiding this comment.
Please add a comment to explain why we need this field and what it is used for and what does the bool represent.
| ) { | ||
| // Keep the DFS on the heap: valid forced-inline chains can be arbitrarily | ||
| // deep, independently of the compiler thread's remaining call stack. | ||
| let mut pending: Vec<(DefId, bool)> = vec![(instance.def_id(), false)]; |
There was a problem hiding this comment.
Add a comment to explain what the bool represents in this Vec.
| } | ||
| active.insert(def); | ||
| pending.push((def, true)); | ||
| for block in cx.tcx.optimized_mir(def).basic_blocks.iter().rev() { |
There was a problem hiding this comment.
Is this loop in reverse order because calls are terminators, so at the end of basic blocks?
Add a comment to explain why this is in reverse order.
| /// Checks if the function `instance` is recursively inline. | ||
| /// Returns `false` if a functions is guaranteed to be non-recursive, and `true` if it *might* be recursive. | ||
| /// Check forced-inline call chains for cycles. Merely calling another | ||
| /// always-inline function is not recursion. |
There was a problem hiding this comment.
Explain in the doc comment what is the return value.
| // I assume that the recursive-inline issue applies only to functions, and not to drops. | ||
| // In principle, a recursive, `#[inline(always)]` drop could(?) exist, but I don't think it does. |
There was a problem hiding this comment.
Please keep this comment as it is useful.
Preserve
#[inline(always)]for nested, non-recursive calls by detecting cycles instead of treating any always-inline callee as recursion. The traversal is iterative and cached.Add an
-O0assembly regression. Existing tests cover self-recursion and mutual recursion.