Skip to content

Commit c74e3ba

Browse files
authored
Review changes of term directory (#602)
1 parent 0f986ed commit c74e3ba

26 files changed

+174
-152
lines changed

middle_end/flambda2/algorithms/container_types.ml

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,17 @@ module Make_map (T : Thing) (Set : Set_plus_stdlib with type elt = T.t) = struct
161161

162162
let replace _ _ _ : _ t = Misc.fatal_error "Not yet implemented"
163163

164-
let map_sharing = map
164+
let map_sharing f t =
165+
let changed = ref false in
166+
let t' =
167+
map
168+
(fun v ->
169+
let v' = f v in
170+
if not (v == v') then changed := true;
171+
v')
172+
t
173+
in
174+
if not !changed then t else t'
165175
end
166176
[@@inline always]
167177

middle_end/flambda2/algorithms/lmap.ml

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -120,16 +120,8 @@ module Make (T : Thing) : S with type key = T.t = struct
120120

121121
let mapi f m = List.map (fun (k, v) -> k, f k v) m
122122

123-
let rec map_sharing f l0 =
124-
match l0 with
125-
| a :: l ->
126-
let a' = f a in
127-
let l' = map_sharing f l in
128-
if a' == a && l' == l then l0 else a' :: l'
129-
| [] -> []
130-
131123
let map_sharing f m =
132-
map_sharing
124+
Misc.Stdlib.List.map_sharing
133125
(fun ((k, v) as pair) ->
134126
let v' = f v in
135127
if v' == v then pair else k, v')

middle_end/flambda2/bound_identifiers/bound_parameters.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ let free_names t =
7272
(fun result param -> Name_occurrences.union result (BP.free_names param))
7373
Name_occurrences.empty t
7474

75-
let apply_renaming t perm =
76-
List.map (fun param -> BP.apply_renaming param perm) t
75+
let apply_renaming t renaming =
76+
Misc.Stdlib.List.map_sharing (fun param -> BP.apply_renaming param renaming) t
7777

7878
let all_ids_for_export t =
7979
Ids_for_export.union_list (List.map BP.all_ids_for_export t)

middle_end/flambda2/simplify/expr_builder.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -863,7 +863,7 @@ let add_wrapper_for_fixed_arity_continuation0 uacc cont_or_apply_cont ~use_id
863863
in
864864
new_wrapper params expr ~free_names ~cost_metrics)
865865
| Apply_cont apply_cont -> (
866-
let apply_cont = Apply_cont.update_continuation apply_cont cont in
866+
let apply_cont = Apply_cont.with_continuation apply_cont cont in
867867
match rewrite_use uacc rewrite ~ctx:Apply_cont use_id apply_cont with
868868
| Apply_cont apply_cont -> Apply_cont apply_cont
869869
| Expr build_expr ->
@@ -890,7 +890,7 @@ let add_wrapper_for_switch_arm uacc apply_cont ~use_id arity :
890890
~use_id arity
891891
with
892892
| This_continuation cont ->
893-
Apply_cont (Apply_cont.update_continuation apply_cont cont)
893+
Apply_cont (Apply_cont.with_continuation apply_cont cont)
894894
| Apply_cont apply_cont -> Apply_cont apply_cont
895895
| New_wrapper (cont, wrapper, free_names_of_handler, cost_metrics) ->
896896
New_wrapper (cont, wrapper, free_names_of_handler, cost_metrics)

middle_end/flambda2/simplify/simplify_apply_cont_expr.ml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ let rebuild_apply_cont apply_cont ~args ~rewrite_id uacc ~after_rebuild =
7979
inlined continuation -- before it is wrapped in any [Let]-expressions
8080
needed as a result of the rewrite. *)
8181
let rewrite_use_result =
82-
let apply_cont = AC.update_continuation_and_args apply_cont cont ~args in
82+
let apply_cont = AC.with_continuation_and_args apply_cont cont ~args in
8383
let apply_cont =
8484
Simplify_common.clear_demoted_trap_action_and_patch_unused_exn_bucket
8585
uacc apply_cont

middle_end/flambda2/terms/or_variable.ml renamed to middle_end/flambda2/term_basics/or_variable.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,11 +37,11 @@ let free_names t =
3737
| Const _ -> Name_occurrences.empty
3838
| Var (var, _dbg) -> Name_occurrences.singleton_variable var Name_mode.normal
3939

40-
let apply_renaming t perm =
40+
let apply_renaming t renaming =
4141
match t with
4242
| Const _ -> t
4343
| Var (var, dbg) ->
44-
let var' = Renaming.apply_variable perm var in
44+
let var' = Renaming.apply_variable renaming var in
4545
if var == var' then t else Var (var', dbg)
4646

4747
let value_map t ~default ~f =

middle_end/flambda2/terms/apply_cont_expr.ml

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,10 @@ let free_names { k; args; trap_action; dbg = _ } =
125125
(Name_occurrences.union default (Trap_action.free_names trap_action))
126126
k ~has_traps:true
127127

128-
let apply_renaming ({ k; args; trap_action; dbg } as t) perm =
129-
let k' = Renaming.apply_continuation perm k in
130-
let args' = Simple.List.apply_renaming args perm in
131-
let trap_action' = Trap_action.Option.apply_renaming trap_action perm in
128+
let apply_renaming ({ k; args; trap_action; dbg } as t) renaming =
129+
let k' = Renaming.apply_continuation renaming k in
130+
let args' = Simple.List.apply_renaming args renaming in
131+
let trap_action' = Trap_action.Option.apply_renaming trap_action renaming in
132132
if k == k' && args == args' && trap_action == trap_action'
133133
then t
134134
else { k = k'; args = args'; trap_action = trap_action'; dbg }
@@ -141,9 +141,9 @@ let all_ids_for_export { k; args; trap_action; dbg = _ } =
141141
k)
142142
args
143143

144-
let update_continuation t continuation = { t with k = continuation }
144+
let with_continuation t continuation = { t with k = continuation }
145145

146-
let update_continuation_and_args t cont ~args =
146+
let with_continuation_and_args t cont ~args =
147147
if Continuation.equal t.k cont && args == t.args
148148
then t
149149
else { t with k = cont; args }
@@ -156,13 +156,6 @@ let no_args t = match args t with [] -> true | _ :: _ -> false
156156

157157
let is_goto t = no_args t && Option.is_none (trap_action t)
158158

159-
let is_goto_to t k = Continuation.equal (continuation t) k && is_goto t
160-
161-
let to_goto t =
162-
if no_args t && Option.is_none (trap_action t)
163-
then Some (continuation t)
164-
else None
165-
166159
let is_raise t =
167160
match t.trap_action with
168161
| Some (Pop { exn_handler; _ }) -> Continuation.equal t.k exn_handler

middle_end/flambda2/terms/apply_cont_expr.mli

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,9 @@ val trap_action : t -> Trap_action.t option
4242

4343
val debuginfo : t -> Debuginfo.t
4444

45-
(* CR mshinwell: Use "with" not "update" *)
46-
val update_continuation : t -> Continuation.t -> t
45+
val with_continuation : t -> Continuation.t -> t
4746

48-
val update_continuation_and_args :
49-
t -> Continuation.t -> args:Simple.t list -> t
47+
val with_continuation_and_args : t -> Continuation.t -> args:Simple.t list -> t
5048

5149
val update_args : t -> args:Simple.t list -> t
5250

@@ -56,10 +54,6 @@ val is_raise : t -> bool
5654

5755
val is_goto : t -> bool
5856

59-
val is_goto_to : t -> Continuation.t -> bool
60-
61-
val to_goto : t -> Continuation.t option
62-
6357
val clear_trap_action : t -> t
6458

6559
val to_one_arg_without_trap_action : t -> Simple.t option

middle_end/flambda2/terms/apply_expr.ml

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ module Result_continuation = struct
4646
| Return k -> Name_occurrences.singleton_continuation k
4747
| Never_returns -> Name_occurrences.empty
4848

49-
let apply_renaming t perm =
49+
let apply_renaming t renaming =
5050
match t with
51-
| Return k -> Return (Renaming.apply_continuation perm k)
51+
| Return k -> Return (Renaming.apply_continuation renaming k)
5252
| Never_returns -> Never_returns
5353

5454
let all_ids_for_export t =
@@ -207,14 +207,16 @@ let apply_renaming
207207
inlining_state;
208208
probe_name;
209209
relative_history
210-
} as t) perm =
211-
let continuation' = Result_continuation.apply_renaming continuation perm in
210+
} as t) renaming =
211+
let continuation' =
212+
Result_continuation.apply_renaming continuation renaming
213+
in
212214
let exn_continuation' =
213-
Exn_continuation.apply_renaming exn_continuation perm
215+
Exn_continuation.apply_renaming exn_continuation renaming
214216
in
215-
let callee' = Simple.apply_renaming callee perm in
216-
let args' = Simple.List.apply_renaming args perm in
217-
let call_kind' = Call_kind.apply_renaming call_kind perm in
217+
let callee' = Simple.apply_renaming callee renaming in
218+
let args' = Simple.List.apply_renaming args renaming in
219+
let call_kind' = Call_kind.apply_renaming call_kind renaming in
218220
if continuation == continuation'
219221
&& exn_continuation == exn_continuation'
220222
&& callee == callee' && args == args' && call_kind == call_kind'

