From 5c9a9291e1afcc0653e775beafc22e98c28ef10e Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Thu, 2 May 2024 14:25:14 +0200 Subject: [PATCH 01/25] compiler: Extend sharing status db to handle a sequence of extracts Extend sharing status database to handle chains of extracts. Previously it only handled the innermost embed+extract pair and conservatively classified longer chains as producing an aliased value. --- lib/compiler/src/beam_ssa_ss.erl | 88 ++++++++++++------- .../test/beam_ssa_check_SUITE_data/alias.erl | 43 ++++++++- 2 files changed, 100 insertions(+), 31 deletions(-) diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index ae4fc32d35ea..a8240e489f2f 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -218,6 +218,8 @@ extract(Dst, Src, Element, State) -> OutEdges = beam_digraph:out_edges(State, Src), ?ASSERT(true = is_integer(Element) orelse (Element =:= hd) orelse (Element =:= tl)), + ?DP("dst: ~p, src: ~p, e: ~p, out-edges: ~p~n", + [Dst, Src, Element, OutEdges]), extract_element(Dst, Src, Element, OutEdges, State) end. @@ -239,39 +241,65 @@ extract_element(Dst, Src, Element, [], State0) -> State = ?assert_state(add_edge(State0, Src, Dst, {extract,Element})), extract_status_for_element(Element, Src, Dst, State). -extract_status_for_element(Element, Src, Dst, State) -> +extract_status_for_element(Element, Src, Dst, State0) -> ?DP(" extract_status_for_element(~p, ~p)~n", [Element, Src]), - InEdges = beam_digraph:in_edges(State, Src), - extract_status_for_element(InEdges, Element, Src, Dst, State). - -extract_status_for_element([{N,_,{embed,Element}}|_InEdges], - Element, _Src, Dst, State0) -> - ?DP(" found new source ~p~n", [N]), - ?DP(" SS ~p~n", [State0]), - ?DP(" status ~p~n", [beam_digraph:vertex(State0, N)]), - State = set_status(Dst, beam_digraph:vertex(State0, N), State0), - ?DP(" Returned SS ~p~n", [State]), - ?assert_state(State); -extract_status_for_element([{N,_,{extract,SrcElement}}|InEdges], - Element, Src, Dst, State0) -> - ?DP(" found source: ~p[~p]~n", [N,SrcElement]), - Origs = [Var || {Var,_,{embed,SE}} <- beam_digraph:in_edges(State0, N), - SrcElement =:= SE], - ?DP(" original var: ~p~n", [Origs]), - case Origs of - [] -> + InEdges = beam_digraph:in_edges(State0, Src), + ?DP(" in-edges: ~p~n", [InEdges]), + Embeddings = [Var || {Var,_,{embed,SE}} <- InEdges, Element =:= SE], + Extracts = [Ex || {_,_,{extract,_}}=Ex <- InEdges], + case {Embeddings,Extracts} of + {[],[]} -> + %% Nothing found, the status will be aliased. ?DP(" no known source~n"), - extract_status_for_element(InEdges, Element, Src, Dst, State0); - [Orig] -> - extract_status_for_element(Element, Orig, Dst, State0) + ?DP(" status of ~p will be aliased~n", [Dst]), + ?assert_state(set_status(Dst, aliased, State0)); + {[Var],[]} -> + %% The element which is looked up is an embedding. + ?DP(" found embedding~n"), + ?DP(" the source is ~p~n", [Var]), + ?DP(" SS ~p~n", [State0]), + ?DP(" status ~p~n", [beam_digraph:vertex(State0, Var)]), + State = set_status(Dst, beam_digraph:vertex(State0, Var), State0), + ?DP(" Returned SS ~p~n", [State]), + ?assert_state(State); + {[], [{Aggregate,_Dst,{extract,E}}]} -> + %% We are trying extract from an extraction. The database + %% keeps no information about the strucure of the + %% extracted term. We have to conservatively set the + %% status of Dst to aliased. + + S = get_status_of_extracted_element(Aggregate, [E,Element], State0), + ?DP(" status of ~p will be ~p~n", [Dst, S]), + ?assert_state(set_status(Dst, S, State0)) + end. + + +get_status_of_extracted_element(Aggregate, [First|Rest]=Elements, State) -> + ?DP(" ~s(~p, ~p, ...)~n", [?FUNCTION_NAME, Aggregate, Elements]), + %% This clause will only be called when there is a chain of + %% extracts from unique aggregates. This implies that when the + %% chain is traced backwards, no aliased aggregates will be found, + %% but in case that invariant is ever broken, assert. + unique = beam_digraph:vertex(State, Aggregate), + ?DP(" aggregate is unique~n"), + InEdges = beam_digraph:in_edges(State, Aggregate), + Embeddings = [Src || {Src,_,{embed,E}} <- InEdges, First =:= E], + Extracts = [{Src,E} || {Src,_,{extract,E}} <- InEdges], + ?DP(" embeddings ~p~n", [Embeddings]), + ?DP(" extracts ~p~n", [Extracts]), + case {Embeddings,Extracts} of + {[Embedding],[]} -> + get_status_of_extracted_element(Embedding, Rest, State); + {[],[{Extract,E}]} -> + get_status_of_extracted_element(Extract, [E|Elements], State); + {[],[]} -> + aliased end; -extract_status_for_element([_Edge|InEdges], Element, Src, Dst, State) -> - ?DP(" ignoring in-edge ~p~n", [_Edge]), - extract_status_for_element(InEdges, Element, Src, Dst, State); -extract_status_for_element([], _Element, _Src, Dst, State) -> - %% Nothing found, the status will be aliased. - ?DP(" status of ~p will be aliased~n", [Dst]), - ?assert_state(set_status(Dst, aliased, State)). +get_status_of_extracted_element(Aggregate, [], State) -> + ?DP(" ~s(~p, [], ...)~n", [?FUNCTION_NAME, Aggregate]), + S = beam_digraph:vertex(State, Aggregate), + ?DP(" bottomed out, status is ~p~n", [S]), + S. %% A cut-down version of merge/2 which only considers variables in %% Main and whether they have been aliased in Other. diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl index 197bc0d7410d..5d71a4159428 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl @@ -106,7 +106,11 @@ fuzz0/0, fuzz0/1, alias_after_phi/0, - check_identifier_type/0]). + check_identifier_type/0, + + nested_tuple/0, + nested_cons/0, + nested_mixed/0]). %% Trivial smoke test transformable0(L) -> @@ -1153,3 +1157,40 @@ should_return_unique({X}) -> %ssa% (_) when post_ssa_opt -> %ssa% ret(R) { unique => [R] }. X. + +%% Check that the alias analysis handles a chain of extracts from +%% tuples. +nested_tuple_inner() -> + {{{{<<>>, e:x()}}}}. + +nested_tuple() -> +%ssa% () when post_ssa_opt -> +%ssa% U = bs_create_bin(append, _, T, ...) { unique => [T] }, +%ssa% R = put_tuple(U, A) { aliased => [A], unique => [U] }, +%ssa% ret(R). + {{{{Z,X}}}} = nested_tuple_inner(), + {<>,X}. + +%% Check that the alias analysis handles a chain of extracts from +%% pairs. +nested_cons_inner() -> + [[[[<<>>, e:x()]]]]. + +nested_cons() -> +%ssa% () when post_ssa_opt -> +%ssa% U = bs_create_bin(append, _, T, ...) { unique => [T] }, +%ssa% R = put_tuple(U, A) { aliased => [A], unique => [U] }, +%ssa% ret(R). + [[[[Z,X]]]] = nested_cons_inner(), + {<>,X}. + +nested_mixed_inner() -> + [{[{<<>>, e:x()}]}]. + +nested_mixed() -> +%ssa% () when post_ssa_opt -> +%ssa% U = bs_create_bin(append, _, T, ...) { unique => [T] }, +%ssa% R = put_tuple(U, A) { aliased => [A], unique => [U] }, +%ssa% ret(R). + [{[{Z,X}]}] = nested_mixed_inner(), + {<>,X}. From 08bd121f1bb0c92868242a072227d1edfa6a5cbe Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 5 Aug 2024 09:39:50 +0200 Subject: [PATCH 02/25] compiler SSA-checker: Handle succeeded instructions There was previously no way to properly match succeed instructions, this patch fixes this omission. --- lib/compiler/src/beam_ssa_check.erl | 6 ++++++ .../test/beam_ssa_check_SUITE_data/sanity_checks.erl | 11 ++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/compiler/src/beam_ssa_check.erl b/lib/compiler/src/beam_ssa_check.erl index 29a1264daf6c..0bc4ed9cb65e 100644 --- a/lib/compiler/src/beam_ssa_check.erl +++ b/lib/compiler/src/beam_ssa_check.erl @@ -159,6 +159,12 @@ op_check([set,Result,{{atom,_,bif},{atom,_,Op}}|PArgs], PAnno, [Op, Result, Dst, PArgs, AArgs, _I]), Env = op_check_call(Op, Result, Dst, PArgs, AArgs, Env0), check_annos(PAnno, AAnno, Env); +op_check([set,Result,{{atom,_,succeeded},{atom,_,Kind}}|PArgs], PAnno, + #b_set{dst=Dst,args=AArgs,op={succeeded,Kind},anno=AAnno}=_I, Env0) -> + ?DP("trying succeed ~p:~n res: ~p <-> ~p~n args: ~p <-> ~p~n i: ~p~n", + [Kind, Result, Dst, PArgs, AArgs, _I]), + Env = op_check_call(dont_care, Result, Dst, PArgs, AArgs, Env0), + check_annos(PAnno, AAnno, Env); op_check([none,{atom,_,ret}|PArgs], PAnno, #b_ret{arg=AArg,anno=AAnno}=_I, Env) -> ?DP("trying return:, arg: ~p <-> ~p~n i: ~p~n", diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/sanity_checks.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/sanity_checks.erl index a47665597916..9624f6000940 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/sanity_checks.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/sanity_checks.erl @@ -37,7 +37,8 @@ t35/1, t36/0, t37/0, t38/0, t39/1, t40/0, t41/0, t42/0, t43/0, t44/0, - check_env/0]). + check_env/0, + check_succeeded/1]). %% Check that we do not trigger on the wrong pass check_wrong_pass() -> @@ -339,3 +340,11 @@ check_env() -> A = <<>>, B = ex:f(), <>. + +%% Check that succeeded-instructions can be matched. +check_succeeded(L) -> +%ssa% (L) when post_ssa_opt -> +%ssa% X = bif:hd(L), +%ssa% Y = succeeded:body(X), +%ssa% ret(X). + hd(L). From 2680c84505d5ffb8d73be06c87df3437042da0ae Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Wed, 29 May 2024 16:05:50 +0200 Subject: [PATCH 03/25] compiler destructive update: Simplify logic in patch_is/5 Simplify the logic of beam_ssa_destructive_update:patch_is/5 to not special-case the extraction of force_copy patches as a part of processing a series of opargs patches. Instead filter out the relevant patches and let the rest be handled by a recursive call. --- .../src/beam_ssa_destructive_update.erl | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/compiler/src/beam_ssa_destructive_update.erl b/lib/compiler/src/beam_ssa_destructive_update.erl index c6e9d2875acf..1cb92440f866 100644 --- a/lib/compiler/src/beam_ssa_destructive_update.erl +++ b/lib/compiler/src/beam_ssa_destructive_update.erl @@ -92,7 +92,7 @@ -export([opt/2]). --import(lists, [foldl/3, foldr/3, keysort/2, reverse/1]). +-import(lists, [foldl/3, foldr/3, keysort/2, splitwith/2, reverse/1]). -include("beam_ssa_opt.hrl"). -include("beam_types.hrl"). @@ -830,19 +830,18 @@ patch_is([I0=#b_set{dst=Dst}|Rest], PD0, Cnt0, Acc, BlockAdditions0) PD = maps:remove(Dst, PD0), case Patches of [{opargs,Dst,_,_,_}|_] -> + Splitter = fun({opargs,D,_Idx,_Lit,_Element}) -> + Dst =:= D; + (_) -> + false + end, + {OpArgs0, Other} = splitwith(Splitter, Patches), OpArgs = [{Idx,Lit,Element} - || {opargs,D,Idx,Lit,Element} <- Patches, Dst =:= D], - Forced = [ F || {force_copy,_}=F <- Patches], - I1 = case Forced of - [] -> - I0; - _ -> - no_reuse(I0) - end, - 0 = length(Patches) - length(Forced) - length(OpArgs), + || {opargs,_D,Idx,Lit,Element} <- OpArgs0], Ps = keysort(1, OpArgs), - {Is,Cnt} = patch_opargs(I1, Ps, Cnt0), - patch_is(Rest, PD, Cnt, Is++Acc, BlockAdditions0); + {Is,Cnt} = patch_opargs(I0, Ps, Cnt0), + patch_is([hd(Is)|Rest], PD#{Dst=>Other}, Cnt, + tl(Is)++Acc, BlockAdditions0); [{appendable_binary,Dst,#b_literal{val= <<>>}=Lit}] -> %% Special case for when the first fragment is a literal %% <<>> and it has to be replaced with a bs_init_writable. From 31d430242ebdb0c28164595011ec1ce0ed977869 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Tue, 11 Jun 2024 15:30:33 +0200 Subject: [PATCH 04/25] compiler beam_digraph: Implement del_vertex/2 Implement del_vertex/2 analogous to digraph:del_vertex/2. --- lib/compiler/src/beam_digraph.erl | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/compiler/src/beam_digraph.erl b/lib/compiler/src/beam_digraph.erl index 5cf4b23d11fe..891d6982f90f 100644 --- a/lib/compiler/src/beam_digraph.erl +++ b/lib/compiler/src/beam_digraph.erl @@ -30,6 +30,7 @@ -export([new/0, add_vertex/2, add_vertex/3, add_edge/3, add_edge/4, del_edge/2, del_edges/2, + del_vertex/2, foldv/3, has_vertex/2, is_path/3, @@ -76,6 +77,18 @@ add_vertex(Dg, V, Label) -> Vs = Vs0#{V=>Label}, Dg#dg{vs=Vs}. +-spec del_vertex(graph(), vertex()) -> graph(). +del_vertex(Dg, V) -> + #dg{vs=Vs0,in_es=InEsMap0,out_es=OutEsMap0} = Dg, + InEs = maps:get(V, InEsMap0, []), + OutEsMap = foldl(fun({From,_,_}=E, A) -> edge_map_del(From, E, A) end, + maps:remove(V, OutEsMap0), InEs), + OutEs = maps:get(V, OutEsMap0, []), + InEsMap = foldl(fun({_,To,_}=E, A) -> edge_map_del(To, E, A) end, + maps:remove(V, InEsMap0), OutEs), + Vs = maps:remove(V, Vs0), + Dg#dg{vs=Vs,in_es=InEsMap,out_es=OutEsMap}. + -spec add_edge(graph(), vertex(), vertex()) -> graph(). add_edge(Dg, From, To) -> add_edge(Dg, From, To, edge). From 9c52efd49551b0354cfe1f4508185451b2ff79cf Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Thu, 27 Jun 2024 14:50:20 +0200 Subject: [PATCH 05/25] beam_digraph: Add beam_digraph:vertex/3 analogous to maps:get/3 Add beam_digraph:vertex/3 which allows for a default label if the vertex does not exist. --- lib/compiler/src/beam_digraph.erl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/compiler/src/beam_digraph.erl b/lib/compiler/src/beam_digraph.erl index 891d6982f90f..342c426452b5 100644 --- a/lib/compiler/src/beam_digraph.erl +++ b/lib/compiler/src/beam_digraph.erl @@ -37,7 +37,7 @@ in_degree/2, in_edges/2, in_neighbours/2, no_vertices/1, out_degree/2, out_edges/2, out_neighbours/2, - vertex/2, vertices/1, + vertex/2, vertex/3, vertices/1, reverse_postorder/2, roots/1, topsort/1, @@ -189,6 +189,12 @@ no_vertices(#dg{vs=Vs}) -> vertex(#dg{vs=Vs}, V) -> map_get(V, Vs). +%% As vertex/2 but if the vertex does not exist a default value is +%% returned. +-spec vertex(graph(), vertex(), label()) -> label(). +vertex(#dg{vs=Vs}, V, Default) -> + maps:get(V, Vs, Default). + -spec vertices(graph()) -> [{vertex(), label()}]. vertices(#dg{vs=Vs}) -> maps:to_list(Vs). From 3ce859c18335f090eeb54f104d0d3fa1be97bc41 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 17 Jun 2024 10:17:08 +0200 Subject: [PATCH 06/25] compiler beam_digraph: Add edges/1 Implement edges/1 analogous to digraph:edges/1. --- lib/compiler/src/beam_digraph.erl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/compiler/src/beam_digraph.erl b/lib/compiler/src/beam_digraph.erl index 342c426452b5..f89d39bb2b56 100644 --- a/lib/compiler/src/beam_digraph.erl +++ b/lib/compiler/src/beam_digraph.erl @@ -31,6 +31,7 @@ add_vertex/2, add_vertex/3, add_edge/3, add_edge/4, del_edge/2, del_edges/2, del_vertex/2, + edges/1, foldv/3, has_vertex/2, is_path/3, @@ -236,6 +237,12 @@ topsort(G) -> Seen = roots(G), reverse_postorder(G, Seen). +-spec edges(graph()) -> [edge()]. +edges(#dg{out_es=OutEsMap}) -> + maps:fold(fun(_, Es, Acc) -> + Es ++ Acc + end, [], OutEsMap). + %% %% Kosaraju's algorithm %% From d6d613f8dd35e761ed1ca9fa65f5ac7b6be314e3 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 17 Jun 2024 10:17:43 +0200 Subject: [PATCH 07/25] compiler beam_ssa_ss: Improve debug dumps Improve debug dumps of the sharing state database by dumping the graph structure in Graphviz format. --- lib/compiler/src/beam_ssa_ss.erl | 55 +++++++++++++++++++------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index a8240e489f2f..a561ae140376 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -61,11 +61,9 @@ -ifdef(DEBUG). -define(DP(FMT, ARGS), io:format(FMT, ARGS)). -define(DP(FMT), io:format(FMT)). --define(DBG(STMT), STMT). -else. -define(DP(FMT, ARGS), skip). -define(DP(FMT), skip). --define(DBG(STMT), skip). -endif. @@ -124,7 +122,7 @@ add_edge(State, Src, Dst, Lbl) -> -spec derive_from(beam_ssa:b_var(), beam_ssa:b_var(), sharing_state()) -> sharing_state(). derive_from(Dst, Src, State) -> - ?DP("Deriving ~p from ~p~nSS:~p~n", [Dst,Src,State]), + ?DP("Deriving ~p from ~p~nSS:~n~s~n", [Dst, Src, dump(State)]), ?assert_state(State), ?ASSERT(assert_variable_exists(Dst, State)), ?ASSERT(assert_variable_exists(Src, State)), @@ -154,7 +152,7 @@ derive_from(Dst, Src, State) -> -spec embed_in(beam_ssa:b_var(), [{element(),beam_ssa:b_var()}], sharing_state()) -> sharing_state(). embed_in(Dst, Elements, State0) -> - ?DP("Embedding ~p into ~p~nSS:~p~n", [Elements,Dst,State0]), + ?DP("Embedding ~p into ~p~nSS:~n~s~n", [Elements, Dst, dump(State0)]), ?assert_state(State0), ?ASSERT(assert_variable_exists(Dst, State0)), ?ASSERT([assert_variable_exists(Src, State0) @@ -257,10 +255,10 @@ extract_status_for_element(Element, Src, Dst, State0) -> %% The element which is looked up is an embedding. ?DP(" found embedding~n"), ?DP(" the source is ~p~n", [Var]), - ?DP(" SS ~p~n", [State0]), + ?DP(" SS~n~s~n", [dump(State0)]), ?DP(" status ~p~n", [beam_digraph:vertex(State0, Var)]), State = set_status(Dst, beam_digraph:vertex(State0, Var), State0), - ?DP(" Returned SS ~p~n", [State]), + ?DP(" Returned SS~n~s~n", [dump(State)]), ?assert_state(State); {[], [{Aggregate,_Dst,{extract,E}}]} -> %% We are trying extract from an extraction. The database @@ -345,9 +343,9 @@ merge(StateA, StateB) -> end, ?DP("Merging Small into Large~nLarge:~n"), ?DP("Small:~n"), - ?DBG(dump(Small)), + ?DP(dump(Small)), ?DP("Large:~n"), - ?DBG(dump(Large)), + ?DP(dump(Large)), R = merge(Large, Small, beam_digraph:vertices(Small), sets:new(), sets:new()), ?assert_state(R). @@ -449,10 +447,10 @@ new() -> prune(LiveVars, State) -> ?assert_state(State), ?DP("Pruning to ~p~n", [sets:to_list(LiveVars)]), - ?DBG(dump(State)), + ?DP("~s", [dump(State)]), R = prune([{0,V} || V <- sets:to_list(LiveVars)], [], new(), State), ?DP("Pruned result~n"), - ?DBG(dump(R)), + ?DP("~s", [dump(R)]), ?assert_state(R). prune([{Depth0,V}|Wanted], Edges, New0, Old) -> @@ -629,8 +627,8 @@ meet_in_args_elems1([], Large, Result) -> -spec merge_in_args([beam_ssa:b_var()], call_in_arg_info(), sharing_state()) -> call_in_arg_info(). merge_in_args([Arg|Args], [ArgStatus|Status], State) -> - ?DP(" merge_in_arg: ~p~n current: ~p~n SS: ~p.~n", - [Arg,ArgStatus,State]), + ?DP(" merge_in_arg: ~p~n current: ~p~n SS:~n~s.~n", + [Arg, ArgStatus, dump(State)]), Info = merge_in_arg(Arg, ArgStatus, ?ARGS_DEPTH_LIMIT, State), [Info|merge_in_args(Args, Status, State)]; merge_in_args([], [], _State) -> @@ -818,7 +816,7 @@ assert_apia(Parent, State) -> ok; _ -> io:format("Expected ~p to be aliased as is derived from ~p.~n" - "state: ~p", [Child, Parent, State]), + "state: ~s", [Child, Parent, dump(State)]), throw(assertion_failure) end || Child <- Children]. @@ -836,8 +834,8 @@ assert_eiaia(Embedder, State) -> State; _ -> io:format("Expected ~p to be aliased as" - " they are embedded in aliased values.~n~p.~n", - [NotAliased, State]), + " they are embedded in aliased values.~n~s.~n", + [NotAliased, dump(State)]), throw(assertion_failure) end. @@ -851,7 +849,7 @@ assert_mefa(V, State) -> beam_digraph:vertex(State, B) =/= aliased], case NotAliased of [_,_|_] -> - io:format("Expected ~p in ~p to be aliased.~n", [V,State]), + io:format("Expected ~p to be aliased in:~n~s.~n", [V, dump(State)]), throw(assertion_failure); _ -> State @@ -883,21 +881,21 @@ assert_mxfa(V, State) -> [] -> State; _ -> - io:format("~p should be aliased~nstate:~p.~n", [V,State]), + io:format("~p should be aliased~nstate:~n~s.~n", [V, dump(State)]), throw(assertion_failure) end. assert_variable_exists(plain, State) -> case beam_digraph:has_vertex(State, plain) of false -> - io:format("Expected ~p in ~p.~n", [plain,State]), + io:format("Expected ~p in~n ~s.~n", [plain, dump(State)]), throw(assertion_failure); _ -> case beam_digraph:vertex(State, plain) of unique -> State; Other -> - io:format("Expected plain in ~p to be unique," - " was ~p.~n", [State,Other]), + io:format("Expected plain to be unique, was ~p, in:~n~p~n", + [Other, dump(State)]), throw(assertion_failure) end end; @@ -906,7 +904,7 @@ assert_variable_exists(#b_literal{}, State) -> assert_variable_exists(#b_var{}=V, State) -> case beam_digraph:has_vertex(State, V) of false -> - io:format("Expected ~p in ~p.~n", [V,State]), + io:format("Expected ~p in~n~s.~n", [V, dump(State)]), throw(assertion_failure); _ -> State @@ -916,6 +914,17 @@ assert_variable_exists(#b_var{}=V, State) -> -ifdef(DEBUG). dump(State) -> - io:format("~p~n", [State]). - + Ls = lists:enumerate(0, beam_digraph:vertices(State)), + V2Id = #{ V=>Id || {Id,{V,_}} <- Ls }, + [ + "digraph G {\n", + [ io_lib:format(" ~p [label=\"~p:~p\",shape=\"box\"]~n", + [Id,V,beam_digraph:vertex(State, V)]) + || V:=Id <- V2Id], + [ io_lib:format(" ~p -> ~p [label=\"~p\"]~n", + [maps:get(From, V2Id), + maps:get(To, V2Id), + Lbl]) + || {From,To,Lbl} <- beam_digraph:edges(State)], + "}\n"]. -endif. From b332468b03424ead290ce5c771454f27a7413d96 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Wed, 31 Jul 2024 08:50:17 +0200 Subject: [PATCH 08/25] compiler beam_ssa_ss: Remove TODO after investigating performance beam_ssa_ss:variables/1 does not show up as a critical function during profiling, so remove the TODO. Pushing beam_ssa_ss-specific functionality down into beam_digraph is not motivated. --- lib/compiler/src/beam_ssa_ss.erl | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index a561ae140376..6065ffd61bfa 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -557,7 +557,6 @@ get_alias_edges(V, State) -> -spec variables(sharing_state()) -> [beam_ssa:b_var()]. variables(State) -> - %% TODO: Sink this beam_digraph to avoid splitting the list? [V || {V,_Lbl} <- beam_digraph:vertices(State)]. -type call_in_arg_status() :: no_info From 368a79d75f49318d67f4cbd457a5976d8c47387f Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Thu, 18 Jul 2024 08:53:15 +0200 Subject: [PATCH 09/25] compiler alias analysis: Unify support for debugging dumps Export beam_ssa_ss:dump/1 to make it accessible by the beam_ssa_alias module for printing the internal state of the pass. Break out the various settings for debugging the alias pass into a shared header. --- lib/compiler/src/Makefile | 6 ++-- lib/compiler/src/beam_ssa_alias.erl | 7 ++-- lib/compiler/src/beam_ssa_alias_debug.hrl | 43 +++++++++++++++++++++++ lib/compiler/src/beam_ssa_ss.erl | 22 ++++++------ 4 files changed, 59 insertions(+), 19 deletions(-) create mode 100644 lib/compiler/src/beam_ssa_alias_debug.hrl diff --git a/lib/compiler/src/Makefile b/lib/compiler/src/Makefile index 0ceba13ebe26..9b50016bb559 100644 --- a/lib/compiler/src/Makefile +++ b/lib/compiler/src/Makefile @@ -112,6 +112,7 @@ BEAM_H = $(wildcard ../priv/beam_h/*.h) HRL_FILES= \ beam_asm.hrl \ beam_disasm.hrl \ + beam_ssa_alias_debug.hrl \ beam_ssa_opt.hrl \ beam_ssa.hrl \ beam_types.hrl \ @@ -216,7 +217,8 @@ $(EBIN)/beam_jump.beam: beam_asm.hrl $(EBIN)/beam_listing.beam: core_parse.hrl beam_ssa.hrl \ beam_asm.hrl beam_types.hrl $(EBIN)/beam_ssa.beam: beam_ssa.hrl -$(EBIN)/beam_ssa_alias.beam: beam_ssa_opt.hrl beam_types.hrl +$(EBIN)/beam_ssa_alias.beam: beam_ssa_opt.hrl beam_ssa_alias_debug.hrl \ + beam_types.hrl $(EBIN)/beam_ssa_bsm.beam: beam_ssa.hrl $(EBIN)/beam_ssa_bool.beam: beam_ssa.hrl $(EBIN)/beam_ssa_check.beam: beam_ssa.hrl beam_types.hrl @@ -229,7 +231,7 @@ $(EBIN)/beam_ssa_pp.beam: beam_ssa.hrl beam_types.hrl $(EBIN)/beam_ssa_pre_codegen.beam: beam_ssa.hrl beam_asm.hrl $(EBIN)/beam_ssa_recv.beam: beam_ssa.hrl $(EBIN)/beam_ssa_share.beam: beam_ssa.hrl -$(EBIN)/beam_ssa_ss.beam: beam_ssa.hrl beam_types.hrl +$(EBIN)/beam_ssa_ss.beam: beam_ssa.hrl beam_ssa_alias_debug.hrl beam_types.hrl $(EBIN)/beam_ssa_throw.beam: beam_ssa.hrl beam_types.hrl $(EBIN)/beam_ssa_type.beam: beam_ssa.hrl beam_types.hrl $(EBIN)/beam_trim.beam: beam_asm.hrl diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index 19c438f61fde..b5d9a81e2f2d 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -32,12 +32,9 @@ -include("beam_ssa_opt.hrl"). -include("beam_types.hrl"). +-include("beam_ssa_alias_debug.hrl"). -%% Uncomment the following to get trace printouts. - -%% -define(DEBUG, true). - --ifdef(DEBUG). +-ifdef(DEBUG_ALIAS). -define(DP(FMT, ARGS), io:format(FMT, ARGS)). -define(DP(FMT), io:format(FMT)). -define(DBG(STMT), STMT). diff --git a/lib/compiler/src/beam_ssa_alias_debug.hrl b/lib/compiler/src/beam_ssa_alias_debug.hrl new file mode 100644 index 000000000000..1e3fc43002e0 --- /dev/null +++ b/lib/compiler/src/beam_ssa_alias_debug.hrl @@ -0,0 +1,43 @@ +%% +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2024. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% %CopyrightEnd% +%% +%% The beam_ssa_alias and beam_ssa_ss modules are both used by the +%% alias analysis pass of the compiler, where beam_ssa_ss provides the +%% underlying database manipulated by the beam_ssa_alias +%% module. Debugging beam_ssa_alias is simplified when the module can +%% print out the internal state of beam_ssa_ss, but as that code is +%% redundant otherwise it is ifdefed-out, this header exist in order +%% to avoid having to modify multiple modules when toggling debugging +%% traces for beam_ssa_alias and beam_ssa_ss. + +%% Uncomment the following to get trace printouts. + +%% -define(DEBUG_ALIAS, true). % For beam_ssa_alias + +%% -define(DEBUG_SS, true). % For beam_ssa_ss + +%% Uncomment the following to check that all invariants for the state +%% hold. These checks are expensive and not enabled by default. + +%% -define(SS_EXTRA_ASSERTS, true). + +-if(defined(DEBUG_ALIAS) orelse defined(DEBUG_SS) + orelse defined(SS_EXTRA_ASSERTS)). +-define(PROVIDE_DUMP, true). +-endif. diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index 6065ffd61bfa..4f45eda555e8 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -50,15 +50,18 @@ -include("beam_ssa.hrl"). -include("beam_types.hrl"). +-include("beam_ssa_alias_debug.hrl"). + +-ifdef(PROVIDE_DUMP). +-export([dump/1]). +-endif. -import(lists, [foldl/3]). -define(ARGS_DEPTH_LIMIT, 4). -define(SS_DEPTH_LIMIT, 30). -%% -define(DEBUG, true). - --ifdef(DEBUG). +-ifdef(DEBUG_SS). -define(DP(FMT, ARGS), io:format(FMT, ARGS)). -define(DP(FMT), io:format(FMT)). -else. @@ -66,13 +69,7 @@ -define(DP(FMT), skip). -endif. - -%% Uncomment the following to check that all invariants for the state -%% hold. These checks are expensive and not enabled by default. - -%% -define(EXTRA_ASSERTS, true). - --ifdef(EXTRA_ASSERTS). +-ifdef(SS_EXTRA_ASSERTS). -define(assert_state(State), assert_state(State)). -define(ASSERT(Assert), Assert). -else. @@ -786,7 +783,7 @@ has_out_edges(V, State) -> %% Debug support below --ifdef(EXTRA_ASSERTS). +-ifdef(SS_EXTRA_ASSERTS). -spec assert_state(sharing_state()) -> sharing_state(). @@ -910,8 +907,9 @@ assert_variable_exists(#b_var{}=V, State) -> end. -endif. --ifdef(DEBUG). +-ifdef(PROVIDE_DUMP). +-spec dump(sharing_state()) -> iolist(). dump(State) -> Ls = lists:enumerate(0, beam_digraph:vertices(State)), V2Id = #{ V=>Id || {Id,{V,_}} <- Ls }, From 82f52a1ef8fd74ba60ab3e04c31cba54b32dd124 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 22 Jul 2024 12:56:33 +0200 Subject: [PATCH 10/25] compiler alias analysis: Use beam_ssa_ss:dump/1 for debug printouts Use beam_ssa_ss:dump/1 for all debug printouts of sharing states. --- lib/compiler/src/beam_ssa_alias.erl | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index b5d9a81e2f2d..bbb4334180c9 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -393,7 +393,7 @@ aa_blocks([{?EXCEPTION_BLOCK,_}|Bs], Kills, Lbl2SS, AAS) -> aa_blocks(Bs, Kills, Lbl2SS, AAS); aa_blocks([{L,#b_blk{is=Is0,last=T}}|Bs0], Kills, Lbl2SS0, AAS0) -> #{L:=SS0} = Lbl2SS0, - ?DP("Block: ~p~nSS: ~p~n", [L, SS0]), + ?DP("Block: ~p~nSS:~n~s~n", [L, beam_ssa_ss:dump(SS0)]), {FullSS,AAS1} = aa_is(Is0, SS0, AAS0), #{{live_outs,L}:=LiveOut} = Kills, {Lbl2SS1,Successors} = aa_terminator(T, FullSS, Lbl2SS0), @@ -558,7 +558,7 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> wait_timeout -> {SS1, AAS0} end, - ?DP("Post I: ~p.~n ~p~n", [_I, SS]), + ?DP("Post I: ~p.~n~s~n", [_I, beam_ssa_ss:dump(SS)]), aa_is(Is, SS, AAS); aa_is([], SS, AAS) -> {SS, AAS}. @@ -982,7 +982,7 @@ aa_construct_tuple(Dst, IdxValues, Types, SS, AAS) -> killed=>aa_dies(V, Types, KillSet), plain=>aa_is_plain_value(V, Types)} || {Idx,V} <- IdxValues]]), - ?DP("~p~n", [SS]), + ?DP("~s~n", [beam_ssa_ss:dump(SS)]), aa_build_tuple_or_pair(Dst, IdxValues, Types, KillSet, SS, []). aa_build_tuple_or_pair(Dst, [{Idx,#b_literal{val=Lit}}|IdxValues], Types, @@ -1016,7 +1016,8 @@ aa_build_tuple_or_pair(Dst, [], _Types, _KillSet, SS, Sources) -> aa_construct_pair(Dst, Args0, Types, SS, AAS) -> KillSet = aa_killset_for_instr(Dst, AAS), [Hd,Tl] = Args0, - ?DP("Constructing pair in ~p~n from ~p and ~p~n~p~n", [Dst,Hd,Tl,SS]), + ?DP("Constructing pair in ~p~n from ~p and ~p~n~s~n", + [Dst, Hd, Tl, beam_ssa_ss:dump(SS)]), Args = [{hd,Hd},{tl,Tl}], aa_build_tuple_or_pair(Dst, Args, Types, KillSet, SS, []). @@ -1089,12 +1090,12 @@ aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, ?DP(" callee args: ~p~n", [_CalleeArgs]), ?DP(" caller args: ~p~n", [Args]), SS1 = aa_alias_surviving_args(Args, Dst, SS0, AAS0), - ?DP(" caller ss before call:~n ~p.~n", [SS1]), + ?DP(" caller ss before call:~n~s.~n", [beam_ssa_ss:dump(SS1)]), #aas{alias_map=AliasMap} = AAS = aa_add_call_info(Callee, Args, SS1, AAS0), #{Callee:=#{0:=_CalleeSS}=Lbl2SS} = AliasMap, - ?DP(" callee ss: ~p~n", [_CalleeSS]), - ?DP(" caller ss after call: ~p~n", [SS1]), + ?DP(" callee ss:~n~s~n", [beam_ssa_ss:dump(_CalleeSS)]), + ?DP(" caller ss after call:~n~s~n", [beam_ssa_ss:dump(SS1)]), ReturnStatusByType = maps:get(returns, Lbl2SS, #{}), ?DP(" status by type: ~p~n", [ReturnStatusByType]), @@ -1108,7 +1109,7 @@ aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, ?DP(" result status: ~p~n", [ResultStatus]), {SS,Cnt} = beam_ssa_ss:set_call_result(Dst, ResultStatus, SS1, Cnt0), - ?DP("~p~n", [SS]), + ?DP("~s~n", [beam_ssa_ss:dump(SS)]), {SS, AAS#aas{cnt=Cnt}}; false -> %% We don't know anything about the function, don't change @@ -1136,7 +1137,8 @@ aa_add_call_info(Callee, Args, SS0, ?DBG(#b_local{name=#b_literal{val=_CN},arity=_CA} = _Caller), ?DBG(#b_local{name=#b_literal{val=_N},arity=_A} = Callee), ?DP("Adding call info for ~p/~p when called by ~p/~p~n" - " args: ~p.~n ss:~p.~n", [_N,_A,_CN,_CA,Args,SS0]), + " args: ~p.~n ss:~n~s.~n", + [_N, _A, _CN, _CA, Args, beam_ssa_ss:dump(SS0)]), InStatus = beam_ssa_ss:merge_in_args(Args, InStatus0, SS0), ?DP(" orig in-info: ~p.~n", [InStatus0]), ?DP(" updated in-info for ~p/~p:~n ~p.~n", [_N,_A,InStatus]), From 03ea590162a5e4281766bf91a3c0d044d60ad0d8 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 22 Jul 2024 13:16:08 +0200 Subject: [PATCH 11/25] compiler alias analysis: Unify printing of function names Unify printing of function names in debug printouts. --- lib/compiler/src/beam_ssa_alias.erl | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index bbb4334180c9..359bcc3081ec 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -38,6 +38,10 @@ -define(DP(FMT, ARGS), io:format(FMT, ARGS)). -define(DP(FMT), io:format(FMT)). -define(DBG(STMT), STMT). + +fn(#b_local{name=#b_literal{val=N},arity=A}) -> + io_lib:format("~p/~p", [N, A]). + -else. -define(DP(FMT, ARGS), skip). -define(DP(FMT), skip). @@ -335,13 +339,12 @@ aa_fixpoint(Funs, AAS=#aas{alias_map=AliasMap,call_args=CallArgs, aa_fixpoint([F|Fs], Order, OldAliasMap, OldCallArgs, AAS0=#aas{st_map=StMap}, Limit) -> - #b_local{name=#b_literal{val=_N},arity=_A} = F, AAS1 = AAS0#aas{caller=F}, - ?DP("-= ~p/~p =-~n", [_N, _A]), + ?DP("-= ~s =-~n", [fn(F)]), St = #opt_st{ssa=_Is} = map_get(F, StMap), ?DP("code:~n~p.~n", [_Is]), AAS = aa_fun(F, St, AAS1), - ?DP("Done ~p/~p~n", [_N, _A]), + ?DP("Done ~s~n", [fn(F)]), aa_fixpoint(Fs, Order, OldAliasMap, OldCallArgs, AAS, Limit); aa_fixpoint([], Order, OldAliasMap, OldCallArgs, #aas{alias_map=OldAliasMap,call_args=OldCallArgs, @@ -667,8 +670,7 @@ aa_update_annotations(Funs, #aas{alias_map=AliasMap0,st_map=StMap0}=AAS) -> foldl(fun(F, {StMapAcc,AliasMapAcc}) -> #{F:=Lbl2SS0} = AliasMapAcc, #{F:=OptSt0} = StMapAcc, - #b_local{name=#b_literal{val=_N},arity=_A} = F, - ?DP("Updating annotations for ~p/~p~n", [_N,_A]), + ?DP("Updating annotations for ~s~n", [fn(F)]), {OptSt,Lbl2SS} = aa_update_fun_annotation(OptSt0, Lbl2SS0, AAS#aas{caller=F}), @@ -1081,8 +1083,7 @@ aa_phi(Dst, Args0, SS0, AAS) -> aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, #aas{alias_map=AliasMap,st_map=StMap,cnt=Cnt0}=AAS0) -> - #b_local{name=#b_literal{val=_N},arity=_A} = Callee, - ?DP("A Call~n callee: ~p/~p~n args: ~p~n", [_N, _A, Args]), + ?DP("A Call~n callee: ~s~n args: ~p~n", [fn(Callee), Args]), case is_map_key(Callee, AliasMap) of true -> ?DP(" The callee is known~n"), @@ -1134,22 +1135,20 @@ aa_call(Dst, [_Callee|Args], _Anno, SS0, AAS) -> aa_add_call_info(Callee, Args, SS0, #aas{call_args=InInfo0,caller=_Caller}=AAS) -> #{Callee := InStatus0} = InInfo0, - ?DBG(#b_local{name=#b_literal{val=_CN},arity=_CA} = _Caller), - ?DBG(#b_local{name=#b_literal{val=_N},arity=_A} = Callee), - ?DP("Adding call info for ~p/~p when called by ~p/~p~n" + ?DP("Adding call info for ~s when called by ~s~n" " args: ~p.~n ss:~n~s.~n", - [_N, _A, _CN, _CA, Args, beam_ssa_ss:dump(SS0)]), + [fn(Callee), fn(_Caller), Args, beam_ssa_ss:dump(SS0)]), InStatus = beam_ssa_ss:merge_in_args(Args, InStatus0, SS0), ?DP(" orig in-info: ~p.~n", [InStatus0]), - ?DP(" updated in-info for ~p/~p:~n ~p.~n", [_N,_A,InStatus]), + ?DP(" updated in-info for ~s:~n ~p.~n", [fn(Callee), InStatus]), InInfo = InInfo0#{Callee => InStatus}, AAS#aas{call_args=InInfo}. aa_init_fun_ss(Args, FunId, #aas{call_args=Info,st_map=StMap}) -> #{FunId:=ArgsStatus} = Info, #{FunId:=#opt_st{cnt=Cnt}} = StMap, - ?DP("aa_init_fun_ss: ~p~n args: ~p~n status: ~p~n cnt: ~p~n", - [FunId,Args,ArgsStatus,Cnt]), + ?DP("aa_init_fun_ss: ~s~n args: ~p~n status: ~p~n cnt: ~p~n", + [fn(FunId), Args, ArgsStatus, Cnt]), beam_ssa_ss:new(Args, ArgsStatus, Cnt). %% Pair extraction. From 105abbd27ba808416f87814eb3f544fa0ec84428 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 24 Jun 2024 16:09:43 +0200 Subject: [PATCH 12/25] compiler alias analysis: Speed up beam_ssa_ss:prune() Speed up beam_ssa_ss:prune() by instead of recreating the state by finding all variables reachable from the live variables, instead remove variables that have been killed. The main speedup comes from the observation that in most cases more than half of the variables in the state survives the pruning. --- lib/compiler/src/beam_ssa_alias.erl | 93 +++++++++++++------ lib/compiler/src/beam_ssa_ss.erl | 75 +++++++++------ .../ss_depth_limit.erl | 12 ++- 3 files changed, 119 insertions(+), 61 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index 359bcc3081ec..5239b6728989 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -66,13 +66,25 @@ fn(#b_local{name=#b_literal{val=N},arity=A}) -> %% or the terminator of a block. -type kill_loc() :: #b_var{} | {terminator, beam_ssa:label()} - | {live_outs, beam_ssa:label()}. + | {live_outs, beam_ssa:label()} + | {killed_in_block, beam_ssa:label()}. + +-type var_set() :: sets:set(#b_var{}). %% Map a code location to the set of variables which die at that %% location. --type kill_set() :: #{ kill_loc() => sets:set(#b_var{}) }. +-type kill_set() :: #{ kill_loc() => var_set() }. + +%% Map a label to the set of variables which are live in to that block. +-type live_in_set() :: #{ beam_ssa:label() => var_set() }. + +%% Map a flow-control edge to the set of variables which are live in +%% to the destination block due to phis. +-type phi_live_set() :: #{ {beam_ssa:label(), beam_ssa:label()} => var_set() }. --type kills_map() :: #{ func_id() => kill_set() }. +-type kills_map() :: #{ func_id() => {live_in_set(), + kill_set(), + phi_live_set()} }. -type alias_map() :: #{ func_id() => lbl2ss() }. @@ -136,19 +148,19 @@ killsets_fun(Blocks) -> killsets_blks([{Lbl,Blk}|Blocks], LiveIns0, Kills0, PhiLiveIns) -> {LiveIns,Kills} = killsets_blk(Lbl, Blk, LiveIns0, Kills0, PhiLiveIns), killsets_blks(Blocks, LiveIns, Kills, PhiLiveIns); -killsets_blks([], _LiveIns0, Kills, _PhiLiveIns) -> - Kills. +killsets_blks([], LiveIns, Kills, PhiLiveIns) -> + {LiveIns,Kills,PhiLiveIns}. killsets_blk(Lbl, #b_blk{is=Is0,last=L}=Blk, LiveIns0, Kills0, PhiLiveIns) -> Successors = beam_ssa:successors(Blk), Live1 = killsets_blk_live_outs(Successors, Lbl, LiveIns0, PhiLiveIns), Kills1 = Kills0#{{live_outs,Lbl}=>Live1}, Is = [L|reverse(Is0)], - {Live,Kills} = killsets_is(Is, Live1, Kills1, Lbl), + {Live,Kills} = killsets_is(Is, Live1, Kills1, Lbl, sets:new()), LiveIns = LiveIns0#{Lbl=>Live}, {LiveIns, Kills}. -killsets_is([#b_set{op=phi,dst=Dst}=I|Is], Live, Kills0, Lbl) -> +killsets_is([#b_set{op=phi,dst=Dst}=I|Is], Live, Kills0, Lbl, KilledInBlock0) -> %% The Phi uses are logically located in the predecessors, so we %% don't want them live in to this block. But to correctly %% calculate the aliasing of the arguments to the Phi in this @@ -157,21 +169,24 @@ killsets_is([#b_set{op=phi,dst=Dst}=I|Is], Live, Kills0, Lbl) -> Uses = beam_ssa:used(I), {_,LastUses} = killsets_update_live_and_last_use(Live, Uses), Kills = killsets_add_kills({phi,Dst}, LastUses, Kills0), - killsets_is(Is, sets:del_element(Dst, Live), Kills, Lbl); -killsets_is([I|Is], Live0, Kills0, Lbl) -> + KilledInBlock = sets:union(LastUses, KilledInBlock0), + killsets_is(Is, sets:del_element(Dst, Live), Kills, Lbl, KilledInBlock); +killsets_is([I|Is], Live0, Kills0, Lbl, KilledInBlock0) -> Uses = beam_ssa:used(I), {Live,LastUses} = killsets_update_live_and_last_use(Live0, Uses), + KilledInBlock = sets:union(LastUses, KilledInBlock0), case I of #b_set{dst=Dst} -> killsets_is(Is, sets:del_element(Dst, Live), - killsets_add_kills(Dst, LastUses, Kills0), Lbl); + killsets_add_kills(Dst, LastUses, Kills0), + Lbl, KilledInBlock); _ -> killsets_is(Is, Live, killsets_add_kills({terminator,Lbl}, LastUses, Kills0), - Lbl) + Lbl, KilledInBlock) end; -killsets_is([], Live, Kills, _) -> - {Live,Kills}. +killsets_is([], Live, Kills, Lbl, KilledInBlock) -> + {Live,killsets_add_kills({killed_in_block,Lbl}, KilledInBlock, Kills)}. killsets_update_live_and_last_use(Live0, Uses) -> foldl(fun(Use, {LiveAcc,LastAcc}=Acc) -> @@ -372,9 +387,10 @@ aa_fun(F, #opt_st{ssa=Linear0,args=Args}, %% AAS, we use it. For an exported function, all arguments are %% assumed to be aliased. {SS0,Cnt} = aa_init_fun_ss(Args, F, AAS0), - #{F:=Kills} = KillsMap, + #{F:={LiveIns,Kills,PhiLiveIns}} = KillsMap, {SS,#aas{call_args=CallArgs}=AAS} = - aa_blocks(Linear0, Kills, #{0=>SS0}, AAS0#aas{cnt=Cnt}), + aa_blocks(Linear0, LiveIns, PhiLiveIns, + Kills, #{0=>SS0}, AAS0#aas{cnt=Cnt}), AliasMap = AliasMap0#{ F => SS }, PrevSS = maps:get(F, AliasMap0, #{}), Repeats = case PrevSS =/= SS orelse CallArgs0 =/= CallArgs of @@ -390,21 +406,26 @@ aa_fun(F, #opt_st{ssa=Linear0,args=Args}, AAS#aas{alias_map=AliasMap,repeats=Repeats}. %% Main entry point for the alias analysis -aa_blocks([{?EXCEPTION_BLOCK,_}|Bs], Kills, Lbl2SS, AAS) -> +aa_blocks([{?EXCEPTION_BLOCK,_}|Bs], + LiveIns, PhiLiveIns, Kills, Lbl2SS, AAS) -> %% Nothing happening in the exception block can propagate to the %% other block. - aa_blocks(Bs, Kills, Lbl2SS, AAS); -aa_blocks([{L,#b_blk{is=Is0,last=T}}|Bs0], Kills, Lbl2SS0, AAS0) -> + aa_blocks(Bs, LiveIns, PhiLiveIns, Kills, Lbl2SS, AAS); +aa_blocks([{L,#b_blk{is=Is0,last=T}}|Bs0], + LiveIns, PhiLiveIns, Kills, Lbl2SS0, AAS0) -> #{L:=SS0} = Lbl2SS0, ?DP("Block: ~p~nSS:~n~s~n", [L, beam_ssa_ss:dump(SS0)]), {FullSS,AAS1} = aa_is(Is0, SS0, AAS0), #{{live_outs,L}:=LiveOut} = Kills, + #{{killed_in_block,L}:=KilledInBlock} = Kills, {Lbl2SS1,Successors} = aa_terminator(T, FullSS, Lbl2SS0), - PrunedSS = beam_ssa_ss:prune(LiveOut, FullSS), - Lbl2SS2 = aa_add_block_entry_ss(Successors, PrunedSS, Lbl2SS1), + PrunedSS = beam_ssa_ss:prune(LiveOut, KilledInBlock, FullSS), + ?DP("Live out from ~p: ~p~n", [L, sets:to_list(LiveOut)]), + Lbl2SS2 = aa_add_block_entry_ss(Successors, L, PrunedSS, + LiveOut, LiveIns, PhiLiveIns, Lbl2SS1), Lbl2SS = aa_set_block_exit_ss(L, FullSS, Lbl2SS2), - aa_blocks(Bs0, Kills, Lbl2SS, AAS1); -aa_blocks([], _Kills, Lbl2SS, AAS) -> + aa_blocks(Bs0, LiveIns, PhiLiveIns, Kills, Lbl2SS, AAS1); +aa_blocks([], _LiveIns, _PhiLiveIns, _Kills, Lbl2SS, AAS) -> {Lbl2SS,AAS}. aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> @@ -594,11 +615,25 @@ aa_set_block_exit_ss(ThisBlockLbl, SS, Lbl2SS) -> Lbl2SS#{ThisBlockLbl=>SS}. %% Extend the SS valid on entry to the blocks in the list with NewSS. -aa_add_block_entry_ss([?EXCEPTION_BLOCK|BlockLabels], NewSS, Lbl2SS) -> - aa_add_block_entry_ss(BlockLabels, NewSS, Lbl2SS); -aa_add_block_entry_ss([L|BlockLabels], NewSS, Lbl2SS) -> - aa_add_block_entry_ss(BlockLabels, NewSS, aa_merge_ss(L, NewSS, Lbl2SS)); -aa_add_block_entry_ss([], _, Lbl2SS) -> +aa_add_block_entry_ss([?EXCEPTION_BLOCK|BlockLabels], From, NewSS, + LiveOut, LiveIns, PhiLiveIns, Lbl2SS) -> + aa_add_block_entry_ss(BlockLabels, From, NewSS, LiveOut, + LiveIns, PhiLiveIns, Lbl2SS); +aa_add_block_entry_ss([L|BlockLabels], From, + NewSS, LiveOut, LiveIns, PhiLiveIns, Lbl2SS) -> + #{L:=LiveIn} = LiveIns, + PhiLiveIn = case PhiLiveIns of + #{{From,L}:=Vs} -> Vs; + #{} -> sets:new() + end, + AllLiveIn = sets:union(LiveIn, PhiLiveIn), + KilledOnEdge = sets:subtract(LiveOut, AllLiveIn), + ?DP("Killed on edge to ~p: ~p~n", [L, sets:to_list(KilledOnEdge)]), + ?DP("Live on edge to ~p: ~p~n", [L, sets:to_list(AllLiveIn)]), + Pruned = beam_ssa_ss:prune(AllLiveIn, KilledOnEdge, NewSS), + aa_add_block_entry_ss(BlockLabels, From, NewSS, LiveOut, LiveIns, + PhiLiveIns, aa_merge_ss(L, Pruned, Lbl2SS)); +aa_add_block_entry_ss([], _, _, _, _, _, Lbl2SS) -> Lbl2SS. %% Merge two sharing states when traversing the execution graph @@ -804,7 +839,7 @@ aa_update_annotation_for_var(Var, Status, Anno0) -> %% instruction. dies_at(Var, #b_set{dst=Dst}, AAS) -> #aas{caller=Caller,kills=KillsMap} = AAS, - KillMap = map_get(Caller, KillsMap), + {_LiveIns,KillMap,_PhiLiveIns} = map_get(Caller, KillsMap), sets:is_element(Var, map_get(Dst, KillMap)). aa_set_aliased(Args, SS) -> @@ -852,7 +887,7 @@ aa_alias_surviving_args1([], SS, _KillSet) -> %% Return the kill-set for the instruction defining Dst. aa_killset_for_instr(Dst, #aas{caller=Caller,kills=Kills}) -> - KillMap = map_get(Caller, Kills), + {_LiveIns,KillMap,_PhiLiveIns} = map_get(Caller, Kills), map_get(Dst, KillMap). %% Predicate to check if all variables in `Vars` dies at `Where`. diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index 4f45eda555e8..13e0ee36e1e6 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -43,7 +43,7 @@ merge_in_args/3, new/0, new/3, - prune/2, + prune/3, set_call_result/4, set_status/3, variables/1]). @@ -440,37 +440,56 @@ new() -> %%% Throws `too_deep` if the depth of sharing state value chains %%% exceeds SS_DEPTH_LIMIT. %%% --spec prune(sets:set(beam_ssa:b_var()), sharing_state()) -> sharing_state(). -prune(LiveVars, State) -> +-spec prune(sets:set(beam_ssa:b_var()), + sets:set(beam_ssa:b_var()), + sharing_state()) -> sharing_state(). +prune(LiveVars, Killed, State) -> ?assert_state(State), - ?DP("Pruning to ~p~n", [sets:to_list(LiveVars)]), - ?DP("~s", [dump(State)]), - R = prune([{0,V} || V <- sets:to_list(LiveVars)], [], new(), State), - ?DP("Pruned result~n"), - ?DP("~s", [dump(R)]), - ?assert_state(R). - -prune([{Depth0,V}|Wanted], Edges, New0, Old) -> - case beam_digraph:has_vertex(New0, V) of + ?DP("** pruning **~n~s~n", [dump(State)]), + ?DP("pruning to: ~p~n", [sets:to_list(LiveVars)]), + ?DP("killed: ~p~n", [sets:to_list(Killed)]), + case sets:is_empty(LiveVars) of + false -> + Work = [{?SS_DEPTH_LIMIT,K} || K <- sets:to_list(Killed)], + R = prune(Work, Killed, LiveVars, State), + ?DP("Result:~n~s~n", [dump(R)]), + ?assert_state(R); true -> - %% This variable is already added. - prune(Wanted, Edges, New0, Old); - false when Depth0 < ?SS_DEPTH_LIMIT -> - %% This variable has to be kept. Add it to the new graph. - New = add_vertex(New0, V, beam_digraph:vertex(Old, V)), - %% Add all incoming edges to this node. - InEdges = beam_digraph:in_edges(Old, V), - Depth = Depth0 + 1, - InNodes = [{Depth, From} || {From,_,_} <- InEdges], - prune(InNodes ++ Wanted, InEdges ++ Edges, New, Old); + R = new(), + ?DP("Result (nothing killed):~n~s~n", [dump(R)]), + R + end. + +prune([{0,_}|_], _, _, _) -> + throw(too_deep); +prune([{Depth,V}|Work], Killed, LiveVars, State0) -> + case is_safe_to_prune(V, LiveVars, State0) of + true -> + State = beam_digraph:del_vertex(State0, V), + Ins = [{Depth - 1, I} + || I <- beam_digraph:in_neighbours(State0, V)], + prune(Ins++Work, Killed, LiveVars, State); false -> - %% We're in too deep, give up. - throw(too_deep) + prune(Work, Killed, LiveVars, State0) end; -prune([], Edges, New0, _Old) -> - foldl(fun({Src,Dst,Lbl}, New) -> - add_edge(New, Src, Dst, Lbl) - end, New0, Edges). +prune([], _Killed, _LiveVars, State) -> + State. + +is_safe_to_prune(V, LiveVars, State) -> + case sets:is_element(V, LiveVars) of + true -> + false; + false -> + %% Safe to prune if all out-neighbours are safe-to-prune. + case beam_digraph:out_neighbours(State, V) of + [] -> + true; + Outs -> + lists:all(fun(X) -> + is_safe_to_prune(X, LiveVars, State) + end, Outs) + end + end. -spec set_call_result(beam_ssa:b_var(), call_in_arg_status(), sharing_state(), non_neg_integer()) -> diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/ss_depth_limit.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/ss_depth_limit.erl index ec91b57f26fa..c0c714523890 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/ss_depth_limit.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/ss_depth_limit.erl @@ -16,18 +16,21 @@ do(N) -> "-export([f/0]).\n\n", "f() ->\n", "%ssa% fail () when post_ssa_opt ->\n" - "%ssa% ret(X) { unique => [X] }.\n" + "%ssa% ret(Y) { aliased => [Y] }.\n" + " Y = e:f(),\n", " X0 = e:f(),\n", [io_lib:format(" X~p = {X~p,e:f()},~n", [X, X-1]) || X<- lists:seq(1, N)], - io_lib:format(" X~p.~n", [N]) + io_lib:format(" e:f(X~p),~n", [N]), + " Y.\n" ], file:write_file("ss_depth_limit.erl", Data). -endif. f() -> %ssa% fail () when post_ssa_opt -> -%ssa% ret(X) { unique => [X] }. +%ssa% ret(Y) { aliased => [Y] }. + Y = e:f(), X0 = e:f(), X1 = {X0,e:f()}, X2 = {X1,e:f()}, @@ -60,4 +63,5 @@ f() -> X29 = {X28,e:f()}, X30 = {X29,e:f()}, X31 = {X30,e:f()}, - X31. + e:f(X31), + Y. From 9c02c7854c719e0823ff73253a967cb0611164d4 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Fri, 28 Jun 2024 07:05:14 +0200 Subject: [PATCH 13/25] compiler alias analysis: Avoid variable creation for plain values This change relaxes the requirement that every SSA variable has to be present in the sharing state database by allowing plain values to be omitted. For now, only omit creating a variable for values created by instructions known to produce plain values, i.e. no type annotations are used. As variables are no longer, by default, inserted in the database for each instruction by aa_is/3, avoid two database changes when a variable is created and immediately aliased by inserting and aliasing it in one operation. This change reduces the number of vertices processed by beam_ssa_ss:prune/3 by 7% and the number of pruned vertices by 20%. --- lib/compiler/src/beam_ssa_alias.erl | 110 +++++++++++------- lib/compiler/src/beam_ssa_ss.erl | 82 +++++++------ lib/compiler/test/beam_ssa_check_SUITE.erl | 5 + .../no_type_info.erl | 51 ++++++++ 4 files changed, 169 insertions(+), 79 deletions(-) create mode 100644 lib/compiler/test/beam_ssa_check_SUITE_data/no_type_info.erl diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index 5239b6728989..ef9c3a6fdaa2 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -430,13 +430,13 @@ aa_blocks([], _LiveIns, _PhiLiveIns, _Kills, Lbl2SS, AAS) -> aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> ?DP("I: ~p~n", [_I]), - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), {SS, AAS} = case Op of %% Instructions changing the alias status. {bif,Bif} -> - {aa_bif(Dst, Bif, Args, SS1, AAS0), AAS0}; + {aa_bif(Dst, Bif, Args, SS0, AAS0), AAS0}; bs_create_bin -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), case Args of [#b_literal{val=Flag},_,Arg|_] when Flag =:= private_append ; Flag =:= append -> @@ -453,66 +453,80 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> {aa_set_aliased([Dst|Args], SS1), AAS0} end; bs_extract -> - {aa_set_aliased([Dst|Args], SS1), AAS0}; + {aa_set_aliased([Dst|Args], SS0), AAS0}; bs_get_tail -> - {aa_set_aliased([Dst|Args], SS1), AAS0}; + {aa_set_aliased([Dst|Args], SS0), AAS0}; bs_match -> - {aa_set_aliased([Dst|Args], SS1), AAS0}; + {aa_set_aliased([Dst|Args], SS0), AAS0}; bs_start_match -> [_,Bin] = Args, - {aa_set_aliased([Dst,Bin], SS1), AAS0}; + {aa_set_aliased([Dst,Bin], SS0), AAS0}; build_stacktrace -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), %% build_stacktrace can potentially alias anything %% live at this point in the code. We handle it by %% aliasing everything known to us. Touching %% variables which are dead is harmless. {aa_alias_all(SS1), AAS0}; call -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), aa_call(Dst, Args, Anno0, SS1, AAS0); 'catch_end' -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [_Tag,Arg] = Args, {aa_derive_from(Dst, Arg, SS1), AAS0}; extract -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Arg,_] = Args, {aa_derive_from(Dst, Arg, SS1), AAS0}; get_hd -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Arg] = Args, Type = maps:get(0, maps:get(arg_types, Anno0, #{0=>any}), any), {aa_pair_extraction(Dst, Arg, hd, Type, SS1), AAS0}; get_map_element -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Map,_Key] = Args, {aa_map_extraction(Dst, Map, SS1, AAS0), AAS0}; get_tl -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Arg] = Args, Type = maps:get(0, maps:get(arg_types, Anno0, #{0=>any}), any), {aa_pair_extraction(Dst, Arg, tl, Type, SS1), AAS0}; get_tuple_element -> [Arg,Idx] = Args, Types = maps:get(arg_types, Anno0, #{}), - {aa_tuple_extraction(Dst, Arg, Idx, Types, SS1), AAS0}; + {aa_tuple_extraction(Dst, Arg, Idx, Types, SS0), AAS0}; landingpad -> - {aa_set_aliased(Dst, SS1), AAS0}; + {aa_set_aliased(Dst, SS0), AAS0}; make_fun -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Callee|Env] = Args, aa_make_fun(Dst, Callee, Env, SS1, AAS0); peek_message -> - {aa_set_aliased(Dst, SS1), AAS0}; + {aa_set_aliased(Dst, SS0), AAS0}; phi -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), {aa_phi(Dst, Args, SS1, AAS0), AAS0}; put_list -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), Types = aa_map_arg_to_type(Args, maps:get(arg_types, Anno0, #{})), {aa_construct_pair(Dst, Args, Types, SS1, AAS0), AAS0}; put_map -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), {aa_construct_term(Dst, Args, SS1, AAS0), AAS0}; put_tuple -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), Types = aa_map_arg_to_type(Args, maps:get(arg_types, Anno0, #{})), Values = lists:enumerate(0, Args), {aa_construct_tuple(Dst, Values, Types, SS1, AAS0), AAS0}; update_tuple -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), {aa_construct_term(Dst, Args, SS1, AAS0), AAS0}; update_record -> + SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [#b_literal{val=Hint},_Size,Src|Updates] = Args, RecordType = maps:get(arg_types, Anno0, #{}), ?DP("UPDATE RECORD dst: ~p, src: ~p, type:~p~n", @@ -542,45 +556,45 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> %% Instructions which don't change the alias status {float,_} -> - {SS1, AAS0}; + {SS0, AAS0}; {succeeded,_} -> - {SS1, AAS0}; + {SS0, AAS0}; bs_init_writable -> - {SS1, AAS0}; + {SS0, AAS0}; bs_test_tail -> - {SS1, AAS0}; + {SS0, AAS0}; executable_line -> - {SS1, AAS0}; + {SS0, AAS0}; has_map_field -> - {SS1, AAS0}; + {SS0, AAS0}; is_nonempty_list -> - {SS1, AAS0}; + {SS0, AAS0}; is_tagged_tuple -> - {SS1, AAS0}; + {SS0, AAS0}; kill_try_tag -> - {SS1, AAS0}; + {SS0, AAS0}; match_fail -> - {SS1, AAS0}; + {SS0, AAS0}; new_try_tag -> - {SS1, AAS0}; + {SS0, AAS0}; nif_start -> - {SS1, AAS0}; + {SS0, AAS0}; raw_raise -> - {SS1, AAS0}; + {SS0, AAS0}; recv_marker_bind -> - {SS1, AAS0}; + {SS0, AAS0}; recv_marker_clear -> - {SS1, AAS0}; + {SS0, AAS0}; recv_marker_reserve -> - {SS1, AAS0}; + {SS0, AAS0}; recv_next -> - {SS1, AAS0}; + {SS0, AAS0}; remove_message -> - {SS1, AAS0}; + {SS0, AAS0}; resume -> - {SS1, AAS0}; + {SS0, AAS0}; wait_timeout -> - {SS1, AAS0} + {SS0, AAS0} end, ?DP("Post I: ~p.~n~s~n", [_I, beam_ssa_ss:dump(SS)]), aa_is(Is, SS, AAS); @@ -1068,33 +1082,42 @@ aa_bif(Dst, element, [#b_literal{val=Idx},Tuple], SS, _AAS) %% The element bif is always rewritten to a get_tuple_element %% instruction when the index is an integer and the second %% argument is a known to be a tuple. Therefore this code is only - %% reached when the type of is unknown, thus there is no point in - %% trying to provide aa_tuple_extraction/5 with type information. + %% reached when the type of Tuple is unknown, thus there is no + %% point in trying to provide aa_tuple_extraction/5 with type + %% information. aa_tuple_extraction(Dst, Tuple, #b_literal{val=Idx-1}, #{}, SS); -aa_bif(Dst, element, [#b_literal{},Tuple], SS, _AAS) -> +aa_bif(Dst, element, [#b_literal{},Tuple], SS0, _AAS) -> %% This BIF will fail, but in order to avoid any later transforms %% making use of uniqueness, conservatively alias. + SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_set_aliased([Dst,Tuple], SS); -aa_bif(Dst, element, [#b_var{},Tuple], SS, _AAS) -> +aa_bif(Dst, element, [#b_var{},Tuple], SS0, _AAS) -> + SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_set_aliased([Dst,Tuple], SS); -aa_bif(Dst, hd, [Pair], SS, _AAS) -> +aa_bif(Dst, hd, [Pair], SS0, _AAS) -> %% The hd bif is always rewritten to a get_hd instruction when the %% argument is known to be a pair. Therefore this code is only - %% reached when the type of is unknown, thus there is no point in - %% trying to provide aa_pair_extraction/5 with type information. + %% reached when the type of Pair is unknown, thus there is no + %% point in trying to provide aa_pair_extraction/5 with type + %% information. + SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_pair_extraction(Dst, Pair, hd, SS); -aa_bif(Dst, tl, [Pair], SS, _AAS) -> +aa_bif(Dst, tl, [Pair], SS0, _AAS) -> %% The tl bif is always rewritten to a get_tl instruction when the %% argument is known to be a pair. Therefore this code is only - %% reached when the type of is unknown, thus there is no point in - %% trying to provide aa_pair_extraction/5 with type information. + %% reached when the type of Pair is unknown, thus there is no + %% point in trying to provide aa_pair_extraction/5 with type + %% information. + SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_pair_extraction(Dst, Pair, tl, SS); -aa_bif(Dst, map_get, [_Key,Map], SS, AAS) -> +aa_bif(Dst, map_get, [_Key,Map], SS0, AAS) -> + SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_map_extraction(Dst, Map, SS, AAS); -aa_bif(Dst, binary_part, Args, SS, _AAS) -> +aa_bif(Dst, binary_part, Args, SS0, _AAS) -> %% bif:binary_part/{2,3} is the only guard bif which could lead to %% aliasing, it extracts a sub-binary with a reference to its %% argument. + SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_set_aliased([Dst|Args], SS); aa_bif(Dst, Bif, Args, SS, _AAS) -> Arity = length(Args), @@ -1219,7 +1242,7 @@ aa_map_extraction(Dst, Map, SS, AAS) -> aa_alias_inherit_and_alias_if_arg_does_not_die(Dst, Map, SS, AAS)). %% Extracting elements from a tuple. -aa_tuple_extraction(Dst, #b_var{}=Tuple, #b_literal{val=I}, Types, SS) -> +aa_tuple_extraction(Dst, #b_var{}=Tuple, #b_literal{val=I}, Types, SS0) -> TupleType = maps:get(0, Types, any), TypeIdx = I+1, %% In types tuple indices starting at zero. IsPlainValue = case TupleType of @@ -1237,8 +1260,9 @@ aa_tuple_extraction(Dst, #b_var{}=Tuple, #b_literal{val=I}, Types, SS) -> if IsPlainValue -> %% A plain value was extracted, it doesn't change the %% alias status of Dst nor the tuple. - SS; + SS0; true -> + SS = beam_ssa_ss:add_var(Dst, unique, SS0), beam_ssa_ss:extract(Dst, Tuple, I, SS) end; aa_tuple_extraction(_, #b_literal{}, _, _, SS) -> diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index 13e0ee36e1e6..e7bb29db16be 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -122,12 +122,12 @@ derive_from(Dst, Src, State) -> ?DP("Deriving ~p from ~p~nSS:~n~s~n", [Dst, Src, dump(State)]), ?assert_state(State), ?ASSERT(assert_variable_exists(Dst, State)), - ?ASSERT(assert_variable_exists(Src, State)), - case {beam_digraph:vertex(State, Dst),beam_digraph:vertex(State, Src)} of + case {beam_digraph:vertex(State, Dst, unique), + beam_digraph:vertex(State, Src, plain)} of + {_,plain} -> + State; {aliased,_} -> - %% Nothing to do, already aliased. This can happen when - %% handling Phis, no propagation to the source should be - %% done. + %% Nothing to do, already aliased. State; {_,aliased} -> %% The source is aliased, the destination will become aliased. @@ -152,23 +152,27 @@ embed_in(Dst, Elements, State0) -> ?DP("Embedding ~p into ~p~nSS:~n~s~n", [Elements, Dst, dump(State0)]), ?assert_state(State0), ?ASSERT(assert_variable_exists(Dst, State0)), - ?ASSERT([assert_variable_exists(Src, State0) - || {#b_var{},Src} <- Elements]), foldl(fun({Element,Src}, Acc) -> add_embedding(Dst, Src, Element, Acc) end, State0, Elements). add_embedding(Dst, Src, Element, State0) -> ?DP("add_embedding(~p, ~p, ~p, ...)~n", [Dst,Src,Element]), - - %% Create a node for literals as it isn't in the graph. + %% Create a node for literals and plain values as they are not in + %% the graph. The node is needed to record the status of the + %% element. State1 = case Src of plain -> beam_digraph:add_vertex(State0, Src, unique); #b_literal{} -> beam_digraph:add_vertex(State0, Src, unique); _ -> - State0 + case beam_digraph:has_vertex(State0, Src) of + true -> + State0; + false -> + beam_digraph:add_vertex(State0, Src, unique) + end end, %% Create the edge, this is done regardless of the aliasing status @@ -201,10 +205,7 @@ add_embedding(Dst, Src, Element, State0) -> extract(Dst, Src, Element, State) -> ?DP("Extracting ~p[~p] into ~p~n", [Src,Element,Dst]), ?assert_state(State), - ?ASSERT(assert_variable_exists(Dst, State)), - ?ASSERT(assert_variable_exists(Src, State)), - - case beam_digraph:vertex(State, Src) of + case beam_digraph:vertex(State, Src, plain) of aliased -> %% The pair/tuple is aliased, so what is extracted will be aliased. ?assert_state(set_status(Dst, aliased, State)); @@ -215,7 +216,13 @@ extract(Dst, Src, Element, State) -> orelse (Element =:= hd) orelse (Element =:= tl)), ?DP("dst: ~p, src: ~p, e: ~p, out-edges: ~p~n", [Dst, Src, Element, OutEdges]), - extract_element(Dst, Src, Element, OutEdges, State) + extract_element(Dst, Src, Element, OutEdges, State); + plain -> + %% Extracting from a plain value is not possible, but the + %% alias analysis pass can sometimes encounter it when no + %% type information is available. Conservatively set the + %% result to aliased. + ?assert_state(set_status(Dst, aliased, State)) end. %% Note that extract_element/5 will never be given an out edge with a @@ -253,8 +260,9 @@ extract_status_for_element(Element, Src, Dst, State0) -> ?DP(" found embedding~n"), ?DP(" the source is ~p~n", [Var]), ?DP(" SS~n~s~n", [dump(State0)]), - ?DP(" status ~p~n", [beam_digraph:vertex(State0, Var)]), - State = set_status(Dst, beam_digraph:vertex(State0, Var), State0), + ?DP(" status ~p~n", [beam_digraph:vertex(State0, Var, unique)]), + State = set_status(Dst, beam_digraph:vertex(State0, Var, unique), + State0), ?DP(" Returned SS~n~s~n", [dump(State)]), ?assert_state(State); {[], [{Aggregate,_Dst,{extract,E}}]} -> @@ -275,7 +283,7 @@ get_status_of_extracted_element(Aggregate, [First|Rest]=Elements, State) -> %% extracts from unique aggregates. This implies that when the %% chain is traced backwards, no aliased aggregates will be found, %% but in case that invariant is ever broken, assert. - unique = beam_digraph:vertex(State, Aggregate), + ?ASSERT(unique = beam_digraph:vertex(State, Aggregate, unique)), ?DP(" aggregate is unique~n"), InEdges = beam_digraph:in_edges(State, Aggregate), Embeddings = [Src || {Src,_,{embed,E}} <- InEdges, First =:= E], @@ -292,7 +300,7 @@ get_status_of_extracted_element(Aggregate, [First|Rest]=Elements, State) -> end; get_status_of_extracted_element(Aggregate, [], State) -> ?DP(" ~s(~p, [], ...)~n", [?FUNCTION_NAME, Aggregate]), - S = beam_digraph:vertex(State, Aggregate), + S = beam_digraph:vertex(State, Aggregate, unique), ?DP(" bottomed out, status is ~p~n", [S]), S. @@ -323,7 +331,7 @@ forward_status(Main, Other) -> -spec get_status(beam_ssa:b_var(), sharing_state()) -> sharing_status(). get_status(V=#b_var{}, State) -> - beam_digraph:vertex(State, V). + beam_digraph:vertex(State, V, unique). -spec merge(sharing_state(), sharing_state()) -> sharing_state(). merge(StateA, StateB) -> @@ -350,12 +358,7 @@ merge(StateA, StateB) -> merge(Dest, Source, [{V,VStatus}|Vertices], Edges0, Forced) -> Edges = accumulate_edges(V, Source, Edges0), - DestStatus = case beam_digraph:has_vertex(Dest, V) of - true -> - beam_digraph:vertex(Dest, V); - false -> - false - end, + DestStatus = beam_digraph:vertex(Dest, V, false), case {DestStatus,VStatus} of {Status,Status} -> %% Same status in both states. @@ -533,8 +536,7 @@ set_status(#b_var{}=V, Status, State0) -> %% is embedded remains unchanged. ?DP("Setting status of ~p to ~p~n", [V,Status]), - ?ASSERT(assert_variable_exists(V, State0)), - case beam_digraph:vertex(State0, V) of + case beam_digraph:vertex(State0, V, unique) of Status -> %% Status is unchanged. State0; @@ -545,7 +547,7 @@ set_status(#b_var{}=V, Status, State0) -> set_alias([#b_var{}=V|Vars], State0) -> %% TODO: fold into the above - case beam_digraph:vertex(State0, V) of + case beam_digraph:vertex(State0, V, unique) of aliased -> set_alias(Vars, State0); _ -> @@ -658,7 +660,7 @@ merge_in_arg(#b_var{}=V, _Status, 0, State) -> %% element-level aliasing info will be kept for this element. get_status(V, State); merge_in_arg(#b_var{}=V, Status, Cutoff, State) -> - case beam_digraph:vertex(State, V) of + case beam_digraph:vertex(State, V, unique) of aliased -> aliased; unique -> @@ -807,12 +809,19 @@ has_out_edges(V, State) -> -spec assert_state(sharing_state()) -> sharing_state(). assert_state(State) -> + assert_bad_edges(State), assert_aliased_parent_implies_aliased(State), assert_embedded_in_aliased_implies_aliased(State), assert_multiple_embeddings_force_aliasing(State), assert_multiple_extractions_force_aliasing(State), State. +%% Check that we don't have edges between non-existing nodes +assert_bad_edges(State) -> + [{assert_variable_exists(F, State), assert_variable_exists(T, State)} + || {F,T,_} <- beam_digraph:edges(State)]. + + %% Check that extracted and embedded elements of an aliased variable %% are aliased. assert_aliased_parent_implies_aliased(State) -> @@ -826,7 +835,7 @@ assert_apia(Parent, State) -> embed -> true; {embed,_} -> false end], - [case beam_digraph:vertex(State, Child) of + [case beam_digraph:vertex(State, Child, unique) of aliased -> ok; _ -> @@ -843,7 +852,7 @@ assert_embedded_in_aliased_implies_aliased(State) -> assert_eiaia(Embedder, State) -> NotAliased = [ Src || {Src,_,embed} <- beam_digraph:in_edges(State, Embedder), - beam_digraph:vertex(State, Src) =/= aliased], + beam_digraph:vertex(State, Src, unique) =/= aliased], case NotAliased of [] -> State; @@ -861,7 +870,7 @@ assert_multiple_embeddings_force_aliasing(State) -> assert_mefa(V, State) -> NotAliased = [ B || {B,_,embed} <- beam_digraph:out_edges(State, V), - beam_digraph:vertex(State, B) =/= aliased], + beam_digraph:vertex(State, B, unique) =/= aliased], case NotAliased of [_,_|_] -> io:format("Expected ~p to be aliased in:~n~s.~n", [V, dump(State)]), @@ -887,7 +896,8 @@ assert_mxfa(V, State) -> end, #{}, beam_digraph:out_edges(State, V)), Bad = maps:fold( fun(_Elem, [_,_|_]=Vars, Acc) -> - [X || X <- Vars, beam_digraph:vertex(State, X) =/= aliased] + [X || X <- Vars, + beam_digraph:vertex(State, X, unique) =/= aliased] ++ Acc; (_, _, Acc) -> Acc @@ -906,7 +916,7 @@ assert_variable_exists(plain, State) -> io:format("Expected ~p in~n ~s.~n", [plain, dump(State)]), throw(assertion_failure); _ -> - case beam_digraph:vertex(State, plain) of + case beam_digraph:vertex(State, plain, unique) of unique -> State; Other -> io:format("Expected plain to be unique, was ~p, in:~n~p~n", @@ -938,7 +948,7 @@ dump(State) -> [Id,V,beam_digraph:vertex(State, V)]) || V:=Id <- V2Id], [ io_lib:format(" ~p -> ~p [label=\"~p\"]~n", - [maps:get(From, V2Id), + [maps:get(From, V2Id, plain), maps:get(To, V2Id), Lbl]) || {From,To,Lbl} <- beam_digraph:edges(State)], diff --git a/lib/compiler/test/beam_ssa_check_SUITE.erl b/lib/compiler/test/beam_ssa_check_SUITE.erl index 54baeb07f286..b1ddcb93492a 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE.erl @@ -33,6 +33,7 @@ appendable_checks/1, bs_size_unit_checks/1, no_reuse_hint_checks/1, + no_type_info_checks/1, private_append_checks/1, ret_annotation_checks/1, sanity_checks/1, @@ -49,6 +50,7 @@ groups() -> annotation_checks, appendable_checks, no_reuse_hint_checks, + no_type_info_checks, private_append_checks, ret_annotation_checks, sanity_checks, @@ -104,6 +106,9 @@ bs_size_unit_checks(Config) when is_list(Config) -> no_reuse_hint_checks(Config) when is_list(Config) -> run_post_ssa_opt(no_reuse_hint, Config). +no_type_info_checks(Config) when is_list(Config) -> + run_post_ssa_opt(no_type_info, Config). + private_append_checks(Config) when is_list(Config) -> run_post_ssa_opt(private_append, Config). diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/no_type_info.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/no_type_info.erl new file mode 100644 index 000000000000..22505ee8eda6 --- /dev/null +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/no_type_info.erl @@ -0,0 +1,51 @@ +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2024. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% %CopyrightEnd% +%% +%% This module tests functions which have previously crashed the +%% compiler when the `no_type_opt` option was used. +%% + +-module(no_type_info). +-export([bug/0]). + +-compile(no_type_opt). + +bug() -> +%ssa% () when post_ssa_opt -> +%ssa% X = bif:hd(L) { unique => [L] }, +%ssa% _ = succeeded:body(X) { aliased => [X] }. + begin + + <<42 || + $s <- + try + something + catch + error:false -> + [] + end + >> + end:(hd(not girl))( + try home of + _ when 34 -> + 8 + catch + _:_ -> + whatever + after + ok + end). From 651032338bd5de2b4fc0f3ee1c9ee7745d7cd582 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 1 Jul 2024 13:21:09 +0200 Subject: [PATCH 14/25] compiler alias analysis: Avoid redundant database changes This is a micro optimization which avoids creating a variable in the sharing database when, during the processing of the instruction, we will immediately update its status. This reduces the number of sharing database operations as we can insert and set the status in a single operation. It also allows us to completely avoid creating a variable for extracted plain values. --- lib/compiler/src/beam_ssa_alias.erl | 62 +++++++++++------------------ lib/compiler/src/beam_ssa_ss.erl | 18 +++++---- 2 files changed, 34 insertions(+), 46 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index ef9c3a6fdaa2..32260224bbea 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -436,21 +436,20 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> {bif,Bif} -> {aa_bif(Dst, Bif, Args, SS0, AAS0), AAS0}; bs_create_bin -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), case Args of [#b_literal{val=Flag},_,Arg|_] when Flag =:= private_append ; Flag =:= append -> case aa_all_dies([Arg], Dst, AAS0) of true -> %% Inherit the status of the argument - {aa_derive_from(Dst, Arg, SS1), AAS0}; + {aa_derive_from(Dst, Arg, SS0), AAS0}; false -> %% We alias with the surviving arg - {aa_set_aliased([Dst|Args], SS1), AAS0} + {aa_set_aliased([Dst|Args], SS0), AAS0} end; _ -> %% TODO: Too conservative? - {aa_set_aliased([Dst|Args], SS1), AAS0} + {aa_set_aliased([Dst|Args], SS0), AAS0} end; bs_extract -> {aa_set_aliased([Dst|Args], SS0), AAS0}; @@ -472,27 +471,22 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), aa_call(Dst, Args, Anno0, SS1, AAS0); 'catch_end' -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [_Tag,Arg] = Args, - {aa_derive_from(Dst, Arg, SS1), AAS0}; + {aa_derive_from(Dst, Arg, SS0), AAS0}; extract -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Arg,_] = Args, - {aa_derive_from(Dst, Arg, SS1), AAS0}; + {aa_derive_from(Dst, Arg, SS0), AAS0}; get_hd -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Arg] = Args, Type = maps:get(0, maps:get(arg_types, Anno0, #{0=>any}), any), - {aa_pair_extraction(Dst, Arg, hd, Type, SS1), AAS0}; + {aa_pair_extraction(Dst, Arg, hd, Type, SS0), AAS0}; get_map_element -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Map,_Key] = Args, - {aa_map_extraction(Dst, Map, SS1, AAS0), AAS0}; + {aa_map_extraction(Dst, Map, SS0, AAS0), AAS0}; get_tl -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [Arg] = Args, Type = maps:get(0, maps:get(arg_types, Anno0, #{0=>any}), any), - {aa_pair_extraction(Dst, Arg, tl, Type, SS1), AAS0}; + {aa_pair_extraction(Dst, Arg, tl, Type, SS0), AAS0}; get_tuple_element -> [Arg,Idx] = Args, Types = maps:get(arg_types, Anno0, #{}), @@ -506,16 +500,14 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> peek_message -> {aa_set_aliased(Dst, SS0), AAS0}; phi -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), - {aa_phi(Dst, Args, SS1, AAS0), AAS0}; + {aa_phi(Dst, Args, SS0, AAS0), AAS0}; put_list -> SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), Types = aa_map_arg_to_type(Args, maps:get(arg_types, Anno0, #{})), {aa_construct_pair(Dst, Args, Types, SS1, AAS0), AAS0}; put_map -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), - {aa_construct_term(Dst, Args, SS1, AAS0), AAS0}; + {aa_construct_term(Dst, Args, SS0, AAS0), AAS0}; put_tuple -> SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), Types = aa_map_arg_to_type(Args, @@ -523,8 +515,7 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> Values = lists:enumerate(0, Args), {aa_construct_tuple(Dst, Values, Types, SS1, AAS0), AAS0}; update_tuple -> - SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), - {aa_construct_term(Dst, Args, SS1, AAS0), AAS0}; + {aa_construct_term(Dst, Args, SS0, AAS0), AAS0}; update_record -> SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), [#b_literal{val=Hint},_Size,Src|Updates] = Args, @@ -1086,32 +1077,27 @@ aa_bif(Dst, element, [#b_literal{val=Idx},Tuple], SS, _AAS) %% point in trying to provide aa_tuple_extraction/5 with type %% information. aa_tuple_extraction(Dst, Tuple, #b_literal{val=Idx-1}, #{}, SS); -aa_bif(Dst, element, [#b_literal{},Tuple], SS0, _AAS) -> +aa_bif(Dst, element, [#b_literal{},Tuple], SS, _AAS) -> %% This BIF will fail, but in order to avoid any later transforms %% making use of uniqueness, conservatively alias. - SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_set_aliased([Dst,Tuple], SS); -aa_bif(Dst, element, [#b_var{},Tuple], SS0, _AAS) -> - SS = beam_ssa_ss:add_var(Dst, unique, SS0), +aa_bif(Dst, element, [#b_var{},Tuple], SS, _AAS) -> aa_set_aliased([Dst,Tuple], SS); -aa_bif(Dst, hd, [Pair], SS0, _AAS) -> +aa_bif(Dst, hd, [Pair], SS, _AAS) -> %% The hd bif is always rewritten to a get_hd instruction when the %% argument is known to be a pair. Therefore this code is only %% reached when the type of Pair is unknown, thus there is no %% point in trying to provide aa_pair_extraction/5 with type %% information. - SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_pair_extraction(Dst, Pair, hd, SS); -aa_bif(Dst, tl, [Pair], SS0, _AAS) -> +aa_bif(Dst, tl, [Pair], SS, _AAS) -> %% The tl bif is always rewritten to a get_tl instruction when the %% argument is known to be a pair. Therefore this code is only %% reached when the type of Pair is unknown, thus there is no %% point in trying to provide aa_pair_extraction/5 with type %% information. - SS = beam_ssa_ss:add_var(Dst, unique, SS0), aa_pair_extraction(Dst, Pair, tl, SS); -aa_bif(Dst, map_get, [_Key,Map], SS0, AAS) -> - SS = beam_ssa_ss:add_var(Dst, unique, SS0), +aa_bif(Dst, map_get, [_Key,Map], SS, AAS) -> aa_map_extraction(Dst, Map, SS, AAS); aa_bif(Dst, binary_part, Args, SS0, _AAS) -> %% bif:binary_part/{2,3} is the only guard bif which could lead to @@ -1215,10 +1201,10 @@ aa_pair_extraction(Dst, Pair, Element, SS) -> aa_pair_extraction(Dst, #b_var{}=Pair, Element, Type, SS) -> IsPlainValue = case {Type,Element} of - {#t_cons{type=Ty},hd} -> - aa_is_plain_type(Ty); {#t_cons{terminator=Ty},tl} -> aa_is_plain_type(Ty); + {#t_cons{type=Ty},hd} -> + aa_is_plain_type(Ty); _ -> %% There is no type information, %% conservatively assume this isn't a plain @@ -1242,11 +1228,11 @@ aa_map_extraction(Dst, Map, SS, AAS) -> aa_alias_inherit_and_alias_if_arg_does_not_die(Dst, Map, SS, AAS)). %% Extracting elements from a tuple. -aa_tuple_extraction(Dst, #b_var{}=Tuple, #b_literal{val=I}, Types, SS0) -> +aa_tuple_extraction(Dst, #b_var{}=Tuple, #b_literal{val=I}, Types, SS) -> TupleType = maps:get(0, Types, any), TypeIdx = I+1, %% In types tuple indices starting at zero. IsPlainValue = case TupleType of - #t_tuple{elements=#{TypeIdx:=T}} -> + #t_tuple{elements=#{TypeIdx:=T}} when T =/= any -> aa_is_plain_type(T); _ -> %% There is no type information, @@ -1257,12 +1243,12 @@ aa_tuple_extraction(Dst, #b_var{}=Tuple, #b_literal{val=I}, Types, SS0) -> ?DP("tuple-extraction dst:~p, tuple: ~p, idx: ~p,~n" " type: ~p,~n plain: ~p~n", [Dst, Tuple, I, TupleType, IsPlainValue]), - if IsPlainValue -> + case IsPlainValue of + true -> %% A plain value was extracted, it doesn't change the %% alias status of Dst nor the tuple. - SS0; - true -> - SS = beam_ssa_ss:add_var(Dst, unique, SS0), + SS; + false -> beam_ssa_ss:extract(Dst, Tuple, I, SS) end; aa_tuple_extraction(_, #b_literal{}, _, _, SS) -> diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index e7bb29db16be..fd4a7fb28981 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -121,7 +121,6 @@ add_edge(State, Src, Dst, Lbl) -> derive_from(Dst, Src, State) -> ?DP("Deriving ~p from ~p~nSS:~n~s~n", [Dst, Src, dump(State)]), ?assert_state(State), - ?ASSERT(assert_variable_exists(Dst, State)), case {beam_digraph:vertex(State, Dst, unique), beam_digraph:vertex(State, Src, plain)} of {_,plain} -> @@ -142,7 +141,13 @@ derive_from(Dst, Src, State) -> false -> %% Source is not aliased and has not been embedded %% in a term, record that it now is. - ?assert_state(add_edge(State, Src, Dst, embed)) + State1 = case beam_digraph:has_vertex(State, Dst) of + true -> + State; + false -> + beam_ssa_ss:add_var(Dst, unique, State) + end, + ?assert_state(add_edge(State1, Src, Dst, embed)) end end. @@ -240,7 +245,8 @@ extract_element(Dst, Src, Element, [], State0) -> %% aliased (checked in extract/4). It could be that we're about to %% extract an element which is known to be aliased. ?DP(" the element has not been extracted so far~n"), - State = ?assert_state(add_edge(State0, Src, Dst, {extract,Element})), + State1 = beam_ssa_ss:add_var(Dst, unique, State0), + State = ?assert_state(add_edge(State1, Src, Dst, {extract,Element})), extract_status_for_element(Element, Src, Dst, State). extract_status_for_element(Element, Src, Dst, State0) -> @@ -266,11 +272,7 @@ extract_status_for_element(Element, Src, Dst, State0) -> ?DP(" Returned SS~n~s~n", [dump(State)]), ?assert_state(State); {[], [{Aggregate,_Dst,{extract,E}}]} -> - %% We are trying extract from an extraction. The database - %% keeps no information about the strucure of the - %% extracted term. We have to conservatively set the - %% status of Dst to aliased. - + %% We are trying extract from an extraction. S = get_status_of_extracted_element(Aggregate, [E,Element], State0), ?DP(" status of ~p will be ~p~n", [Dst, S]), ?assert_state(set_status(Dst, S, State0)) From b046f24bf43a3e7fe7f7f7e03abcd4ac8b7b04c0 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Tue, 23 Jul 2024 08:32:22 +0200 Subject: [PATCH 15/25] compiler alias analysis: See through embed-extract chains Consider embed-extract chains when incorporating information about the aliasing status of function arguments. --- lib/compiler/src/beam_ssa_ss.erl | 17 +++++++++------ .../test/beam_ssa_check_SUITE_data/alias.erl | 21 ++++++++++++++++++- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index fd4a7fb28981..c1c9cb242768 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -750,15 +750,20 @@ merge_elements([{Src,_,{embed,Idx}}|Rest], Elements0, Cutoff, State) when merge_elements(Rest, Elements, Cutoff, State); merge_elements([{_Src,_,embed}|Rest], _Elements0, Cutoff, State) -> %% We don't know where this element is embedded. Src will always - %% be unique as otherwise erge_in_arg/4 will not bother merging + %% be unique as otherwise merge_in_arg/4 will not bother merging %% the in-edges. ?ASSERT(unique = get_status(_Src, State)), merge_elements(Rest, no_info, Cutoff, State); -merge_elements([{_,V,{extract,_}}|_Rest], _Elements0, _, State) -> - %% For now we don't try to derive the structure of this argument - %% further. - %% TODO: Revisit the decision above. - get_status(V, State). +merge_elements([{Src,V,{extract,E}}], Elements, Cutoff, State) -> + ?DP("Looking for an embedding of the ~p element from ~p~n", [E, Src]), + InEdges = beam_digraph:in_edges(State, Src), + case [Other || {Other,_,{embed,EdgeOp}} <- InEdges, EdgeOp =:= E] of + [Next] -> + ?DP("Found: ~p~n", [Next]), + merge_in_arg(Next, {unique,Elements}, Cutoff, State); + [] -> + get_status(V, State) + end. -spec new([beam_ssa:b_var()], call_in_arg_info(), non_neg_integer()) -> sharing_state(). diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl index 5d71a4159428..55400ccd9d27 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl @@ -110,7 +110,9 @@ nested_tuple/0, nested_cons/0, - nested_mixed/0]). + nested_mixed/0, + + see_through/0]). %% Trivial smoke test transformable0(L) -> @@ -1194,3 +1196,20 @@ nested_mixed() -> %ssa% ret(R). [{[{Z,X}]}] = nested_mixed_inner(), {<>,X}. + +%% +%% Check that the analysis can see through embed-extract chains. +%% +-record(see_through, {a,b}). + +see_through() -> + [R] = see_through0(), + see_through1(R). + +see_through1({_,R}) -> +%ssa% (_) when post_ssa_opt -> +%ssa% _ = update_record(reuse, 3, Rec, _, _) {unique => [Rec], source_dies => true}. + R#see_through{a=e:f()}. + +see_through0() -> + [{foo, #see_through{a={bar, [foo]}}}]. From e775560d84f99a401f025e294a1a8228af490452 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Tue, 23 Jul 2024 08:32:57 +0200 Subject: [PATCH 16/25] compiler alias analysis: Introduce explicit no_info alias status Previously the status of a variable in the sharing state database was either `unique` or `aliased`. This patch extends the set of statuses with a third: `no_info`, meaning that nothing is known about the alias status of the variable, nor of its structure. This new status will be used in future changes in order to allow for faster and more efficient calculation of the alias status of a module. --- lib/compiler/src/beam_ssa_alias.erl | 5 +- lib/compiler/src/beam_ssa_ss.erl | 72 +++++++++++++++++++++-------- 2 files changed, 56 insertions(+), 21 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index 32260224bbea..502f4c33d9a0 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -825,7 +825,10 @@ aa_update_annotation_for_var(Var, Status, Anno0) -> ordsets:del_element(Var, Unique0)}; unique -> {ordsets:del_element(Var, Aliased0), - ordsets:add_element(Var, Unique0)} + ordsets:add_element(Var, Unique0)}; + no_info -> + {ordsets:del_element(Var, Aliased0), + ordsets:del_element(Var, Unique0)} end, Anno1 = case Aliased of [] -> diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index c1c9cb242768..088c81fcc9a2 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -78,7 +78,7 @@ -endif. -type sharing_state() :: any(). % A beam_digraph graph. --type sharing_status() :: 'unique' | 'aliased'. +-type sharing_status() :: 'unique' | 'aliased' | 'no_info'. -type element() :: 'hd' | 'tl' | non_neg_integer(). -spec add_var(beam_ssa:b_var(), sharing_status(), sharing_state()) -> @@ -222,6 +222,8 @@ extract(Dst, Src, Element, State) -> ?DP("dst: ~p, src: ~p, e: ~p, out-edges: ~p~n", [Dst, Src, Element, OutEdges]), extract_element(Dst, Src, Element, OutEdges, State); + no_info -> + ?assert_state(set_status(Dst, no_info, State)); plain -> %% Extracting from a plain value is not possible, but the %% alias analysis pass can sometimes encounter it when no @@ -376,7 +378,14 @@ merge(Dest, Source, [{V,VStatus}|Vertices], Edges0, Forced) -> {aliased,unique} -> %% V has to be revisited and non-aliased copied parts will %% be aliased. - merge(Dest, Source, Vertices, Edges, sets:add_element(V, Forced)) + merge(Dest, Source, Vertices, Edges, sets:add_element(V, Forced)); + {aliased,no_info} -> + %% Nothing to do. + merge(Dest, Source, Vertices, Edges, Forced); + {no_info,aliased} -> + %% Alias in Dest. + merge(set_status(V, aliased, Dest), Source, + Vertices, Edges, Forced) end; merge(Dest0, _Source, [], Edges, Forced) -> merge1(Dest0, _Source, sets:to_list(Edges), @@ -544,7 +553,12 @@ set_status(#b_var{}=V, Status, State0) -> State0; unique when Status =:= aliased -> State = add_vertex(State0, V, Status), - set_alias(get_alias_edges(V, State), State) + set_alias(get_alias_edges(V, State), State); + no_info when Status =/= no_info -> + add_vertex(State0, V, Status); + unique when Status =:= no_info -> + %% Can only be used safely for newly created variables. + add_vertex(State0, V, Status) end. set_alias([#b_var{}=V|Vars], State0) -> @@ -657,10 +671,26 @@ merge_in_arg(_, aliased, _, _State) -> aliased; merge_in_arg(plain, _, _, _State) -> unique; -merge_in_arg(#b_var{}=V, _Status, 0, State) -> +merge_in_arg(#b_var{}=V, Status, 0, State) -> %% We will not traverse this argument further, this means that no %% element-level aliasing info will be kept for this element. - get_status(V, State); + case {Status, get_status(V, State)} of + {S,S} -> + S; + {no_info,S} -> + S; + {S,no_info} -> + S; + {_,aliased} -> + aliased + end; +merge_in_arg(#b_var{}=V, unique, _Cutoff, State) -> + case beam_digraph:vertex(State, V, unique) of + no_info -> + unique; + S -> + S + end; merge_in_arg(#b_var{}=V, Status, Cutoff, State) -> case beam_digraph:vertex(State, V, unique) of aliased -> @@ -668,17 +698,18 @@ merge_in_arg(#b_var{}=V, Status, Cutoff, State) -> unique -> InEdges = beam_digraph:in_edges(State, V), Elements = case Status of - unique -> #{}; {unique,Es} -> Es; no_info -> #{} end, - merge_elements(InEdges, Elements, Cutoff, State) + merge_elements(InEdges, Elements, Cutoff, State); + no_info -> + Status end; -merge_in_arg(#b_literal{}, _, 0, _State) -> +merge_in_arg(#b_literal{}, Status, 0, _State) -> %% We have reached the cutoff while traversing a larger construct, - %% as we're not looking deeper down into the structure we indicate - %% that we have no information. - no_info; + %% as we're not looking deeper down into the structure we stop + %% constructing detailed information. + Status; merge_in_arg(#b_literal{val=[Hd|Tl]}, Status, Cutoff, State) -> {HdS,TlS,Elements0} = case Status of {unique,#{hd:=HdS0,tl:=TlS0}=All} -> @@ -694,12 +725,15 @@ merge_in_arg(#b_literal{val=[Hd|Tl]}, Status, Cutoff, State) -> {unique,Elements}; merge_in_arg(#b_literal{val=[]}, Status, _, _State) -> Status; +merge_in_arg(#b_literal{val=T}, unique, _Cutoff, _State) when is_tuple(T) -> + %% The uniqe status cannot be safely upgraded to a more detailed + %% info. + unique; merge_in_arg(#b_literal{val=T}, Status, Cutoff, State) when is_tuple(T) -> SrcElements = tuple_to_list(T), OrigElements = case Status of {unique,TupleElems} -> TupleElems; - unique -> #{}; no_info -> #{} end, Elements = merge_tuple_elems(SrcElements, OrigElements, Cutoff, State), @@ -752,7 +786,8 @@ merge_elements([{_Src,_,embed}|Rest], _Elements0, Cutoff, State) -> %% We don't know where this element is embedded. Src will always %% be unique as otherwise merge_in_arg/4 will not bother merging %% the in-edges. - ?ASSERT(unique = get_status(_Src, State)), + ?ASSERT(true = get_status(_Src, State) =:= unique + orelse get_status(_Src, State) =:= no_info), merge_elements(Rest, no_info, Cutoff, State); merge_elements([{Src,V,{extract,E}}], Elements, Cutoff, State) -> ?DP("Looking for an embedding of the ~p element from ~p~n", [E, Src]), @@ -773,12 +808,8 @@ new(Args, ArgsInfo, Cnt) -> ?assert_state(SS), R. -new([A|As], [S0|Stats], Cnt, SS) - when S0 =:= aliased; S0 =:= unique; S0 =:= no_info -> - S = case S0 of - no_info -> unique; - _ -> S0 - end, +new([A|As], [S|Stats], Cnt, SS) + when S =:= aliased; S =:= unique; S =:= no_info -> new(As, Stats, Cnt, add_var(A, S, SS)); new([A|As], [{unique,Elements}|Stats], Cnt0, SS0) -> SS1 = add_var(A, unique, SS0), @@ -859,7 +890,8 @@ assert_embedded_in_aliased_implies_aliased(State) -> assert_eiaia(Embedder, State) -> NotAliased = [ Src || {Src,_,embed} <- beam_digraph:in_edges(State, Embedder), - beam_digraph:vertex(State, Src, unique) =/= aliased], + beam_digraph:vertex(State, Src, unique) =/= aliased, + beam_digraph:vertex(State, Src, unique) =/= no_info], case NotAliased of [] -> State; From b2d865413a4c21c78dc0a8f43c7db1d4757555e5 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 22 Jul 2024 15:08:06 +0200 Subject: [PATCH 17/25] compiler alias analysis: Improve fixpoint iteration strategy Switch from a dumb fixpoint iteration where we just iterated until the sharing state for a all functions stopped changing, to a more intelligent strategy where we only consider changes that are visible outside the function. Concretely that means that we only re-analyze callers if the callee's result's status changed. And callees if argument aliasing status has changed. The new strategy leads to a test failure (marked xfail), but that limitiation will be removed in a later patch. --- lib/compiler/src/beam_ssa_alias.erl | 128 ++++++++++-------- .../test/beam_ssa_check_SUITE_data/alias.erl | 2 +- 2 files changed, 75 insertions(+), 55 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index 502f4c33d9a0..d9f591c7b55d 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -347,63 +347,61 @@ aa(Funs, KillsMap, StMap, FuncDb) -> %%% to detect incomplete information in a hypothetical %%% ssa_opt_alias_finish pass. %%% -aa_fixpoint(Funs, AAS=#aas{alias_map=AliasMap,call_args=CallArgs, - func_db=FuncDb}) -> +aa_fixpoint(Funs, AAS=#aas{func_db=FuncDb}) -> Order = aa_reverse_post_order(Funs, FuncDb), - aa_fixpoint(Order, Order, AliasMap, CallArgs, AAS, ?MAX_REPETITIONS). + aa_fixpoint(Order, Order, AAS, ?MAX_REPETITIONS). + +aa_fixpoint([F|Fs], Order, AAS0=#aas{st_map=StMap,repeats=Repeats}, Limit) -> -aa_fixpoint([F|Fs], Order, OldAliasMap, OldCallArgs, AAS0=#aas{st_map=StMap}, - Limit) -> - AAS1 = AAS0#aas{caller=F}, ?DP("-= ~s =-~n", [fn(F)]), + %% If, when analysing another function, this function has been + %% scheduled for revisiting, we can remove it, as we otherwise + %% revisit it again in the next iteration. + AAS1 = AAS0#aas{caller=F,repeats=sets:del_element(F, Repeats)}, + St = #opt_st{ssa=_Is} = map_get(F, StMap), ?DP("code:~n~p.~n", [_Is]), AAS = aa_fun(F, St, AAS1), ?DP("Done ~s~n", [fn(F)]), - aa_fixpoint(Fs, Order, OldAliasMap, OldCallArgs, AAS, Limit); -aa_fixpoint([], Order, OldAliasMap, OldCallArgs, - #aas{alias_map=OldAliasMap,call_args=OldCallArgs, - func_db=FuncDb}=AAS, _Limit) -> - ?DP("**** End of iteration ~p ****~n", [_Limit]), - {StMap,_} = aa_update_annotations(Order, AAS), - {StMap, FuncDb}; -aa_fixpoint([], _, _, _, #aas{func_db=FuncDb,orig_st_map=StMap}, 0) -> + aa_fixpoint(Fs, Order, AAS, Limit); +aa_fixpoint([], _, #aas{func_db=FuncDb,orig_st_map=StMap}, 0) -> ?DP("**** End of iteration, too many iterations ****~n"), {StMap, FuncDb}; -aa_fixpoint([], Order, _OldAliasMap, _OldCallArgs, - #aas{alias_map=AliasMap,call_args=CallArgs,repeats=Repeats}=AAS, - Limit) -> - ?DP("**** Things have changed, starting next iteration ****~n"), +aa_fixpoint([], Order, #aas{func_db=FuncDb,repeats=Repeats}=AAS, Limit) -> %% Following the depth first order, select those in Repeats. - NewOrder = [Id || Id <- Order, sets:is_element(Id, Repeats)], - aa_fixpoint(NewOrder, Order, AliasMap, CallArgs, - AAS#aas{repeats=sets:new()}, Limit - 1). + case [Id || Id <- Order, sets:is_element(Id, Repeats)] of + [] -> + ?DP("**** Fixpoint reached after ~p iterations ****~n", + [?MAX_REPETITIONS - Limit]), + {StMap,_} = aa_update_annotations(Order, AAS), + {StMap, FuncDb}; + NewOrder -> + ?DP("**** Starting iteration ~p ****~n", + [?MAX_REPETITIONS - Limit + 1]), + aa_fixpoint(NewOrder, Order, AAS#aas{repeats=sets:new()}, Limit - 1) + end. aa_fun(F, #opt_st{ssa=Linear0,args=Args}, - AAS0=#aas{alias_map=AliasMap0,call_args=CallArgs0, - func_db=FuncDb,kills=KillsMap,repeats=Repeats0}) -> + AAS0=#aas{alias_map=AliasMap0,kills=KillsMap}) -> %% Initially assume all formal parameters are unique for a %% non-exported function, if we have call argument info in the %% AAS, we use it. For an exported function, all arguments are %% assumed to be aliased. {SS0,Cnt} = aa_init_fun_ss(Args, F, AAS0), #{F:={LiveIns,Kills,PhiLiveIns}} = KillsMap, - {SS,#aas{call_args=CallArgs}=AAS} = - aa_blocks(Linear0, LiveIns, PhiLiveIns, - Kills, #{0=>SS0}, AAS0#aas{cnt=Cnt}), + {SS,AAS1} = aa_blocks(Linear0, LiveIns, PhiLiveIns, + Kills, #{0=>SS0}, AAS0#aas{cnt=Cnt}), + Lbl2SS0 = maps:get(F, AliasMap0, #{}), + Type2Status0 = maps:get(returns, Lbl2SS0, #{}), + Type2Status = maps:get(returns, SS, #{}), + AAS = case Type2Status0 =/= Type2Status of + true -> + aa_schedule_revisit_callers(F, AAS1); + false -> + AAS1 + end, AliasMap = AliasMap0#{ F => SS }, - PrevSS = maps:get(F, AliasMap0, #{}), - Repeats = case PrevSS =/= SS orelse CallArgs0 =/= CallArgs of - true -> - %% Alias status has changed, so schedule both - %% our callers and callees for renewed analysis. - #{ F := #func_info{in=In,out=Out} } = FuncDb, - foldl(fun sets:add_element/2, - foldl(fun sets:add_element/2, Repeats0, Out), In); - false -> - Repeats0 - end, - AAS#aas{alias_map=AliasMap,repeats=Repeats}. + AAS#aas{alias_map=AliasMap}. %% Main entry point for the alias analysis aa_blocks([{?EXCEPTION_BLOCK,_}|Bs], @@ -1160,9 +1158,12 @@ aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, ?DP("~s~n", [beam_ssa_ss:dump(SS)]), {SS, AAS#aas{cnt=Cnt}}; false -> + ?DP(" The callee is unknown~n"), %% We don't know anything about the function, don't change - %% the status of any variables - {SS0, AAS0} + %% the status of any variables, but make sure that it will + %% be visited. + + {SS0, aa_schedule_revisit(Callee, AAS0)} end; aa_call(_Dst, [#b_remote{mod=#b_literal{val=erlang}, name=#b_literal{val=exit}, @@ -1186,10 +1187,15 @@ aa_add_call_info(Callee, Args, SS0, " args: ~p.~n ss:~n~s.~n", [fn(Callee), fn(_Caller), Args, beam_ssa_ss:dump(SS0)]), InStatus = beam_ssa_ss:merge_in_args(Args, InStatus0, SS0), - ?DP(" orig in-info: ~p.~n", [InStatus0]), + ?DP(" orig in-info:~n ~p.~n", [InStatus0]), ?DP(" updated in-info for ~s:~n ~p.~n", [fn(Callee), InStatus]), - InInfo = InInfo0#{Callee => InStatus}, - AAS#aas{call_args=InInfo}. + case InStatus0 =/= InStatus of + true -> + InInfo = InInfo0#{Callee => InStatus}, + aa_schedule_revisit(Callee, AAS#aas{call_args=InInfo}); + false -> + AAS + end. aa_init_fun_ss(Args, FunId, #aas{call_args=Info,st_map=StMap}) -> #{FunId:=ArgsStatus} = Info, @@ -1258,8 +1264,7 @@ aa_tuple_extraction(_, #b_literal{}, _, _, SS) -> SS. aa_make_fun(Dst, Callee=#b_local{name=#b_literal{}}, - Env0, SS0, - AAS0=#aas{call_args=Info0,repeats=Repeats0}) -> + Env0, SS0, AAS0=#aas{call_args=Info0}) -> %% When a value is copied into the environment of a fun we assume %% that it has been aliased as there is no obvious way to track %% and ensure that the value is only used once, even if the @@ -1273,16 +1278,15 @@ aa_make_fun(Dst, Callee=#b_local{name=#b_literal{}}, Status = [aliased || _ <- Status0], #{ Callee := PrevStatus } = Info0, Info = Info0#{ Callee := Status }, - Repeats = case PrevStatus =/= Status of - true -> - %% We have new information for the callee, we - %% have to revisit it. - sets:add_element(Callee, Repeats0); - false -> - Repeats0 + AAS1 = case PrevStatus =/= Status of + true -> + %% We have new information for the callee, we + %% have to revisit it. + aa_schedule_revisit(Callee, AAS0); + false -> + AAS0 end, - AAS = AAS0#aas{call_args=Info,repeats=Repeats}, - {SS, AAS}. + {SS, AAS1#aas{call_args=Info}}. aa_reverse_post_order(Funs, FuncDb) -> %% In order to produce a reverse post order of the call graph, we @@ -1329,6 +1333,22 @@ aa_reverse_post_order([], [], _Seen, _FuncDb) -> aa_reverse_post_order([], Next, Seen, FuncDb) -> aa_reverse_post_order(Next, [], Seen, FuncDb). +%% Schedule a function for revisting. +aa_schedule_revisit(FuncId, + #aas{func_db=FuncDb,st_map=StMap,repeats=Repeats}=AAS) + when is_map_key(FuncId, FuncDb), is_map_key(FuncId, StMap) -> + ?DP("Scheduling ~s for revisit~n", [fn(FuncId)]), + AAS#aas{repeats=sets:add_element(FuncId, Repeats)}; +aa_schedule_revisit(_, AAS) -> + AAS. + +%% Schedule the callers of a function for revisting. +aa_schedule_revisit_callers(FuncId, #aas{func_db=FuncDb}=AAS) -> + #{FuncId:=#func_info{in=In}} = FuncDb, + ?DP("Scheduling callers of ~s for revisit: ~s.~n", + [fn(FuncId), string:join([fn(I) || I <- In], ", ")]), + foldl(fun aa_schedule_revisit/2, AAS, In). + expand_record_update(#opt_st{ssa=Linear0,cnt=First,anno=Anno0}=OptSt) -> {Linear,Cnt} = eru_blocks(Linear0, First), Anno = Anno0#{orig_cnt=>First}, diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl index 55400ccd9d27..e520bb3c4735 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl @@ -519,7 +519,7 @@ transformable24(L) -> transformable24(L, {<<>>, ex:foo(),ex:foo()}). transformable24([H|T], {Acc,X,Y}) -> -%ssa% (_, Arg1) when post_ssa_opt -> +%ssa% xfail (_, Arg1) when post_ssa_opt -> %ssa% X = get_tuple_element(Arg1, 1), %ssa% Acc = get_tuple_element(Arg1, 0), %ssa% A = bs_create_bin(append, _, Acc, _, _, _, Sum, _) { unique => [Sum,Acc], first_fragment_dies => true }, From 4a58947128179bb2cfd86b70900962e48c9be177 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Tue, 23 Jul 2024 09:50:48 +0200 Subject: [PATCH 18/25] compiler alias analysis: Analyze zero-arity functions first A zero-arity function, without internal calls, is likely to only have to be analyzed once before producing a stable alias status for its result. This means that callers of the zero-arity function will benefit from, at their first analysis pass, having a not `no_info` call result, thus making the the fixpoint iteration converging faster. This patch therefore changes the traversal order to explicitly analyze zero-arity functions first. --- lib/compiler/src/beam_ssa_alias.erl | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index d9f591c7b55d..cf8ec3e79c7d 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -348,7 +348,9 @@ aa(Funs, KillsMap, StMap, FuncDb) -> %%% ssa_opt_alias_finish pass. %%% aa_fixpoint(Funs, AAS=#aas{func_db=FuncDb}) -> - Order = aa_reverse_post_order(Funs, FuncDb), + Order = aa_order(Funs, FuncDb), + ?DP("Traversal order:~n ~s~n", + [string:join([fn(F) || F <- Order], ",\n ")]), aa_fixpoint(Order, Order, AAS, ?MAX_REPETITIONS). aa_fixpoint([F|Fs], Order, AAS0=#aas{st_map=StMap,repeats=Repeats}, Limit) -> @@ -1288,10 +1290,14 @@ aa_make_fun(Dst, Callee=#b_local{name=#b_literal{}}, end, {SS, AAS1#aas{call_args=Info}}. -aa_reverse_post_order(Funs, FuncDb) -> - %% In order to produce a reverse post order of the call graph, we - %% have to make sure all exported functions without local callers - %% are visited before exported functions with local callers. +aa_order(Funs, FuncDb) -> + %% Information about the alias status flows from callers to + %% callees and then back through the status of the result. In + %% order to avoid the most pessimistic estimate of the aliasing + %% status we want to process callers before callees with one + %% exception: zero-arity functions are always processed before + %% their callers as they are likely to give the most precise alias + %% information to their callers. IsExportedNoLocalCallers = fun (F) -> #{ F := #func_info{exported=E,in=In} } = FuncDb, @@ -1304,9 +1310,12 @@ aa_reverse_post_order(Funs, FuncDb) -> #{ F := #func_info{exported=E,in=In} } = FuncDb, E andalso In =/= [] end, + + ZeroArityFunctions = lists:sort([ F || #b_local{arity=0}=F <- Funs]), ExportedLocalCallers = lists:sort([ F || F <- Funs, IsExportedLocalCallers(F)]), - aa_reverse_post_order(ExportedNoLocalCallers, ExportedLocalCallers, + aa_reverse_post_order(ZeroArityFunctions ++ ExportedNoLocalCallers, + ExportedLocalCallers, sets:new(), FuncDb). aa_reverse_post_order([F|Work], Next, Seen, FuncDb) -> From a6868d6d249254d5cc13a77e5fc77badeadc998d Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Tue, 23 Jul 2024 09:51:28 +0200 Subject: [PATCH 19/25] compiler alias analysis: Always record call argument status Always record call argument status regardless of whether the callee has been analyzed. This change temporarily leads to test-suite failures, but also fixes a test which was broken by the commit which improved the fixpoint iteration strategy. --- lib/compiler/src/beam_ssa_alias.erl | 18 ++++++++---------- .../test/beam_ssa_check_SUITE_data/alias.erl | 4 ++-- .../tuple_inplace_checks.erl | 4 ++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index cf8ec3e79c7d..d470db77ff17 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -1131,16 +1131,16 @@ aa_phi(Dst, Args0, SS0, AAS) -> aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, #aas{alias_map=AliasMap,st_map=StMap,cnt=Cnt0}=AAS0) -> ?DP("A Call~n callee: ~s~n args: ~p~n", [fn(Callee), Args]), + ?DP(" caller args: ~p~n", [Args]), + SS1 = aa_alias_surviving_args(Args, Dst, SS0, AAS0), + ?DP(" caller ss before call:~n~s.~n", [beam_ssa_ss:dump(SS1)]), + #aas{alias_map=AliasMap} = AAS = + aa_add_call_info(Callee, Args, SS1, AAS0), case is_map_key(Callee, AliasMap) of true -> ?DP(" The callee is known~n"), #opt_st{args=_CalleeArgs} = map_get(Callee, StMap), ?DP(" callee args: ~p~n", [_CalleeArgs]), - ?DP(" caller args: ~p~n", [Args]), - SS1 = aa_alias_surviving_args(Args, Dst, SS0, AAS0), - ?DP(" caller ss before call:~n~s.~n", [beam_ssa_ss:dump(SS1)]), - #aas{alias_map=AliasMap} = AAS = - aa_add_call_info(Callee, Args, SS1, AAS0), #{Callee:=#{0:=_CalleeSS}=Lbl2SS} = AliasMap, ?DP(" callee ss:~n~s~n", [beam_ssa_ss:dump(_CalleeSS)]), ?DP(" caller ss after call:~n~s~n", [beam_ssa_ss:dump(SS1)]), @@ -1161,11 +1161,9 @@ aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, {SS, AAS#aas{cnt=Cnt}}; false -> ?DP(" The callee is unknown~n"), - %% We don't know anything about the function, don't change - %% the status of any variables, but make sure that it will - %% be visited. - - {SS0, aa_schedule_revisit(Callee, AAS0)} + %% We don't know anything about the function, so don't + %% change the status of the result. + {SS0, AAS} end; aa_call(_Dst, [#b_remote{mod=#b_literal{val=erlang}, name=#b_literal{val=exit}, diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl index e520bb3c4735..d8d82e3571e4 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl @@ -519,7 +519,7 @@ transformable24(L) -> transformable24(L, {<<>>, ex:foo(),ex:foo()}). transformable24([H|T], {Acc,X,Y}) -> -%ssa% xfail (_, Arg1) when post_ssa_opt -> +%ssa% (_, Arg1) when post_ssa_opt -> %ssa% X = get_tuple_element(Arg1, 1), %ssa% Acc = get_tuple_element(Arg1, 0), %ssa% A = bs_create_bin(append, _, Acc, _, _, _, Sum, _) { unique => [Sum,Acc], first_fragment_dies => true }, @@ -1207,7 +1207,7 @@ see_through() -> see_through1(R). see_through1({_,R}) -> -%ssa% (_) when post_ssa_opt -> +%ssa% xfail (_) when post_ssa_opt -> %ssa% _ = update_record(reuse, 3, Rec, _, _) {unique => [Rec], source_dies => true}. R#see_through{a=e:f()}. diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl index 61e551b10806..641340879b07 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl @@ -56,7 +56,7 @@ r0([], Acc) -> -record(ds,{a}). make_ds(a) -> -%ssa% (K) when post_ssa_opt -> +%ssa% xfail (K) when post_ssa_opt -> %ssa% switch(K, Fail, [{a,IsA},{b,IsB}]), %ssa% label IsB, %ssa% ret({0,0}), @@ -70,7 +70,7 @@ make_ds(b) -> %% Check that #ds{} is updated using update_record+inplace work_ds([X|Rest], {A,B,C=#ds{a=F}}) -> -%ssa% (Ls, Acc) when post_ssa_opt -> +%ssa% xfail (Ls, Acc) when post_ssa_opt -> %ssa% Size = bif:tuple_size(Acc), %ssa% switch(Size, Fail, [{2,_},{3,RecordLbl}]), %ssa% label RecordLbl, From f4310bc77a5379e00f6915f543dec6a44fe2c065 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Fri, 12 Jul 2024 12:04:02 +0200 Subject: [PATCH 20/25] compiler alias analysis: Status of not analyzed callee is `no_info` Return a `no_info` sharing status for the result of calling a function which has not yet been analyzed. This change restores the test cases broken by "compiler alias analysis: Always record call argument status". --- lib/compiler/src/beam_ssa_alias.erl | 24 ++++---- .../test/beam_ssa_check_SUITE_data/alias.erl | 2 +- .../beam_ssa_check_SUITE_data/no_info0.erl | 60 +++++++++++++++++++ .../tuple_inplace_checks.erl | 4 +- 4 files changed, 77 insertions(+), 13 deletions(-) create mode 100644 lib/compiler/test/beam_ssa_check_SUITE_data/no_info0.erl diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index d470db77ff17..f9602566ddc7 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -59,7 +59,9 @@ fn(#b_local{name=#b_literal{val=N},arity=A}) -> orig_st_map :: st_map(), repeats = sets:new() :: sets:set(func_id()), %% The next unused variable name in caller - cnt = 0 :: non_neg_integer() + cnt = 0 :: non_neg_integer(), + %% Functions which have been analyzed at least once. + analyzed = sets:new() :: sets:set(func_id()) }). %% A code location refering to either the #b_set{} defining a variable @@ -384,7 +386,7 @@ aa_fixpoint([], Order, #aas{func_db=FuncDb,repeats=Repeats}=AAS, Limit) -> end. aa_fun(F, #opt_st{ssa=Linear0,args=Args}, - AAS0=#aas{alias_map=AliasMap0,kills=KillsMap}) -> + AAS0=#aas{alias_map=AliasMap0,analyzed=Analyzed,kills=KillsMap}) -> %% Initially assume all formal parameters are unique for a %% non-exported function, if we have call argument info in the %% AAS, we use it. For an exported function, all arguments are @@ -403,7 +405,7 @@ aa_fun(F, #opt_st{ssa=Linear0,args=Args}, AAS1 end, AliasMap = AliasMap0#{ F => SS }, - AAS#aas{alias_map=AliasMap}. + AAS#aas{alias_map=AliasMap,analyzed=sets:add_element(F, Analyzed)}. %% Main entry point for the alias analysis aa_blocks([{?EXCEPTION_BLOCK,_}|Bs], @@ -1129,16 +1131,17 @@ aa_phi(Dst, Args0, SS0, AAS) -> aa_derive_from(Dst, Args, SS). aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, - #aas{alias_map=AliasMap,st_map=StMap,cnt=Cnt0}=AAS0) -> + #aas{alias_map=AliasMap,analyzed=Analyzed, + st_map=StMap,cnt=Cnt0}=AAS0) -> ?DP("A Call~n callee: ~s~n args: ~p~n", [fn(Callee), Args]), ?DP(" caller args: ~p~n", [Args]), SS1 = aa_alias_surviving_args(Args, Dst, SS0, AAS0), ?DP(" caller ss before call:~n~s.~n", [beam_ssa_ss:dump(SS1)]), #aas{alias_map=AliasMap} = AAS = aa_add_call_info(Callee, Args, SS1, AAS0), - case is_map_key(Callee, AliasMap) of + case sets:is_element(Callee, Analyzed) of true -> - ?DP(" The callee is known~n"), + ?DP(" The callee has been analyzed~n"), #opt_st{args=_CalleeArgs} = map_get(Callee, StMap), ?DP(" callee args: ~p~n", [_CalleeArgs]), #{Callee:=#{0:=_CalleeSS}=Lbl2SS} = AliasMap, @@ -1160,10 +1163,11 @@ aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, ?DP("~s~n", [beam_ssa_ss:dump(SS)]), {SS, AAS#aas{cnt=Cnt}}; false -> - ?DP(" The callee is unknown~n"), - %% We don't know anything about the function, so don't - %% change the status of the result. - {SS0, AAS} + ?DP(" The callee has not been analyzed~n"), + %% We don't know anything about the function, so + %% explicitly mark that we don't know anything about the + %% result. + {beam_ssa_ss:set_status(Dst, no_info, SS0), AAS} end; aa_call(_Dst, [#b_remote{mod=#b_literal{val=erlang}, name=#b_literal{val=exit}, diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl index d8d82e3571e4..55400ccd9d27 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl @@ -1207,7 +1207,7 @@ see_through() -> see_through1(R). see_through1({_,R}) -> -%ssa% xfail (_) when post_ssa_opt -> +%ssa% (_) when post_ssa_opt -> %ssa% _ = update_record(reuse, 3, Rec, _, _) {unique => [Rec], source_dies => true}. R#see_through{a=e:f()}. diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/no_info0.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/no_info0.erl new file mode 100644 index 000000000000..53b3b25344c1 --- /dev/null +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/no_info0.erl @@ -0,0 +1,60 @@ +%% %CopyrightBegin% +%% +%% Copyright Ericsson AB 2024. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% %CopyrightEnd% +%% +%% Extracts from beam_clean.erl to test cases when special handling of +%% the no_info sharing state is required for correct functioning. +%% +-module(no_info0). +-moduledoc false. + +-export([clean_labels/1]). + +-import(lists, [reverse/1]). + +-type label() :: beam_asm:label(). + +-record(st, {lmap :: [{label(),label()}], %Translation tables for labels. + entry :: beam_asm:label(), %Number of entry label. + lc :: non_neg_integer() %Label counter + }). + +clean_labels(Fs0) -> + St0 = #st{lmap=[],entry=1,lc=1}, + function_renumber(Fs0, St0, []). + +function_renumber([{function,Name,Arity,_Entry,Asm0}|Fs], St0, Acc) -> + {Asm,St} = renumber_labels(Asm0, [], St0), + function_renumber(Fs, St, [{function,Name,Arity,St#st.entry,Asm}|Acc]); +function_renumber([], St, Acc) -> {Acc,St}. + +renumber_labels([{label,Old}|Is], [{label,New}|_]=Acc, #st{lmap=D0}=St) -> +%ssa% (_, _, Rec) when post_ssa_opt -> +%ssa% _ = update_record(inplace, 4, Rec, ...), +%ssa% _ = update_record(inplace, 4, Rec, ...), +%ssa% _ = update_record(inplace, 4, Rec, ...). + D = [{Old,New}|D0], + renumber_labels(Is, Acc, St#st{lmap=D}); +renumber_labels([{label,Old}|Is], Acc, St0) -> + New = St0#st.lc, + D = [{Old,New}|St0#st.lmap], + renumber_labels(Is, [{label,New}|Acc], St0#st{lmap=D,lc=New+1}); +renumber_labels([{func_info,_,_,_}=Fi|Is], Acc, St0) -> + renumber_labels(Is, [Fi|Acc], St0#st{entry=St0#st.lc}); +renumber_labels([I|Is], Acc, St0) -> + renumber_labels(Is, [I|Acc], St0); +renumber_labels([], Acc, St) -> {Acc,St}. diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl index 641340879b07..61e551b10806 100644 --- a/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/tuple_inplace_checks.erl @@ -56,7 +56,7 @@ r0([], Acc) -> -record(ds,{a}). make_ds(a) -> -%ssa% xfail (K) when post_ssa_opt -> +%ssa% (K) when post_ssa_opt -> %ssa% switch(K, Fail, [{a,IsA},{b,IsB}]), %ssa% label IsB, %ssa% ret({0,0}), @@ -70,7 +70,7 @@ make_ds(b) -> %% Check that #ds{} is updated using update_record+inplace work_ds([X|Rest], {A,B,C=#ds{a=F}}) -> -%ssa% xfail (Ls, Acc) when post_ssa_opt -> +%ssa% (Ls, Acc) when post_ssa_opt -> %ssa% Size = bif:tuple_size(Acc), %ssa% switch(Size, Fail, [{2,_},{3,RecordLbl}]), %ssa% label RecordLbl, From b4f7a6c342e555cce284992ff42bb6b3c698964b Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Mon, 15 Jul 2024 15:09:21 +0200 Subject: [PATCH 21/25] compiler alias analysis: Improve handling of Phi-instructions Stop treating Phi instructions as an extraction, instead make use of the infrastructure used to derive the structure and alias status of function arguments. The sharing state database representation for the result of a Phi instruction is the same as for a function argument of a function with multiple call sites, where the concrete argument values at the call sites are the value at the respective predecessor block. --- lib/compiler/src/beam_ssa_alias.erl | 12 ++-- lib/compiler/src/beam_ssa_ss.erl | 15 +++++ .../test/beam_ssa_check_SUITE_data/phis.erl | 57 +++++++++++++++++++ 3 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 lib/compiler/test/beam_ssa_check_SUITE_data/phis.erl diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index f9602566ddc7..9da813d96ab0 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -502,7 +502,7 @@ aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> peek_message -> {aa_set_aliased(Dst, SS0), AAS0}; phi -> - {aa_phi(Dst, Args, SS0, AAS0), AAS0}; + aa_phi(Dst, Args, SS0, AAS0); put_list -> SS1 = beam_ssa_ss:add_var(Dst, unique, SS0), Types = @@ -1125,10 +1125,14 @@ aa_bif(Dst, Bif, Args, SS, _AAS) -> aa_set_aliased([Dst|Args], SS) end. -aa_phi(Dst, Args0, SS0, AAS) -> +aa_phi(Dst, Args0, SS0, #aas{cnt=Cnt0}=AAS) -> + %% TODO: Use type info? Args = [V || {V,_} <- Args0], - SS = aa_alias_surviving_args(Args, {phi,Dst}, SS0, AAS), - aa_derive_from(Dst, Args, SS). + ?DP("Phi~n"), + SS1 = aa_alias_surviving_args(Args, {phi,Dst}, SS0, AAS), + ?DP(" after aa_alias_surviving_args:~n~s.~n", [beam_ssa_ss:dump(SS1)]), + {SS,Cnt} = beam_ssa_ss:phi(Dst, Args, SS1, Cnt0), + {SS,AAS#aas{cnt=Cnt}}. aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0, #aas{alias_map=AliasMap,analyzed=Analyzed, diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index 088c81fcc9a2..e878f6ef7671 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -43,6 +43,7 @@ merge_in_args/3, new/0, new/3, + phi/4, prune/3, set_call_result/4, set_status/3, @@ -450,6 +451,20 @@ accumulate_edges(V, State, Edges0) -> new() -> beam_digraph:new(). +-spec phi(beam_ssa:b_var(), [beam_ssa:b_var()], + sharing_state(), non_neg_integer()) + -> sharing_state(). +phi(Dst, Args, State0, Cnt) -> + ?assert_state(State0), + ?DP("** phi **~n~s~n", [dump(State0)]), + ?DP(" dst: ~p~n", [Dst]), + ?DP(" args: ~p~n", [Args]), + Structure = foldl(fun(Arg, Acc) -> + merge_in_arg(Arg, Acc, ?ARGS_DEPTH_LIMIT, State0) + end, no_info, Args), + ?DP(" structure: ~p~n", [Structure]), + new([Dst], [Structure], Cnt, State0). + %%% %%% Throws `too_deep` if the depth of sharing state value chains %%% exceeds SS_DEPTH_LIMIT. diff --git a/lib/compiler/test/beam_ssa_check_SUITE_data/phis.erl b/lib/compiler/test/beam_ssa_check_SUITE_data/phis.erl new file mode 100644 index 000000000000..cff2b64338e2 --- /dev/null +++ b/lib/compiler/test/beam_ssa_check_SUITE_data/phis.erl @@ -0,0 +1,57 @@ +%% Extracted from lib/syntax_tools/src/erl_recomment.erl to test +%% omissions in handling of Phi instructions. + +%% ===================================================================== +%% Licensed under the Apache License, Version 2.0 (the "License"); you may +%% not use this file except in compliance with the License. You may obtain +%% a copy of the License at +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%% +%% Alternatively, you may use this file under the terms of the GNU Lesser +%% General Public License (the "LGPL") as published by the Free Software +%% Foundation; either version 2.1, or (at your option) any later version. +%% If you wish to allow use of your version of this file only under the +%% terms of the LGPL, you should delete the provisions above and replace +%% them with the notice and other provisions required by the LGPL; see +%% . If you do not delete the provisions +%% above, a recipient may use your version of this file under the terms of +%% either the Apache License or the LGPL. +%% +%% @copyright 1997-2006 Richard Carlsson +%% @author Richard Carlsson +%% @end +%% ===================================================================== + +-module(phis). + +-export([filter_forms/1]). + +-record(filter, {file = undefined :: file:filename() | 'undefined', + line = 0 :: integer()}). + +filter_forms(Fs) -> + filter_forms(Fs, #filter{}). + +filter_forms([{A1, A2} | Fs], S) -> +%ssa% (_, Rec0) when post_ssa_opt -> +%ssa% Rec = update_record(inplace, 3, Rec0, ...), +%ssa% Phi = phi({Rec0, _}, {Rec, _}), +%ssa% _ = update_record(inplace, 3, Phi, ...). + S1 = case ex:f() of + undefined -> + S#filter{file = A1, line = A2}; + _ -> + S + end, + if S1#filter.file =:= A1 -> + filter_forms(Fs, S1#filter{line = A2}); + true -> + filter_forms(Fs, S1) + end; +filter_forms([], _) -> + []. From d0b64a2b6b4441c5439797035dc523ca4b7a112d Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Tue, 23 Jul 2024 14:03:22 +0200 Subject: [PATCH 22/25] compiler alias analysis: Change work limiting strategy Instead of limiting the maximum number of allowed traversals of the functions in the module, limit the number of times a function is analyzed before aborting the alias analysis pass. Compared to the old limit strategy, this allows all modules compiled by scripts/diffable to successfully complete, without a noticeable change in runtime. --- lib/compiler/src/beam_ssa_alias.erl | 33 +++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index 9da813d96ab0..aab967b76897 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -61,7 +61,8 @@ fn(#b_local{name=#b_literal{val=N},arity=A}) -> %% The next unused variable name in caller cnt = 0 :: non_neg_integer(), %% Functions which have been analyzed at least once. - analyzed = sets:new() :: sets:set(func_id()) + analyzed = sets:new() :: sets:set(func_id()), + run_count = #{} :: #{ func_id() => non_neg_integer() } }). %% A code location refering to either the #b_set{} defining a variable @@ -353,9 +354,11 @@ aa_fixpoint(Funs, AAS=#aas{func_db=FuncDb}) -> Order = aa_order(Funs, FuncDb), ?DP("Traversal order:~n ~s~n", [string:join([fn(F) || F <- Order], ",\n ")]), - aa_fixpoint(Order, Order, AAS, ?MAX_REPETITIONS). + aa_fixpoint(Order, Order, AAS, 1). -aa_fixpoint([F|Fs], Order, AAS0=#aas{st_map=StMap,repeats=Repeats}, Limit) -> +aa_fixpoint([F|Fs], Order, + AAS0=#aas{func_db=FuncDb,st_map=StMap, + repeats=Repeats,run_count=RC}, NoofIters) -> ?DP("-= ~s =-~n", [fn(F)]), %% If, when analysing another function, this function has been @@ -367,22 +370,26 @@ aa_fixpoint([F|Fs], Order, AAS0=#aas{st_map=StMap,repeats=Repeats}, Limit) -> ?DP("code:~n~p.~n", [_Is]), AAS = aa_fun(F, St, AAS1), ?DP("Done ~s~n", [fn(F)]), - aa_fixpoint(Fs, Order, AAS, Limit); -aa_fixpoint([], _, #aas{func_db=FuncDb,orig_st_map=StMap}, 0) -> - ?DP("**** End of iteration, too many iterations ****~n"), - {StMap, FuncDb}; -aa_fixpoint([], Order, #aas{func_db=FuncDb,repeats=Repeats}=AAS, Limit) -> + case maps:get(F, RC, ?MAX_REPETITIONS) of + 0 -> + ?DP("**** End of iteration, too many iterations ****~n"), + {StMap, FuncDb}; + N -> + aa_fixpoint(Fs, Order, + AAS#aas{run_count=RC#{F => N - 1}}, NoofIters) + end; +aa_fixpoint([], Order, #aas{func_db=FuncDb,repeats=Repeats}=AAS, NoofIters) -> %% Following the depth first order, select those in Repeats. case [Id || Id <- Order, sets:is_element(Id, Repeats)] of [] -> - ?DP("**** Fixpoint reached after ~p iterations ****~n", - [?MAX_REPETITIONS - Limit]), + ?DP("**** Fixpoint reached after ~p traversals ****~n", + [NoofIters]), {StMap,_} = aa_update_annotations(Order, AAS), {StMap, FuncDb}; NewOrder -> - ?DP("**** Starting iteration ~p ****~n", - [?MAX_REPETITIONS - Limit + 1]), - aa_fixpoint(NewOrder, Order, AAS#aas{repeats=sets:new()}, Limit - 1) + ?DP("**** Starting traversal ~p ****~n", [NoofIters + 1]), + aa_fixpoint(NewOrder, Order, + AAS#aas{repeats=sets:new()}, NoofIters + 1) end. aa_fun(F, #opt_st{ssa=Linear0,args=Args}, From e66fa4d195ac8b203f93cfa325f44a73ebdd3e2d Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Tue, 23 Jul 2024 14:11:12 +0200 Subject: [PATCH 23/25] compiler alias analysis: Alternate function traversal order Sharing state information flows from callers to callees for function arguments, and from callees to callers for results. Previously callers were always analyzed before callees, but it turns out that the alias analysis converges faster if the traversal order is alternated between caller-callee and callee-caller. --- lib/compiler/src/beam_ssa_alias.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index aab967b76897..0f9e407949d7 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -354,7 +354,7 @@ aa_fixpoint(Funs, AAS=#aas{func_db=FuncDb}) -> Order = aa_order(Funs, FuncDb), ?DP("Traversal order:~n ~s~n", [string:join([fn(F) || F <- Order], ",\n ")]), - aa_fixpoint(Order, Order, AAS, 1). + aa_fixpoint(Order, reverse(Order), AAS, 1). aa_fixpoint([F|Fs], Order, AAS0=#aas{func_db=FuncDb,st_map=StMap, @@ -388,7 +388,7 @@ aa_fixpoint([], Order, #aas{func_db=FuncDb,repeats=Repeats}=AAS, NoofIters) -> {StMap, FuncDb}; NewOrder -> ?DP("**** Starting traversal ~p ****~n", [NoofIters + 1]), - aa_fixpoint(NewOrder, Order, + aa_fixpoint(NewOrder, reverse(Order), AAS#aas{repeats=sets:new()}, NoofIters + 1) end. From ccf5a662cb637bcb87680d89fbe5cab33ac5b976 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Thu, 25 Jul 2024 09:59:52 +0200 Subject: [PATCH 24/25] compiler alias analysis: Speed up state pruning In around 80% of the cases when prune is called, more than half of the nodes in the sharing state database survive. Therefore a pruning strategy which removes nodes from the database has been used. This patch resurrects the previously used pruning algorithm which instead recreated the pruned state from scratch instead of removing nodes which is faster when only a few nodes survive the pruning. The alias analysis implementation is then extended to, for each pruned basic block, by looking at the number of nodes in the database before and after the prune, record which strategy is the most efficient. If the basic block is revisited, the fastest prune implementation is selected. On the modules compiled by the `scripts/diffable` tool, this change reduces the time spent on alias analysis by close to six percent. --- lib/compiler/src/beam_ssa_alias.erl | 60 ++++++++++++++++++++++------- lib/compiler/src/beam_ssa_ss.erl | 58 ++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 13 deletions(-) diff --git a/lib/compiler/src/beam_ssa_alias.erl b/lib/compiler/src/beam_ssa_alias.erl index 0f9e407949d7..9d3433855f64 100644 --- a/lib/compiler/src/beam_ssa_alias.erl +++ b/lib/compiler/src/beam_ssa_alias.erl @@ -62,7 +62,10 @@ fn(#b_local{name=#b_literal{val=N},arity=A}) -> cnt = 0 :: non_neg_integer(), %% Functions which have been analyzed at least once. analyzed = sets:new() :: sets:set(func_id()), - run_count = #{} :: #{ func_id() => non_neg_integer() } + run_count = #{} :: #{ func_id() => non_neg_integer() }, + prune_strategy = #{} :: #{ func_id() => + #{beam_ssa:label() => + 'add' | 'del'} } }). %% A code location refering to either the #b_set{} defining a variable @@ -393,15 +396,18 @@ aa_fixpoint([], Order, #aas{func_db=FuncDb,repeats=Repeats}=AAS, NoofIters) -> end. aa_fun(F, #opt_st{ssa=Linear0,args=Args}, - AAS0=#aas{alias_map=AliasMap0,analyzed=Analyzed,kills=KillsMap}) -> + AAS0=#aas{alias_map=AliasMap0,analyzed=Analyzed,kills=KillsMap, + prune_strategy=StrategyMap0}) -> %% Initially assume all formal parameters are unique for a %% non-exported function, if we have call argument info in the %% AAS, we use it. For an exported function, all arguments are %% assumed to be aliased. {SS0,Cnt} = aa_init_fun_ss(Args, F, AAS0), #{F:={LiveIns,Kills,PhiLiveIns}} = KillsMap, - {SS,AAS1} = aa_blocks(Linear0, LiveIns, PhiLiveIns, - Kills, #{0=>SS0}, AAS0#aas{cnt=Cnt}), + Strategy0 = maps:get(F, StrategyMap0, #{}), + {SS,Strategy,AAS1} = + aa_blocks(Linear0, LiveIns, PhiLiveIns, + Kills, #{0=>SS0}, Strategy0, AAS0#aas{cnt=Cnt}), Lbl2SS0 = maps:get(F, AliasMap0, #{}), Type2Status0 = maps:get(returns, Lbl2SS0, #{}), Type2Status = maps:get(returns, SS, #{}), @@ -412,30 +418,58 @@ aa_fun(F, #opt_st{ssa=Linear0,args=Args}, AAS1 end, AliasMap = AliasMap0#{ F => SS }, - AAS#aas{alias_map=AliasMap,analyzed=sets:add_element(F, Analyzed)}. + StrategyMap = StrategyMap0#{F => Strategy}, + AAS#aas{alias_map=AliasMap,analyzed=sets:add_element(F, Analyzed), + prune_strategy=StrategyMap}. %% Main entry point for the alias analysis aa_blocks([{?EXCEPTION_BLOCK,_}|Bs], - LiveIns, PhiLiveIns, Kills, Lbl2SS, AAS) -> + LiveIns, PhiLiveIns, Kills, Lbl2SS, Strategy, AAS) -> %% Nothing happening in the exception block can propagate to the %% other block. - aa_blocks(Bs, LiveIns, PhiLiveIns, Kills, Lbl2SS, AAS); + aa_blocks(Bs, LiveIns, PhiLiveIns, Kills, Lbl2SS, Strategy, AAS); aa_blocks([{L,#b_blk{is=Is0,last=T}}|Bs0], - LiveIns, PhiLiveIns, Kills, Lbl2SS0, AAS0) -> + LiveIns, PhiLiveIns, Kills, Lbl2SS0, Strategy0, AAS0) -> #{L:=SS0} = Lbl2SS0, ?DP("Block: ~p~nSS:~n~s~n", [L, beam_ssa_ss:dump(SS0)]), {FullSS,AAS1} = aa_is(Is0, SS0, AAS0), #{{live_outs,L}:=LiveOut} = Kills, - #{{killed_in_block,L}:=KilledInBlock} = Kills, {Lbl2SS1,Successors} = aa_terminator(T, FullSS, Lbl2SS0), - PrunedSS = beam_ssa_ss:prune(LiveOut, KilledInBlock, FullSS), + %% In around 80% of the cases when prune is called, more than half + %% of the nodes in the sharing state database survive. Therefore + %% we default to a pruning strategy which removes nodes from the + %% database. But if only a few nodes survive it is faster to + %% recreate the pruned state from scratch. We therefore track the + %% result of a previous prune for the current basic block and + %% select the, hopefully, best pruning strategy. + Before = beam_ssa_ss:size(FullSS), + S = maps:get(L, Strategy0, del), + PrunedSS = + case S of + del -> + #{{killed_in_block,L}:=KilledInBlock} = Kills, + beam_ssa_ss:prune(LiveOut, KilledInBlock, FullSS); + add -> + beam_ssa_ss:prune_by_add(LiveOut, FullSS) + end, + After = beam_ssa_ss:size(PrunedSS), + Strategy = case After < (Before div 2) of + true when S =:= add -> + Strategy0; + false when S =:= del -> + Strategy0; + true -> + Strategy0#{L => add}; + false -> + Strategy0#{L => del} + end, ?DP("Live out from ~p: ~p~n", [L, sets:to_list(LiveOut)]), Lbl2SS2 = aa_add_block_entry_ss(Successors, L, PrunedSS, LiveOut, LiveIns, PhiLiveIns, Lbl2SS1), Lbl2SS = aa_set_block_exit_ss(L, FullSS, Lbl2SS2), - aa_blocks(Bs0, LiveIns, PhiLiveIns, Kills, Lbl2SS, AAS1); -aa_blocks([], _LiveIns, _PhiLiveIns, _Kills, Lbl2SS, AAS) -> - {Lbl2SS,AAS}. + aa_blocks(Bs0, LiveIns, PhiLiveIns, Kills, Lbl2SS, Strategy, AAS1); +aa_blocks([], _LiveIns, _PhiLiveIns, _Kills, Lbl2SS, Strategy, AAS) -> + {Lbl2SS, Strategy, AAS}. aa_is([_I=#b_set{dst=Dst,op=Op,args=Args,anno=Anno0}|Is], SS0, AAS0) -> ?DP("I: ~p~n", [_I]), diff --git a/lib/compiler/src/beam_ssa_ss.erl b/lib/compiler/src/beam_ssa_ss.erl index e878f6ef7671..bf5fd649e040 100644 --- a/lib/compiler/src/beam_ssa_ss.erl +++ b/lib/compiler/src/beam_ssa_ss.erl @@ -45,8 +45,10 @@ new/3, phi/4, prune/3, + prune_by_add/2, set_call_result/4, set_status/3, + size/1, variables/1]). -include("beam_ssa.hrl"). @@ -520,6 +522,58 @@ is_safe_to_prune(V, LiveVars, State) -> end end. +%%% +%%% As prune/3, but doing the pruning by rebuilding the surviving +%%% state from scratch. +%%% +%%% Throws `too_deep` if the depth of sharing state value chains +%%% exceeds SS_DEPTH_LIMIT. +%%% +-spec prune_by_add(sets:set(beam_ssa:b_var()), sharing_state()) + -> sharing_state(). +prune_by_add(LiveVars, State) -> + ?assert_state(State), + ?DP("Pruning to ~p~n", [sets:to_list(LiveVars)]), + ?DP("~s~n", [dump(State)]), + ?DP("Vertices: ~p~n", [beam_digraph:vertices(State)]), + R = prune_by_add([{0,V} || V <- sets:to_list(LiveVars), + beam_digraph:has_vertex(State, V)], + [], new(), State), + ?DP("Pruned result~n~s~n", [dump(R)]), + ?assert_state(R). + +prune_by_add([{Depth0,V}|Wanted], Edges, New0, Old) -> + ?DP("Looking at ~p~n", [V]), + ?DP("Curr:~n~s~n", [dump(New0)]), + ?DP("Wanted: ~p~n", [Wanted]), + case beam_digraph:has_vertex(New0, V) of + true -> + %% This variable is already added. + prune_by_add(Wanted, Edges, New0, Old); + false when Depth0 < ?SS_DEPTH_LIMIT -> + %% This variable has to be kept. Add it to the new graph. + New = add_vertex(New0, V, beam_digraph:vertex(Old, V)), + %% Add all incoming edges to this node. + InEdges = beam_digraph:in_edges(Old, V), + Depth = Depth0 + 1, + InNodes = [{Depth, From} || {From,_,_} <- InEdges], + prune_by_add(InNodes ++ Wanted, InEdges ++ Edges, New, Old); + false -> + %% We're in too deep, give up. This case will probably + %% never be hit as it would require a previous prune/3 + %% application which doesn't hit the depth limit and for + %% it to remove more than half of the nodes to trigger the + %% use of prune_by_add/2, and in a later iteration trigger + %% the depth limit. As it cannot be definitely ruled out, + %% take the hit against the coverage statistics, as the + %% handling code in beam_ssa_alias is tested. + throw(too_deep) + end; +prune_by_add([], Edges, New0, _Old) -> + foldl(fun({Src,Dst,Lbl}, New) -> + add_edge(New, Src, Dst, Lbl) + end, New0, Edges). + -spec set_call_result(beam_ssa:b_var(), call_in_arg_status(), sharing_state(), non_neg_integer()) -> {sharing_state(),non_neg_integer()}. @@ -604,6 +658,10 @@ get_alias_edges(V, State) -> end], EmbedEdges ++ OutEdges. +-spec size(sharing_state()) -> non_neg_integer(). +size(State) -> + beam_digraph:no_vertices(State). + -spec variables(sharing_state()) -> [beam_ssa:b_var()]. variables(State) -> [V || {V,_Lbl} <- beam_digraph:vertices(State)]. From b83561a2b863ae5e4a88c71f28fcf90970cb6085 Mon Sep 17 00:00:00 2001 From: Frej Drejhammar Date: Wed, 24 Jul 2024 09:37:55 +0200 Subject: [PATCH 25/25] compiler: Avoid reuse hint for update_record with > 1 update Take care to not produce a reuse hint when more than one update exists. There is no point in attempting the reuse optimization when more than one element is updated, as checking more than one element at runtime is known to be slower than just copying the tuple in most cases. Additionally, using a copy hint occasionally allows the alias analysis pass to do a better job. --- lib/compiler/src/beam_ssa_type.erl | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/compiler/src/beam_ssa_type.erl b/lib/compiler/src/beam_ssa_type.erl index fdc5d06dd9b9..f34dd1cb51f4 100644 --- a/lib/compiler/src/beam_ssa_type.erl +++ b/lib/compiler/src/beam_ssa_type.erl @@ -1561,12 +1561,26 @@ will_succeed_1(#b_set{op=wait_timeout}, _Src, _Ts) -> will_succeed_1(#b_set{}, _Src, _Ts) -> 'maybe'. +%% Take care to not produce a reuse hint when more than one update +%% exists. There is no point in attempting the reuse optimization when +%% more than one element is updated, as checking more than one element +%% at runtime is known to be slower than just copying the tuple in +%% most cases. Additionally, using a copy hint occasionally allows the +%% alias analysis pass to do a better job. simplify_update_record(Src, Hint0, Updates, Ts) -> case sur_1(Updates, concrete_type(Src, Ts), Ts, Hint0, []) of + {#b_literal{val=reuse}, []} when length(Updates) > 2 -> + {changed, #b_literal{val=copy}, Updates}; {Hint0, []} -> unchanged; - {Hint, Skipped} -> - {changed, Hint, sur_skip(Updates, Skipped)} + {Hint1, Skipped} -> + Updates1 = sur_skip(Updates, Skipped), + Hint = if length(Updates1) > 2 -> + #b_literal{val=copy}; + true -> + Hint1 + end, + {changed, Hint, Updates1} end. sur_1([Index, Arg | Updates], RecordType, Ts, Hint, Skipped) ->