Skip to content

Commit 41454a0

Browse files
committed
Revert "Transform tail-recursive functions into recursive continuations (ocaml-flambda#893)"
This reverts commit 5e903ca.
1 parent 4dcd83d commit 41454a0

34 files changed

+68
-614
lines changed

middle_end/flambda2/from_lambda/closure_conversion.ml

+10-37
Original file line numberDiff line numberDiff line change
@@ -1093,28 +1093,8 @@ let close_one_function acc ~code_id ~external_env ~by_function_slot decl
10931093
let params = Function_decl.params decl in
10941094
let return = Function_decl.return decl in
10951095
let return_continuation = Function_decl.return_continuation decl in
1096-
let acc, exn_continuation =
1097-
close_exn_continuation acc external_env
1098-
(Function_decl.exn_continuation decl)
1099-
in
1100-
assert (
1101-
match Exn_continuation.extra_args exn_continuation with
1102-
| [] -> true
1103-
| _ :: _ -> false);
1104-
let my_closure = Variable.create "my_closure" in
11051096
let recursive = Function_decl.recursive decl in
1106-
(* Mark function available for loopify only if it is a single recursive
1107-
function *)
1108-
let is_single_recursive_function =
1109-
match recursive, Function_decls.to_list function_declarations with
1110-
| Recursive, [_] -> true
1111-
| Recursive, ([] | _ :: _ :: _) -> false
1112-
| Non_recursive, _ -> false
1113-
in
1114-
let acc =
1115-
Acc.push_closure_info acc ~return_continuation ~exn_continuation ~my_closure
1116-
~is_purely_tailrec:is_single_recursive_function
1117-
in
1097+
let my_closure = Variable.create "my_closure" in
11181098
let my_region = Function_decl.my_region decl in
11191099
let function_slot = Function_decl.function_slot decl in
11201100
let my_depth = Variable.create "my_depth" in
@@ -1300,6 +1280,14 @@ let close_one_function acc ~code_id ~external_env ~by_function_slot decl
13001280
Let_with_acc.create acc bound (Named.create_rec_info next_depth_expr) ~body
13011281
in
13021282
let cost_metrics = Acc.cost_metrics acc in
1283+
let acc, exn_continuation =
1284+
close_exn_continuation acc external_env
1285+
(Function_decl.exn_continuation decl)
1286+
in
1287+
assert (
1288+
match Exn_continuation.extra_args exn_continuation with
1289+
| [] -> true
1290+
| _ :: _ -> false);
13031291
let inline : Inline_attribute.t =
13041292
(* We make a decision based on [fallback_inlining_heuristic] here to try to
13051293
mimic Closure's behaviour as closely as possible, particularly when there
@@ -1333,7 +1321,6 @@ let close_one_function acc ~code_id ~external_env ~by_function_slot decl
13331321
|> Acc.remove_continuation_from_free_names
13341322
(Exn_continuation.exn_handler exn_continuation)
13351323
in
1336-
let closure_info, acc = Acc.pop_closure_info acc in
13371324
let params_arity = Bound_parameters.arity_with_subkinds params in
13381325
let is_tupled =
13391326
match Function_decl.kind decl with Curried _ -> false | Tupled -> true
@@ -1345,15 +1332,6 @@ let close_one_function acc ~code_id ~external_env ~by_function_slot decl
13451332
then Function_decl_inlining_decision_type.Stub
13461333
else Function_decl_inlining_decision_type.Not_yet_decided
13471334
in
1348-
let loopify : Loopify_attribute.t =
1349-
match Function_decl.loop decl with
1350-
| Always_loop -> Always_loopify
1351-
| Never_loop -> Never_loopify
1352-
| Default_loop ->
1353-
if closure_info.is_purely_tailrec
1354-
then Default_loopify_and_tailrec
1355-
else Default_loopify_and_not_tailrec
1356-
in
13571335
let code =
13581336
Code.create code_id ~params_and_body
13591337
~free_names_of_params_and_body:(Acc.free_names acc) ~params_arity
@@ -1373,7 +1351,7 @@ let close_one_function acc ~code_id ~external_env ~by_function_slot decl
13731351
~dbg ~is_tupled
13741352
~is_my_closure_used:
13751353
(Function_params_and_body.is_my_closure_used params_and_body)
1376-
~inlining_decision ~absolute_history ~relative_history ~loopify
1354+
~inlining_decision ~absolute_history ~relative_history
13771355
in
13781356
let approx =
13791357
let code = Code_or_metadata.create code in
@@ -1502,7 +1480,6 @@ let close_functions acc external_env ~current_region function_declarations =
15021480
~inlining_decision:Recursive
15031481
~absolute_history:(Inlining_history.Absolute.empty compilation_unit)
15041482
~relative_history:Inlining_history.Relative.empty
1505-
~loopify:Never_loopify
15061483
in
15071484
let code = Code_or_metadata.create_metadata_only metadata in
15081485
let approx =
@@ -1763,7 +1740,6 @@ let wrap_partial_application acc env apply_continuation (apply : IR.apply)
17631740
specialise = Default_specialise;
17641741
local = Default_local;
17651742
check = Default_check;
1766-
loop = Default_loop;
17671743
is_a_functor = false;
17681744
stub = false;
17691745
poll = Default_poll
@@ -2239,9 +2215,6 @@ let close_program (type mode) ~(mode : mode Flambda_features.mode)
22392215
defining_expr ~body)
22402216
(acc, body) (Acc.declared_symbols acc)
22412217
in
2242-
if Option.is_some (Acc.top_closure_info acc)
2243-
then
2244-
Misc.fatal_error "Information on nested closures should be empty at the end";
22452218
let get_code_metadata code_id =
22462219
Code_id.Map.find code_id (Acc.code acc) |> Code.code_metadata
22472220
in