middle_end/flambda2/terms/call_kind.ml

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ let free_names t =
156156
| Function
157157
{ function_call = Direct { code_id; return_arity = _ }; alloc_mode = _ }
158158
->
159-
Name_occurrences.add_code_id Name_occurrences.empty code_id Name_mode.normal
159+
Name_occurrences.singleton_code_id code_id Name_mode.normal
160160
| Function { function_call = Indirect_unknown_arity; alloc_mode = _ }
161161
| Function
162162
{ function_call =
@@ -165,16 +165,12 @@ let free_names t =
165165
}
166166
| C_call { alloc = _; param_arity = _; return_arity = _; is_c_builtin = _ } ->
167167
Name_occurrences.empty
168-
| Method { kind = _; obj; alloc_mode = _ } ->
169-
Simple.pattern_match obj
170-
~name:(fun obj ~coercion:_ ->
171-
Name_occurrences.singleton_name obj Name_mode.normal)
172-
~const:(fun _ -> Name_occurrences.empty)
168+
| Method { kind = _; obj; alloc_mode = _ } -> Simple.free_names obj
173169

174-
let apply_renaming t perm =
170+
let apply_renaming t renaming =
175171
match t with
176172
| Function { function_call = Direct { code_id; return_arity }; alloc_mode } ->
177-
let code_id' = Renaming.apply_code_id perm code_id in
173+
let code_id' = Renaming.apply_code_id renaming code_id in
178174
if code_id == code_id'
179175
then t
180176
else
@@ -191,7 +187,7 @@ let apply_renaming t perm =
191187
| C_call { alloc = _; param_arity = _; return_arity = _; is_c_builtin = _ } ->
192188
t
193189
| Method { kind; obj; alloc_mode } ->
194-
let obj' = Simple.apply_renaming obj perm in
190+
let obj' = Simple.apply_renaming obj renaming in
195191
if obj == obj' then t else Method { kind; obj = obj'; alloc_mode }
196192

