Skip to content

Commit

Permalink
Fix register allocation in the presence of singled-valued phi nodes
Browse files Browse the repository at this point in the history
With optimizations disabled, the register allocator in
`beam_ssa_pre_codegen` could allocate different variables to the
same register if single-valued phi nodes in a try/catch construct
were present.

Closes erlang#7248
  • Loading branch information
bjorng committed May 24, 2023
1 parent 8c0ea6b commit b7dd6cc
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 6 deletions.
20 changes: 16 additions & 4 deletions lib/compiler/src/beam_ssa_pre_codegen.erl
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,13 @@ sanitize_arg(#b_var{}=Var, Values) ->
sanitize_arg(Arg, _Values) ->
Arg.

sanitize_instr(phi, [{#b_var{}=V,_}], I, _Blocks) ->
%% GH-7248: With optimizations disabled, the register allocator
%% can accidentaly allocate different variables to the same
%% register if single-valued phi nodes in a try/catch construct
%% are present. Prevent that by turning singled-valued phi nodes
%% into a `copy` instructions.
{ok,I#b_set{anno=#{was_phi_node => true},op=copy,args=[V]}};
sanitize_instr(phi, PhiArgs0, I, Blocks) ->
PhiArgs = [{V,L} || {V,L} <- PhiArgs0,
is_map_key(L, Blocks)],
Expand Down Expand Up @@ -1927,11 +1934,16 @@ copy_retval_1([F|Fs], Blocks0, Count0) ->
copy_retval_1([], Blocks, Count) ->
{Blocks,Count}.

collect_yregs([#b_set{op=copy,dst=Y,args=[#b_var{}=X]}|Is],
collect_yregs([#b_set{anno=Anno,op=copy,dst=Y,args=[#b_var{}=X]}|Is],
Yregs0) ->
true = sets:is_element(X, Yregs0), %Assertion.
Yregs = sets:add_element(Y, sets:del_element(X, Yregs0)),
collect_yregs(Is, Yregs);
case Anno of
#{was_phi_node := _} ->
collect_yregs(Is, Yregs0);
#{} ->
true = sets:is_element(X, Yregs0), %Assertion.
Yregs = sets:add_element(Y, sets:del_element(X, Yregs0)),
collect_yregs(Is, Yregs)
end;
collect_yregs([#b_set{}|Is], Yregs) ->
collect_yregs(Is, Yregs);
collect_yregs([], Yregs) -> Yregs.
Expand Down
55 changes: 53 additions & 2 deletions lib/compiler/test/beam_ssa_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@
beam_ssa_dead_crash/1,stack_init/1,
mapfoldl/0,mapfoldl/1,
grab_bag/1,redundant_br/1,
coverage/1,normalize/1]).
coverage/1,normalize/1,
trycatch/1]).

suite() -> [{ct_hooks,[ts_install_cth]}].

Expand All @@ -49,7 +50,8 @@ groups() ->
grab_bag,
redundant_br,
coverage,
normalize
normalize,
trycatch
]}].

init_per_suite(Config) ->
Expand Down Expand Up @@ -1346,5 +1348,54 @@ unpack_bset({b_set,Anno,{b_var,1000},Op,Args}) ->
ArgTypes = maps:get(arg_types, Anno, #{}),
{lists:sort(maps:to_list(ArgTypes)),Op,Args}.

trycatch(_Config) ->
8 = trycatch_1(),

ok = trycatch_2(id(ok)),
ok = trycatch_2(id(z)),

false = trycatch_3(id(42)),

ok.

trycatch_1() ->
try B = (A = bit_size(iolist_to_binary("a"))) rem 1 of
_ ->
A;
_ ->
B
after
ok
end.

trycatch_2(A) ->
try not (B = (ok >= A)) of
B ->
iolist_size(maybe
[] ?= B,
<<>> ?= list_to_binary(ok)
end);
_ ->
ok
after
ok
end.

trycatch_3(A) ->
try erlang:bump_reductions(A) of
B ->
try not (C = (B andalso is_number(ok))) of
C ->
ok andalso ok;
_ ->
C
catch
_ ->
ok
end
after
ok
end.

%% The identity function.
id(I) -> I.

0 comments on commit b7dd6cc

Please sign in to comment.