middle_end/flambda2/from_lambda/closure_conversion_aux.ml

+8-129
Original file line numberDiff line numberDiff line change
@@ -375,13 +375,6 @@ module Acc = struct
375375
| Trackable_arguments of Env.value_approximation list
376376
| Untrackable
377377

378-
type closure_info =
379-
{ return_continuation : Continuation.t;
380-
exn_continuation : Exn_continuation.t;
381-
my_closure : Variable.t;
382-
is_purely_tailrec : bool
383-
}
384-
385378
type t =
386379
{ declared_symbols : (Symbol.t * Static_const.t) list;
387380
lifted_sets_of_closures :
@@ -396,8 +389,7 @@ module Acc = struct
396389
seen_a_function : bool;
397390
symbol_for_global : Ident.t -> Symbol.t;
398391
slot_offsets : Slot_offsets.t;
399-
regions_closed_early : Ident.Set.t;
400-
closure_infos : closure_info list
392+
regions_closed_early : Ident.Set.t
401393
}
402394

403395
let cost_metrics t = t.cost_metrics
@@ -422,8 +414,7 @@ module Acc = struct
422414
seen_a_function = false;
423415
symbol_for_global;
424416
slot_offsets;
425-
regions_closed_early = Ident.Set.empty;
426-
closure_infos = []
417+
regions_closed_early = Ident.Set.empty
427418
}
428419

429420
let declared_symbols t = t.declared_symbols
@@ -460,47 +451,15 @@ module Acc = struct
460451
let add_free_names free_names t =
461452
{ t with free_names = Name_occurrences.union free_names t.free_names }
462453

463-
let add_free_names_and_check_my_closure_use free_names t =
464-
let t =
465-
match t.closure_infos with
466-
| [] -> t
467-
| closure_info :: closure_infos ->
468-
if closure_info.is_purely_tailrec
469-
&& Name_occurrences.mem_var free_names closure_info.my_closure
470-
then
471-
{ t with
472-
closure_infos =
473-
{ closure_info with is_purely_tailrec = false } :: closure_infos
474-
}
475-
else t
476-
in
477-
add_free_names free_names t
478-
479-
let add_name_to_free_names ~is_tail_call ~name t =
480-
let closure_infos =
481-
match is_tail_call, t.closure_infos with
482-
| true, closure_infos -> closure_infos
483-
| false, [] -> []
484-
| false, closure_info :: closure_infos ->
485-
if closure_info.is_purely_tailrec
486-
&& Name.equal (Name.var closure_info.my_closure) name
487-
then { closure_info with is_purely_tailrec = false } :: closure_infos
488-
else t.closure_infos
489-
in
454+
let add_name_to_free_names ~name t =
490455
{ t with
491-
closure_infos;
492456
free_names = Name_occurrences.add_name t.free_names name Name_mode.normal
493457
}
494458

