Skip to content

Comments

multi-block inliner#200

Open
sbillig wants to merge 5 commits intofe-lang:mainfrom
sbillig:inliner2
Open

multi-block inliner#200
sbillig wants to merge 5 commits intofe-lang:mainfrom
sbillig:inliner2

Conversation

@sbillig
Copy link
Collaborator

@sbillig sbillig commented Feb 23, 2026

No description provided.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b1bd3de969

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

let analysis = module_analysis::analyze_module(module);
let mut inlinee_summaries: FxHashMap<FuncRef, cost::InlineeSummary> =
FxHashMap::default();
let mut growth_by_caller: FxHashMap<FuncRef, usize> = FxHashMap::default();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep caller growth budget across fixed-point iterations

growth_by_caller is reinitialized inside the outer while loop, so max_growth_per_caller is enforced only within a single sweep of callsites. If one iteration inlines a callee that exposes additional calls in the same caller, the next iteration starts that caller back at zero and can inline past the configured per-caller cap, which defeats the intended code-growth guardrail for multi-iteration inlining.

Useful? React with 👍 / 👎.

return InlineDecision::Skip(InlineSkipReason::NoBody);
}

let predicted_growth = summary.insts.saturating_sub(1);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include synthesized merge phi in growth prediction

The budget gate uses predicted_growth = summary.insts - 1, but full inlining can insert an extra merge phi when a multi-return callee's result is used (full.rs increments inserted_insts for that case). This makes the pre-inline budget check optimistic, so max_growth_per_caller and max_total_growth can be exceeded by one per such inline even though the decision path said the site was within budget.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant