Skip to content

Commit

Permalink
Revert "Classic mode: partially lift the restrictions on inlining fun…
Browse files Browse the repository at this point in the history
…ctions containing closures" (ocaml-flambda#2546)

Revert "Classic mode: partially lift the restrictions on inlining functions c…"

This reverts commit 2384937.
  • Loading branch information
mshinwell authored May 7, 2024
1 parent 23b1314 commit ad876d2
Showing 1 changed file with 14 additions and 19 deletions.
33 changes: 14 additions & 19 deletions middle_end/flambda2/from_lambda/closure_conversion.ml
Original file line number Diff line number Diff line change
Expand Up @@ -2136,22 +2136,19 @@ 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 =
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
(* 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)
in
let free_names_of_body = Acc.free_names acc in
let params_and_body =
Expand Down Expand Up @@ -2259,9 +2256,7 @@ 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 =
if has_lifted_closure then acc else Acc.with_seen_a_function acc true
in
let acc = Acc.with_seen_a_function acc true in
( acc,
( Function_slot.Map.add function_slot approx by_function_slot,
function_code_ids ) )
Expand Down

0 comments on commit ad876d2

Please sign in to comment.