197193
let all_ids_for_export t =

middle_end/flambda2/terms/code0.ml

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -86,17 +86,6 @@ let create ~print_function_params_and_body code_id ~params_and_body
8686
~contains_no_escaping_local_allocs ~stub ~(inline : Inline_attribute.t)
8787
~is_a_functor ~recursive ~cost_metrics ~inlining_arguments ~dbg ~is_tupled
8888
~is_my_closure_used ~inlining_decision ~absolute_history ~relative_history =
89-
begin
90-
match stub, inline with
91-
| true, (Available_inline | Never_inline | Default_inline)
92-
| ( false,
93-
( Never_inline | Default_inline | Always_inline | Available_inline
94-
| Unroll _ ) ) ->
95-
()
96-
| true, (Always_inline | Unroll _) ->
97-
Misc.fatal_error
98-
"Stubs may not be annotated as [Always_inline] or [Unroll]"
99-
end;
10089
check_free_names_of_params_and_body ~print_function_params_and_body code_id
10190
~params_and_body ~free_names_of_params_and_body;
10291
let code_metadata =

middle_end/flambda2/terms/code_metadata.ml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -292,17 +292,17 @@ let apply_renaming
292292
inlining_decision = _;
293293
absolute_history = _;
294294
relative_history = _
295-
} as t) perm =
295+
} as t) renaming =
296296
(* inlined and modified version of Option.map to preserve sharing *)
297297
let newer_version_of' =
298298
match newer_version_of with
299299
| None -> newer_version_of
300300
| Some code_id ->
301-
let code_id' = Renaming.apply_code_id perm code_id in
301+
let code_id' = Renaming.apply_code_id renaming code_id in
302302
if code_id == code_id' then newer_version_of else Some code_id'
303303
in
304-
let code_id' = Renaming.apply_code_id perm code_id in
305-
let result_types' = Result_types.apply_renaming result_types perm in
304+
let code_id' = Renaming.apply_code_id renaming code_id in
305+
let result_types' = Result_types.apply_renaming result_types renaming in
306306
if code_id == code_id'
307307
&& newer_version_of == newer_version_of'
308308
&& result_types == result_types'