495-
let add_simple_to_free_names_maybe_tail_call ~is_tail_call acc simple =
459+
let add_simple_to_free_names acc simple =
496460
Simple.pattern_match simple
497461
~const:(fun _ -> acc)
498-
~name:(fun name ~coercion ->
499-
let acc = add_name_to_free_names ~is_tail_call ~name acc in
500-
add_free_names (Coercion.free_names coercion) acc)
501-
502-
let add_simple_to_free_names acc simple =
503-
add_simple_to_free_names_maybe_tail_call ~is_tail_call:false acc simple
462+
~name:(fun name ~coercion:_ -> add_name_to_free_names ~name acc)
504463

505464
let remove_code_id_or_symbol_from_free_names code_id_or_symbol t =
506465
{ t with
@@ -579,36 +538,6 @@ module Acc = struct
579538
set_of_closures
580539
in
581540
{ t with slot_offsets }
582-
583-
let top_closure_info t =
584-
match t.closure_infos with
585-
| [] -> None
586-
| closure_info :: _ -> Some closure_info
587-
588-
let push_closure_info t ~return_continuation ~exn_continuation ~my_closure
589-
~is_purely_tailrec =
590-
{ t with
591-
closure_infos =
592-
{ return_continuation; exn_continuation; my_closure; is_purely_tailrec }
593-
:: t.closure_infos
594-
}
595-
596-
let pop_closure_info t =
597-
let closure_info, closure_infos =
598-
match t.closure_infos with
599-
| [] -> Misc.fatal_error "pop_closure_info called on empty stack"
600-
| closure_info :: closure_infos -> closure_info, closure_infos
601-
in
602-
let closure_infos =
603-
match closure_infos with
604-
| [] -> []
605-
| closure_info2 :: closure_infos2 ->
606-
if closure_info2.is_purely_tailrec
607-
&& Name_occurrences.mem_var t.free_names closure_info2.my_closure
608-
then { closure_info2 with is_purely_tailrec = false } :: closure_infos2
609-
else closure_infos
610-
in
611-
closure_info, { t with closure_infos }
612541
end
613542

614543
module Function_decls = struct
@@ -687,8 +616,6 @@ module Function_decls = struct
687616

688617
let poll_attribute t = t.attr.poll
689618

690-
let loop t = t.attr.loop
691-
692619
let is_a_functor t = t.attr.is_a_functor
693620

694621
let check_attribute t = t.attr.check
@@ -782,40 +709,7 @@ module Expr_with_acc = struct
782709
(Code_size.apply apply |> Cost_metrics.from_size)
783710
acc
784711
in
785-
let is_tail_call =
786-
match Acc.top_closure_info acc with
787-
| None -> false
788-
| Some { return_continuation; exn_continuation; _ } -> (
789-
(match Apply_expr.continuation apply with
790-
| Never_returns -> true
791-
| Return cont -> Continuation.equal cont return_continuation)
792-
&& Exn_continuation.equal
793-
(Apply_expr.exn_continuation apply)
794-
exn_continuation
795-
(* If the return and exn continuation match, the call is in tail
796-
position, but could still be an under- or over-application. By
797-
checking that it is a direct call, we are sure it has the correct
798-
arity. *)
799-
&&
800-
match Apply.call_kind apply with
801-
| Function { function_call = Direct _; _ } -> true
802-
| Function
803-
{ function_call = Indirect_unknown_arity | Indirect_known_arity _;
804-
_
805-
} ->
806-
false
807-
| Method _ -> false
808-
| C_call _ -> false)
809-
in
810-
let acc =
811-
Acc.add_simple_to_free_names_maybe_tail_call ~is_tail_call acc
812-
(Apply.callee apply)
813-
in
814-
let acc =
815-
Acc.add_free_names_and_check_my_closure_use
816-
(Apply_expr.free_names_except_callee apply)
817-
acc
818-
in
712+
let acc = Acc.add_free_names (Apply_expr.free_names apply) acc in
819713
let acc =
820714
match Apply_expr.continuation apply with
821715
| Never_returns -> acc
@@ -848,11 +742,7 @@ module Apply_cont_with_acc = struct
848742
let create acc ?trap_action ?args_approx cont ~args ~dbg =
849743
let apply_cont = Apply_cont.create ?trap_action cont ~args ~dbg in
850744
let acc = Acc.add_continuation_application ~cont args_approx acc in
851-
let acc =
852-
Acc.add_free_names_and_check_my_closure_use
853-
(Apply_cont.free_names apply_cont)
854-
acc
855-
in
745+
let acc = Acc.add_free_names (Apply_cont.free_names apply_cont) acc in
856746
acc, apply_cont
857747

858748
let goto acc cont =
@@ -907,18 +797,7 @@ module Let_with_acc = struct
907797
~code_id:(fun acc cid -> Acc.remove_code_id_from_free_names cid acc)
908798
in
909799
let let_expr = Let.create let_bound named ~body ~free_names_of_body in
910-
let is_project_value_slot =
911-
match[@ocaml.warning "-4"] (named : Named.t) with
912-
| Prim (Unary (Project_value_slot _, _), _) -> true
913-
| _ -> false
914-
in
915-
let acc =
916-
if is_project_value_slot
917-
then Acc.add_free_names (Named.free_names named) acc
918-
else
919-
Acc.add_free_names_and_check_my_closure_use (Named.free_names named)
920-
acc
921-
in
800+
let acc = Acc.add_free_names (Named.free_names named) acc in
922801
acc, Expr.create_let let_expr
923802
end
924803

middle_end/flambda2/from_lambda/closure_conversion_aux.mli

-21
Original file line numberDiff line numberDiff line change
@@ -179,13 +179,6 @@ end
179179

180180
(** Used to pipe some data through closure conversion *)
181181
module Acc : sig
182-
type closure_info = private
183-
{ return_continuation : Continuation.t;
184-
exn_continuation : Exn_continuation.t;
185-
my_closure : Variable.t;
186-
is_purely_tailrec : bool
187-
}
188-
189182
type t
190183

191184
val create :
@@ -259,18 +252,6 @@ module Acc : sig
259252

260253
val add_set_of_closures_offsets :
261254
is_phantom:bool -> t -> Set_of_closures.t -> t
262-
263-
val top_closure_info : t -> closure_info option
264-
265-
val push_closure_info :
266-
t ->
267-
return_continuation:Continuation.t ->
268-
exn_continuation:Exn_continuation.t ->
269-
my_closure:Variable.t ->
270-
is_purely_tailrec:bool ->
271-
t
272-
273-
val pop_closure_info : t -> closure_info * t
274255
end
275256

276257
(** Used to represent information about a set of function declarations during
@@ -324,8 +305,6 @@ module Function_decls : sig
324305

325306
val poll_attribute : t -> Lambda.poll_attribute
326307

327-
val loop : t -> Lambda.loop_attribute
328-
329308
val is_a_functor : t -> bool
330309

331310
val check_attribute : t -> Lambda.check_attribute

middle_end/flambda2/parser/fexpr_to_flambda.ml

-2
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,6 @@ let rec expr env (e : Fexpr.expr) : Flambda.Expr.t =
778778
in
779779
let code =
780780
(* CR mshinwell: [inlining_decision] should maybe be set properly *)
781-
(* CR ncourant: same for loopify *)
782781
Code.create code_id ~params_and_body ~free_names_of_params_and_body
783782
~newer_version_of ~params_arity ~num_trailing_local_params:0
784783
~result_arity ~result_types:Unknown
@@ -794,7 +793,6 @@ let rec expr env (e : Fexpr.expr) : Flambda.Expr.t =
794793
(Inlining_history.Absolute.empty
795794
(Compilation_unit.get_current_exn ()))
796795
~relative_history:Inlining_history.Relative.empty
797-
~loopify:Never_loopify
798796
in
799797
Flambda.Static_const_or_code.create_code code
800798
in

0 commit comments

Comments
 (0)