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.

Sign up for GitHub

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

Re-roll "Classic mode: partially lift the restrictions on inlining functions containing closures" #2556

Merged
merged 1 commit into from
May 10, 2024
Merged

Re-roll "Classic mode: partially lift the restrictions on inlining functions containing closures" #2556

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 19 additions & 14 deletions middle_end/flambda2/from_lambda/closure_conversion.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2145,19 +2145,22 @@ let close_one_function acc ~code_id ~external_env ~by_function_slot
let contains_subfunctions = Acc.seen_a_function acc in
let cost_metrics = Acc.cost_metrics acc in
let inline : Inline_attribute.t =
(* We make a decision based on [fallback_inlining_heuristic] here to try to
mimic Closure's behaviour as closely as possible, particularly when there
are functions involving constant closures, which are not lifted during
Closure (but will prevent inlining) but will likely have been lifted by
our other check in [Inlining_cost] (thus preventing us seeing they were
originally there). Note that while Closure never marks as inlinable
functions in a set a recursive definitions with more than one function,
we do not try to reproduce this particular property and can mark as
inlinable such functions. *)
if contains_subfunctions
&& Flambda_features.Expert.fallback_inlining_heuristic ()
then Never_inline
else Inline_attribute.from_lambda (Function_decl.inline decl)
match Inline_attribute.from_lambda (Function_decl.inline decl) with
| (Always_inline | Available_inline | Never_inline) as attr -> attr
| (Unroll _ | Default_inline) as attr ->
(* We make a decision based on [fallback_inlining_heuristic] here to try
to mimic Closure's behaviour as closely as possible, particularly when
there are functions involving constant closures, which are not lifted
during Closure (but will prevent inlining) but will likely have been
lifted by our other check in [Inlining_cost] (thus preventing us seeing
they were originally there). Note that while Closure never marks as
inlinable functions in a set a recursive definitions with more than one
function, we do not try to reproduce this particular property and can
mark as inlinable such functions. *)
if contains_subfunctions
&& Flambda_features.Expert.fallback_inlining_heuristic ()
then (* CR vlaviron: Store reason *) Never_inline
else attr
in
let free_names_of_body = Acc.free_names acc in
let params_and_body =
Expand Down Expand Up @@ -2263,7 +2266,9 @@ let close_one_function acc ~code_id ~external_env ~by_function_slot
else meta
in
let acc = Acc.add_code ~code_id ~code acc in
let acc = Acc.with_seen_a_function acc true in
let acc =
if has_lifted_closure then acc else Acc.with_seen_a_function acc true
in
( acc,
( Function_slot.Map.add function_slot approx by_function_slot,
function_code_ids ) )
Expand Down
Loading