Skip to content

Commit 40c7d3b

Browse files
authored
flambda2(types): Refactor add_equation (#3873)
Previously, the `add_equation` logic handled multiple concerns at the same time in a relatively convoluted way, in part due to the need to accomodate both basic and advanced meet algorithms in the same code. With the removal of the basic meet (#3726), we can considerably simplify the code. This commit refactors `add_equation` by decomposing it into smaller, well-defined steps and recognizing that concrete types and aliases need to be handled differently. This restructuring clarifies the process of adding an equation: 1. Compute the canonical of the LHS with `get_canonical_simple_ignoring_name_mode` 2. Check if the type is a concrete type or an alias, which is then resolved to its current canonical (`add_equation_on_canonical`). 3. If an alias, merge the two canonical simples in the Aliases structure, then call `record_demotion` to update the typing env, transferring the existing concrete type from the demoted element to its new canonical element. 4. If a concrete type, perform the `meet` with the existing concrete type, then call `replace_concrete_equation` if a more precise concrete type is found. `replace_concrete_equation` automatically demotes to a constant instead if the concrete type is a singleton (without going through `record_demotion`, since we have already computed the `meet`). Additionally, `Type_grammar.recover_const_alias` is renamed to `must_be_singleton`, which better captures its behavior. Overall, this should be easier to read and maintain. It also facilitates extensions; for instance, the improved relations tracking for the match-in-match heuristics will rely on `record_demotion` and `add_alias_between_canonicals`.
1 parent 980f0d1 commit 40c7d3b

File tree

6 files changed

+176
-190
lines changed

6 files changed

+176
-190
lines changed

middle_end/flambda2/types/env/aliases.ml

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -729,30 +729,30 @@ let choose_canonical_element_to_be_demoted ~binding_time_resolver
729729

730730
type add_result =
731731
{ t : t;
732-
canonical_element : Simple.t;
733-
alias_of_demoted_element : Simple.t
732+
demoted_name : Name.t;
733+
canonical_element : Simple.t
734734
}
735735

736736
let invariant_add_result ~binding_time_resolver ~binding_times_and_modes
737-
~original_t { canonical_element; alias_of_demoted_element; t } =
737+
~original_t { demoted_name; canonical_element; t } =
738738
if Flambda_features.check_invariants ()
739739
then (
740740
invariant ~binding_time_resolver ~binding_times_and_modes t;
741741
if not
742742
(defined_earlier ~binding_time_resolver ~binding_times_and_modes
743-
canonical_element ~than:alias_of_demoted_element)
743+
canonical_element ~than:(Simple.name demoted_name))
744744
then
745745
Misc.fatal_errorf
746746
"Canonical element %a should be defined earlier than %a after alias \
747747
addition.@ Original alias tracker:@ %a@ Resulting alias tracker:@ %a"
748-
Simple.print canonical_element Simple.print alias_of_demoted_element
749-
print original_t print t;
750-
match canonical t alias_of_demoted_element with
748+
Simple.print canonical_element Name.print demoted_name print original_t
749+
print t;
750+
match canonical t (Simple.name demoted_name) with
751751
| Is_canonical ->
752752
Misc.fatal_errorf
753753
"Alias %a must not be must not be canonical anymore.@ Original alias \
754754
tracker:@ %a@ Resulting alias tracker:@ %a"
755-
Simple.print alias_of_demoted_element print original_t print t
755+
Name.print demoted_name print original_t print t
756756
| Alias_of_canonical _ -> ())
757757

758758
let add_alias ~binding_time_resolver ~binding_times_and_modes t
@@ -789,10 +789,9 @@ let add_alias ~binding_time_resolver ~binding_times_and_modes t
789789
in
790790
t, which_element
791791

792-
let add ~binding_time_resolver ~binding_times_and_modes t
792+
let add0 ~binding_time_resolver ~binding_times_and_modes t
793793
~canonical_element1:element1_with_coercion
794794
~canonical_element2:element2_with_coercion =
795-
let original_t = t in
796795
(* element1_with_coercion <--[c1]-- element1
797796
* +
798797
* element2_with_coercion <--[c2]-- element2
@@ -814,26 +813,35 @@ let add ~binding_time_resolver ~binding_times_and_modes t
814813
then
815814
Misc.fatal_errorf "Cannot alias an element to itself: %a" Simple.print
816815
canonical_element1;
816+
add_alias ~binding_time_resolver ~binding_times_and_modes t
817+
~canonical_element1 ~coercion_from_canonical_element2_to_canonical_element1
818+
~canonical_element2
819+
820+
let add ~binding_time_resolver ~binding_times_and_modes t ~canonical_element1
821+
~canonical_element2 =
817822
let open Or_bottom.Let_syntax in
823+
let original_t = t in
818824
let<+ t, which_element =
819-
add_alias ~binding_time_resolver ~binding_times_and_modes t
820-
~canonical_element1
821-
~coercion_from_canonical_element2_to_canonical_element1
825+
add0 ~binding_time_resolver ~binding_times_and_modes t ~canonical_element1
822826
~canonical_element2
823827
in
824828
let canonical_element, alias_of_demoted_element =
825829
match which_element with
826-
| Demote_canonical_element1 ->
827-
element2_with_coercion, element1_with_coercion
828-
| Demote_canonical_element2 ->
829-
element1_with_coercion, element2_with_coercion
830+
| Demote_canonical_element1 -> canonical_element2, canonical_element1
831+
| Demote_canonical_element2 -> canonical_element1, canonical_element2
830832
in
831-
let add_result = { t; canonical_element; alias_of_demoted_element } in
832-
if Flambda_features.check_invariants ()
833-
then
834-
invariant_add_result ~binding_time_resolver ~binding_times_and_modes
835-
~original_t add_result;
836-
add_result
833+
Simple.pattern_match alias_of_demoted_element
834+
~const:(fun _ -> Misc.fatal_error "Cannot demote a constant")
835+
~name:(fun name ~coercion ->
836+
let canonical_element =
837+
Simple.apply_coercion_exn canonical_element (Coercion.inverse coercion)
838+
in
839+
let add_result = { t; canonical_element; demoted_name = name } in
840+
if Flambda_features.check_invariants ()
841+
then
842+
invariant_add_result ~binding_time_resolver ~binding_times_and_modes
843+
~original_t add_result;
844+
add_result)
837845

838846
(* CR-someday mshinwell: For the moment we allow relations between canonical
839847
elements that are actually incomparable under the name mode ordering, and
@@ -1036,11 +1044,16 @@ let add_alias_set ~binding_time_resolver ~binding_times_and_modes t name aliases
10361044
then t, canonical_element1
10371045
else
10381046
match
1039-
add ~binding_time_resolver ~binding_times_and_modes t
1047+
add0 ~binding_time_resolver ~binding_times_and_modes t
10401048
~canonical_element1 ~canonical_element2:elt
10411049
with
10421050
| Bottom -> Misc.fatal_error "Discovered Bottom during alias join"
1043-
| Ok { t; canonical_element; alias_of_demoted_element = _ } ->
1051+
| Ok (t, which_element) ->
1052+
let canonical_element =
1053+
match which_element with
1054+
| Demote_canonical_element1 -> elt
1055+
| Demote_canonical_element2 -> canonical_element1
1056+
in
10441057
t, canonical_element
10451058
in
10461059
let ({ const; names } : Alias_set.t) = aliases in

middle_end/flambda2/types/env/aliases.mli

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,14 @@ val is_empty : t -> bool
6060
(** The result of calling [add] to state that two [Simple.t]s are now aliases. *)
6161
type add_result = private
6262
{ t : t; (** The new state of the alias tracker. *)
63-
canonical_element : Simple.t;
64-
(** The canonical element of the combined equivalence class. In the type
65-
environment, this will be the name (if it is a name) that is
66-
assigned a concrete type. *)
67-
alias_of_demoted_element : Simple.t
63+
demoted_name : Name.t;
6864
(** Whichever argument to [add] had its equivalence class consumed and
6965
its canonical element demoted to an alias. It is this name that
7066
needs its type to change to record the new canonical element. *)
67+
canonical_element : Simple.t
68+
(** The canonical element of the combined equivalence class. In the type
69+
environment, this will be the name (if it is a name) that is
70+
assigned a concrete type. *)
7171
}
7272

7373
(** Add an alias relationship to the tracker. The two simple expressions must be

0 commit comments

Comments
 (0)