Skip to content

Commit

Permalink
Avoid optimization that beam_validator might consider unsafe
Browse files Browse the repository at this point in the history
Eliminating a repeated `=:=` test can result in code that
beam_validator doesn't consider safe. This commit makes the optimization
keep the `=:=` if it seems to be significant, that is, if one or both
operands will gain type information from it.

That makes it possible to compile code from erlang#6599 such as:

    f(X, X) when is_integer(X) ->
        ok;
    f(Y, Y = #{}) ->
        Y#{ok := ok}.

(Seven other examples from erlang#6599 can now also be compiled.)

Running `scripts/diffable` shows that this fix prevented the
optimization in 14 modules (out of about 1000).
  • Loading branch information
bjorng committed May 31, 2023
1 parent f982f55 commit e5b8254
Showing 1 changed file with 40 additions and 3 deletions.
43 changes: 40 additions & 3 deletions lib/compiler/src/beam_ssa_dead.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1176,9 +1176,19 @@ opt_redundant_tests_is([#b_set{op=Op,args=Args,dst=Bool}=I0], Tests, Acc) ->
{Test,MustInvert} ->
case old_result(Test, Tests) of
Result0 when is_boolean(Result0) ->
Result = #b_literal{val=Result0 xor MustInvert},
I = I0#b_set{op={bif,'=:='},args=[Result,#b_literal{val=true}]},
{old_test,reverse(Acc, [I]),Bool,Result};
case gains_type_information(I0) of
false ->
Result = #b_literal{val=Result0 xor MustInvert},
I = I0#b_set{op={bif,'=:='},args=[Result,#b_literal{val=true}]},
{old_test,reverse(Acc, [I]),Bool,Result};
true ->
%% At least one variable will gain type
%% information from this `=:=`
%% operation. Removing it could make it
%% impossible for beam_validator to
%% realize that the code is type-safe.
none
end;
none ->
{new_test,Bool,Test,MustInvert}
end
Expand All @@ -1187,6 +1197,33 @@ opt_redundant_tests_is([I|Is], Tests, Acc) ->
opt_redundant_tests_is(Is, Tests, [I|Acc]);
opt_redundant_tests_is([], _Tests, _Acc) -> none.

%% Will any of the variables gain type information from this
%% operation?
gains_type_information(#b_set{anno=Anno,op={bif,'=:='},args=Args}) ->
Types0 = maps:get(arg_types, Anno, #{}),
Types = complete_type_information(Args, 0, Types0),
case map_size(Types) of
0 ->
false;
1 ->
true;
2 ->
case Types of
#{0 := Same,1 := Same} ->
false;
#{} ->
true
end
end;
gains_type_information(#b_set{}) -> false.

complete_type_information([#b_literal{val=Value}|As], N, Types) ->
Type = beam_types:make_type_from_value(Value),
complete_type_information(As, N+1, Types#{N => Type});
complete_type_information([#b_var{}|As], N, Types) ->
complete_type_information(As, N+1, Types);
complete_type_information([], _, Types) -> Types.

old_result(Test, Tests) ->
case Tests of
#{Test := Val} -> Val;
Expand Down

0 comments on commit e5b8254

Please sign in to comment.