Skip to content

Commit 10601fa

Browse files
committed
Avoid sets, maps of simple expressions
A `Name.Set.t` or `'a Name.Map.t` is a fine thing, but a `Simple.Set.t` or `'a Simple.Map.t` is much sketchier, since whether we want to treat two simples that differ only in `Rec_info.t` as equivalent depends on context. Fortunately, we usually just need a set or map of names. The one place where this gets tricky is in the uses of `Aliases.get_alias`, so there's now `Aliases.Alias_set.t` to encapsulate the needed operations.
1 parent bc27e1e commit 10601fa

9 files changed

+460
-297
lines changed

middle_end/flambda/basic/kinded_parameter.ml

-2
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,6 @@ module List = struct
111111

112112
let name_set t = Name.Set.of_list (List.map Name.var (vars t))
113113

114-
let simple_set t = Simple.Set.of_list (simples t)
115-
116114
let rename t = List.map (fun t -> rename t) t
117115

118116
let arity t = List.map (fun t -> Flambda_kind.With_subkind.kind (kind t)) t

middle_end/flambda/basic/kinded_parameter.mli

-2
Original file line numberDiff line numberDiff line change
@@ -68,8 +68,6 @@ module List : sig
6868
(** As for [var_set] but returns a set of [Name]s. *)
6969
val name_set : t -> Name.Set.t
7070

71-
val simple_set : t -> Simple.Set.t
72-
7371
val equal_vars : t -> Variable.t list -> bool
7472

7573
val rename : t -> t

middle_end/flambda/simplify/common_subexpression_elimination.ml

+12-5
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,12 @@ end
117117

118118
let cse_with_eligible_lhs ~typing_env_at_fork ~cse_at_each_use ~params prev_cse
119119
(extra_bindings: EPA.t) extra_equations =
120-
let params = KP.List.simple_set params in
120+
let params = KP.List.name_set params in
121+
let is_param simple =
122+
Simple.pattern_match simple
123+
~name:(fun name -> Name.Set.mem name params)
124+
~const:(fun _ -> false)
125+
in
121126
List.fold_left cse_at_each_use ~init:EP.Map.empty
122127
~f:(fun eligible (env_at_use, id, cse) ->
123128
let find_new_name =
@@ -184,15 +189,17 @@ let cse_with_eligible_lhs ~typing_env_at_fork ~cse_at_each_use ~params prev_cse
184189
since they are defined in [env_at_fork]. However these
185190
aren't bound at the use sites, so we must choose another
186191
alias that is. *)
187-
if not (Simple.Set.mem bound_to params) then Some bound_to
192+
if not (is_param bound_to) then Some bound_to
188193
else
189194
let aliases =
190195
TE.aliases_of_simple env_at_use
191196
~min_name_mode:NM.normal bound_to
192-
|> Simple.Set.filter (fun simple ->
193-
not (Simple.Set.mem simple params))
197+
|> Aliases.Alias_set.filter ~f:(fun simple ->
198+
not (is_param simple))
194199
in
195-
Simple.Set.get_singleton aliases
200+
(* CR lmaurer: This could be using
201+
[Aliases.Alias_set.find_best], I think? *)
202+
Aliases.Alias_set.get_singleton aliases
196203
in
197204
match bound_to with
198205
| None -> eligible

0 commit comments

Comments
 (0)