middle_end/flambda2/terms/code_or_metadata.ml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ let free_names t =
6262
let apply_renaming t renaming =
6363
match t with
6464
| Code_present code ->
65-
let code = Code.apply_renaming code renaming in
66-
Code_present code
65+
let code' = Code.apply_renaming code renaming in
66+
if code == code' then t else Code_present code'
6767
| Metadata_only code_metadata ->
68-
let code_metadata = Code_metadata.apply_renaming code_metadata renaming in
69-
Metadata_only code_metadata
68+
let code_metadata' = Code_metadata.apply_renaming code_metadata renaming in
69+
if code_metadata == code_metadata' then t else Metadata_only code_metadata'
7070

7171
let all_ids_for_export t =
7272
match t with

middle_end/flambda2/terms/code_size.ml

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ type t = int
2020

2121
let zero = 0
2222

23-
let equal a b = a = b
23+
let equal (a : t) (b : t) = a = b
2424

2525
let ( + ) (a : t) (b : t) : t = a + b
2626

27-
let ( <= ) a b = a <= b
27+
let ( <= ) (a : t) (b : t) = a <= b
2828

2929
let arch32 = Targetint_32_64.size = 32 (* are we compiling for a 32-bit arch *)
3030

@@ -65,7 +65,10 @@ let unary_int_prim_size kind op =
6565
(op : Flambda_primitive.unary_int_arith_op) )
6666
with
6767
| Tagged_immediate, Neg -> 1
68-
| Tagged_immediate, Swap_byte_endianness -> 2 + nonalloc_extcall_size + 1
68+
| Tagged_immediate, Swap_byte_endianness ->
69+
(* CR pchambart: size depends a lot of the architecture. If the backend
70+
handles it, this is a single arith op. *)
71+
2 + nonalloc_extcall_size + 1
6972
| Naked_immediate, Neg -> 1
7073
| Naked_immediate, Swap_byte_endianness -> nonalloc_extcall_size + 1
7174
| Naked_int64, Neg when arch32 -> nonalloc_extcall_size + 1
@@ -140,18 +143,17 @@ let array_load (kind : Flambda_primitive.Array_kind.t) =
140143

141144
let block_set (kind : Flambda_primitive.Block_access_kind.t)
142145
(init : Flambda_primitive.Init_or_assign.t) =
143-
(* XXX these need checking for [Local_assignment] *)
144146
match kind, init with
145-
| Values _, (Assignment _ | Initialization) -> 1 (* cadda + store *)
147+
| Values _, Assignment Heap -> nonalloc_extcall_size (* caml_modify *)
148+
| Values _, (Assignment Local | Initialization) -> 1 (* cadda + store *)
146149
| Naked_floats _, (Assignment _ | Initialization) -> 1
147150

