Skip to content

Commit c37889e

Browse files
committed
Allow overlap of invalid rewrite id and extra_args
As the comment states, it can happen that, when adding a extra param and args, there is an overlap between the domain of the extra_args map, and the set of invalids (because of the way these are computed by the unboxing code). This case is reasonable, so in such a case, we can allow the invalid set to take precedenhce.
1 parent 3c0d811 commit c37889e

File tree

1 file changed

+13
-13
lines changed

1 file changed

+13
-13
lines changed

middle_end/flambda2/simplify/continuation_extra_params_and_args.ml

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -65,18 +65,12 @@ let empty = Empty
6565
let is_empty = function Empty -> true | Non_empty _ -> false
6666

6767
let add t ~invalids ~extra_param ~extra_args =
68-
if not
69-
(Apply_cont_rewrite_id.Set.is_empty
70-
(Apply_cont_rewrite_id.Set.inter invalids
71-
(Apply_cont_rewrite_id.Map.keys extra_args)))
72-
then
73-
Misc.fatal_errorf
74-
"Broken invariants: when adding an extra param to a continuation, every \
75-
Apply_cont_rewrite_id should either have a valid extra arg, or be \
76-
invalid, but not both:@ %a@ %a"
77-
Apply_cont_rewrite_id.Set.print invalids
78-
(Apply_cont_rewrite_id.Map.print Extra_arg.print)
79-
extra_args;
68+
(* Note: there can be some overlap between the invalid ids and the keys of the
69+
[extra_args] map. This is notably used by the unboxing code which may
70+
compute some extra args and only later (when computing extra args for
71+
another parameter) realize that some rewrite ids are invalids, and then
72+
call this function with this new invalid set and the extra_args computed
73+
before this invalid set was known. *)
8074
match t with
8175
| Empty ->
8276
let extra_params = Bound_parameters.create [extra_param] in
@@ -96,6 +90,10 @@ let add t ~invalids ~extra_param ~extra_args =
9690
let extra_args =
9791
Apply_cont_rewrite_id.Map.merge
9892
(fun id already_extra_args extra_arg ->
93+
(* The [invalids] set is expected to be small (actually, empty most of
94+
the time), so the lookups in each case of the merge should be
95+
reasonable, compared to merging (and allocating) the [invalids] set
96+
and the [extra_args] map. *)
9997
match already_extra_args, extra_arg with
10098
| None, None -> None
10199
| None, Some _ ->
@@ -125,7 +123,9 @@ let add t ~invalids ~extra_param ~extra_args =
125123
extra_args Apply_cont_rewrite_id.Set.print invalids print t
126124
| Some Or_invalid.Invalid, Some _ -> Some Or_invalid.Invalid
127125
| Some (Or_invalid.Ok already_extra_args), Some extra_arg ->
128-
Some (Or_invalid.Ok (extra_arg :: already_extra_args)))
126+
if Apply_cont_rewrite_id.Set.mem id invalids
127+
then Some Or_invalid.Invalid
128+
else Some (Or_invalid.Ok (extra_arg :: already_extra_args)))
129129
already_extra_args extra_args
130130
in
131131
Non_empty { extra_params; extra_args }

0 commit comments

Comments
 (0)