Skip to content

Commit

Permalink
compiler alias analysis: Fix repeated argument bug
Browse files Browse the repository at this point in the history
When the same variable occurs multiple times in the argument list to a
call, that variable should become aliased.

This patch corrects the above described omission by renaming the
current aa_alias_surviving_args/4 to aa_alias_surviving_phi_args/4
which we use for Phi-instructions and creating a new
aa_alias_surviving_args/4 (for calls) which makes use of
aa_alias_surviving_phi_args/4 but also does a second pass through the
argument list to find repeated variables.
  • Loading branch information
frej committed Jan 28, 2025
1 parent 4726e39 commit 6a703cc
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 7 deletions.
34 changes: 28 additions & 6 deletions lib/compiler/src/beam_ssa_alias.erl
Original file line number Diff line number Diff line change
Expand Up @@ -857,19 +857,41 @@ aa_get_status_by_type(Type, StatusByType) ->
beam_ssa_ss:meet_in_args(Statuses)
end.

aa_alias_surviving_args(Args, Call, SS, AAS) ->
aa_alias_surviving_phi_args(Args, Call, SS, AAS) ->
KillSet = aa_killset_for_instr(Call, AAS),
aa_alias_surviving_args1(Args, SS, KillSet).
aa_alias_surviving_phi_args1(Args, SS, KillSet).

aa_alias_surviving_args1([A|Args], SS0, KillSet) ->
aa_alias_surviving_phi_args1([#b_var{}=A|Args], SS0, KillSet) ->
SS = case sets:is_element(A, KillSet) of
true ->
SS0;
false ->
aa_set_status(A, aliased, SS0)
end,
aa_alias_surviving_args1(Args, SS, KillSet);
aa_alias_surviving_args1([], SS, _KillSet) ->
aa_alias_surviving_phi_args1(Args, SS, KillSet);
aa_alias_surviving_phi_args1([_|Args], SS, KillSet) ->
aa_alias_surviving_phi_args1(Args, SS, KillSet);
aa_alias_surviving_phi_args1([], SS, _KillSet) ->
SS.

aa_alias_surviving_args(Args, Call, SS0, AAS) ->
SS = aa_alias_surviving_phi_args(Args, Call, SS0, AAS),
%% Compared to phi arguments, function arguments are all used, and
%% if the same variable is used more than once, it becomes
%% aliased.
aa_alias_repeated_args(Args, SS, sets:new()).

aa_alias_repeated_args([#b_var{}=A|Args], SS0, Seen) ->
SS = case sets:is_element(A, Seen) of
true ->
aa_set_status(A, aliased, SS0);
false ->
SS0
end,
aa_alias_repeated_args(Args, SS, sets:add_element(A, Seen));
aa_alias_repeated_args([_|Args], SS, Seen) ->
aa_alias_repeated_args(Args, SS, Seen);
aa_alias_repeated_args([], SS, _Seen) ->
SS.

%% Return the kill-set for the instruction defining Dst.
Expand Down Expand Up @@ -1099,7 +1121,7 @@ aa_bif(Dst, Bif, Args, SS, _AAS) ->

aa_phi(Dst, Args0, SS0, AAS) ->
Args = [V || {V,_} <- Args0],
SS = aa_alias_surviving_args(Args, {phi,Dst}, SS0, AAS),
SS = aa_alias_surviving_phi_args(Args, {phi,Dst}, SS0, AAS),
aa_derive_from(Dst, Args, SS).

aa_call(Dst, [#b_local{}=Callee|Args], Anno, SS0,
Expand Down
15 changes: 14 additions & 1 deletion lib/compiler/test/beam_ssa_check_SUITE_data/alias.erl
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@
fuzz0/0, fuzz0/1,
alias_after_phi/0,
check_identifier_type/0,
gh9014_main/0]).
gh9014_main/0,

duplicated_args/1]).

%% Trivial smoke test
transformable0(L) ->
Expand Down Expand Up @@ -1180,3 +1182,14 @@ gh9014_wibble(State) ->

gh9014_main() ->
{counter, 1} = gh9014_wibble({state, {counter, 0}}).

duplicated_args(V) ->
%% The constructed list was considered unique, which made
%% duplicated_args/2 think that both arguments were unique
%% (which is wrong).
duplicated_args([V], [V]).

duplicated_args(A, B) ->
%ssa% (A, B) when post_ssa_opt ->
%ssa% _ = put_tuple(A, B) {aliased => [A, B]}.
{A, B}.

0 comments on commit 6a703cc

Please # to comment.