148151
let array_set (kind : Flambda_primitive.Array_kind.t)
149-
(_init : Flambda_primitive.Init_or_assign.t) =
150-
(* CR mshinwell: Check whether [init] should matter *)
151-
match kind with
152-
| Immediates -> 1 (* cadda + store *)
153-
| Naked_floats -> 1
154-
| Values -> 1
152+
(init : Flambda_primitive.Init_or_assign.t) =
153+
match kind, init with
154+
| Values, Assignment Heap -> nonalloc_extcall_size
155+
| Values, (Assignment Local | Initialization) -> 1
156+
| (Immediates | Naked_floats), (Assignment _ | Initialization) -> 1
155157

156158
let string_or_bigstring_load kind width =
157159
let start_address_load =
@@ -390,6 +392,7 @@ let prim (prim : Flambda_primitive.t) =
390392
| Variadic (p, args) -> variadic_prim_size p args
391393

392394
let simple simple =
395+
(* CR pchambart: some large const on ARM might be considered larger *)
393396
Simple.pattern_match simple ~const:(fun _ -> 1) ~name:(fun _ ~coercion:_ -> 0)
394397

395398
let static_consts _ = 0

middle_end/flambda2/terms/exn_continuation.ml

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,24 @@ let free_names { exn_handler; extra_args } =
8787
(Name_occurrences.singleton_continuation exn_handler)
8888
(Simple.List.free_names extra_args)
8989

90-
let apply_renaming ({ exn_handler; extra_args } as t) perm =
91-
let exn_handler' = Renaming.apply_continuation perm exn_handler in
92-
let extra_args' =
90+
let apply_renaming ({ exn_handler; extra_args } as t) renaming =
91+
let exn_handler' = Renaming.apply_continuation renaming exn_handler in
92+
let extra_args_changed = ref false in
93+
let new_extra_args =
9394
List.map
9495
(fun ((simple, kind) as extra_arg) ->
95-
let simple' = Simple.apply_renaming simple perm in
96-
if simple == simple' then extra_arg else simple', kind)
96+
let simple' = Simple.apply_renaming simple renaming in
97+
if simple == simple'
98+
then extra_arg
99+
else begin
100+
extra_args_changed := true;
101+
simple', kind
102+
end)
97103
extra_args
98104
in
105+
let extra_args' =
106+
if !extra_args_changed then new_extra_args else extra_args
107+
in
99108
if exn_handler == exn_handler' && extra_args == extra_args'
100109
then t
101110
else { exn_handler = exn_handler'; extra_args = extra_args' }

middle_end/flambda2/terms/field_of_static_block.ml

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,21 @@ include Container_types.Make (struct
4848

4949
let print ppf t =
5050
match t with
51-
| Symbol symbol -> Symbol.print ppf symbol
52-
| Tagged_immediate immediate -> Targetint_31_63.print ppf immediate
53-
| Dynamically_computed (var, _dbg) -> Variable.print ppf var
51+
| Symbol symbol ->
52+
Format.fprintf ppf "%s%a%s"
53+
(Flambda_colours.symbol ())
54+
Symbol.print symbol
55+
(Flambda_colours.normal ())
56+
| Tagged_immediate immediate ->
57+
Format.fprintf ppf "%s%a%s"
58+
(Flambda_colours.tagged_immediate ())
59+
Targetint_31_63.print immediate
60+
(Flambda_colours.normal ())
61+
| Dynamically_computed (var, _dbg) ->
62+
Format.fprintf ppf "%s%a%s"
63+
(Flambda_colours.variable ())
64+
Variable.print var
65+
(Flambda_colours.normal ())
5466
end)
5567

5668
let apply_renaming t renaming =

0 commit comments

Comments
 (0)