aboutsummaryrefslogtreecommitdiffstats
path: root/lib/compiler
diff options
context:
space:
mode:
authorBjörn Gustavsson <[email protected]>2018-12-18 12:59:38 +0100
committerBjörn Gustavsson <[email protected]>2019-01-07 12:55:26 +0100
commit7a01269e62f21ac3ba9858dd358716081549bea3 (patch)
treefb11fe61a52e2f1b3dd1801491646d773c90b42a /lib/compiler
parentf0ea49125815ec9197ffb6c74e20ebb5f10732d4 (diff)
downloadotp-7a01269e62f21ac3ba9858dd358716081549bea3.tar.gz
otp-7a01269e62f21ac3ba9858dd358716081549bea3.tar.bz2
otp-7a01269e62f21ac3ba9858dd358716081549bea3.zip
Remove unsafe optimization for delaying creation of stackframe
b89044a800c4 introduced an optimization that tries to delay creation of stack frames. It turns out that this optimization is not always safe. (See the new test case for an example.) Since the code generator is completely rewritten in the `master` branch for the upcoming OTP 22 release, it does not make sense trying to mend this optimization. It is better to remove it. Out of a sample of about 1000 modules in OTP, about 50 of them are changed as a result of removing this optimization. The compiler in OTP 22 will do the same optimization in a cleaner, safer, and more effective way. https://bugs.erlang.org/browse/ERL-807
Diffstat (limited to 'lib/compiler')
-rw-r--r--lib/compiler/src/v3_codegen.erl136
-rw-r--r--lib/compiler/test/match_SUITE.erl22
2 files changed, 23 insertions, 135 deletions
diff --git a/lib/compiler/src/v3_codegen.erl b/lib/compiler/src/v3_codegen.erl
index e9152ba88f..d7a7778740 100644
--- a/lib/compiler/src/v3_codegen.erl
+++ b/lib/compiler/src/v3_codegen.erl
@@ -79,13 +79,9 @@ function(#k_fdef{anno=#k{a=Anno},func=Name,arity=Arity,
try
#k_match{} = Kb, %Assertion.
- %% Try to suppress the stack frame unless it is
- %% really needed.
- Body0 = avoid_stack_frame(Kb),
-
%% Annotate kernel records with variable usage.
Vdb0 = init_vars(As),
- {Body,_,Vdb} = body(Body0, 1, Vdb0),
+ {Body,_,Vdb} = body(Kb, 1, Vdb0),
%% Generate the BEAM assembly code.
{Asm,EntryLabel,St} = cg_fun(Body, As, Vdb, AtomMod,
@@ -98,136 +94,6 @@ function(#k_fdef{anno=#k{a=Anno},func=Name,arity=Arity,
erlang:raise(Class, Error, Stack)
end.
-
-%% avoid_stack_frame(Kernel) -> Kernel'
-%% If possible, avoid setting up a stack frame. Functions
-%% that only do matching, calls to guard BIFs, and tail-recursive
-%% calls don't need a stack frame.
-
-avoid_stack_frame(#k_match{body=Body}=M) ->
- try
- M#k_match{body=avoid_stack_frame_1(Body)}
- catch
- impossible ->
- M
- end.
-
-avoid_stack_frame_1(#k_alt{first=First0,then=Then0}=Alt) ->
- First = avoid_stack_frame_1(First0),
- Then = avoid_stack_frame_1(Then0),
- Alt#k_alt{first=First,then=Then};
-avoid_stack_frame_1(#k_bif{op=Op}=Bif) ->
- case Op of
- #k_internal{} ->
- %% Most internal BIFs clobber the X registers.
- throw(impossible);
- _ ->
- Bif
- end;
-avoid_stack_frame_1(#k_break{anno=Anno,args=Args}) ->
- #k_guard_break{anno=Anno,args=Args};
-avoid_stack_frame_1(#k_guard_break{}=Break) ->
- Break;
-avoid_stack_frame_1(#k_enter{}=Enter) ->
- %% Tail-recursive calls don't need a stack frame.
- Enter;
-avoid_stack_frame_1(#k_guard{clauses=Cs0}=Guard) ->
- Cs = avoid_stack_frame_list(Cs0),
- Guard#k_guard{clauses=Cs};
-avoid_stack_frame_1(#k_guard_clause{guard=G0,body=B0}=C) ->
- G = avoid_stack_frame_1(G0),
- B = avoid_stack_frame_1(B0),
- C#k_guard_clause{guard=G,body=B};
-avoid_stack_frame_1(#k_match{anno=A,vars=Vs,body=B0,ret=Ret}) ->
- %% Use #k_guard_match{} instead to avoid saving the X registers
- %% to the stack before matching.
- B = avoid_stack_frame_1(B0),
- #k_guard_match{anno=A,vars=Vs,body=B,ret=Ret};
-avoid_stack_frame_1(#k_guard_match{body=B0}=M) ->
- B = avoid_stack_frame_1(B0),
- M#k_guard_match{body=B};
-avoid_stack_frame_1(#k_protected{arg=Arg0}=Prot) ->
- Arg = avoid_stack_frame_1(Arg0),
- Prot#k_protected{arg=Arg};
-avoid_stack_frame_1(#k_put{}=Put) ->
- Put;
-avoid_stack_frame_1(#k_return{}=Ret) ->
- Ret;
-avoid_stack_frame_1(#k_select{var=#k_var{anno=Vanno},types=Types0}=Select) ->
- case member(reuse_for_context, Vanno) of
- false ->
- Types = avoid_stack_frame_list(Types0),
- Select#k_select{types=Types};
- true ->
- %% Including binary patterns that overwrite the register containing
- %% the binary with the match context may not be safe. For example,
- %% bs_match_SUITE:bin_tail_e/1 with inlining will be rejected by
- %% beam_validator.
- %%
- %% Essentially the following code is produced:
- %%
- %% bs_match {x,0} => {x,0}
- %% ...
- %% bs_match {x,0} => {x,1} %% ILLEGAL
- %%
- %% A bs_match instruction will only accept a match context as the
- %% source operand if the source and destination registers are the
- %% the same (as in the first bs_match instruction above).
- %% The second bs_match instruction is therefore illegal.
- %%
- %% This situation is avoided if there is a stack frame:
- %%
- %% move {x,0} => {y,0}
- %% bs_match {x,0} => {x,0}
- %% ...
- %% bs_match {y,0} => {x,1} %% LEGAL
- %%
- throw(impossible)
- end;
-avoid_stack_frame_1(#k_seq{arg=#k_call{anno=Anno,op=Op}=Call,
- body=#k_break{args=BrArgs0}}=Seq) ->
- case Op of
- #k_remote{mod=#k_atom{val=Mod},
- name=#k_atom{val=Name},
- arity=Arity} ->
- case erl_bifs:is_exit_bif(Mod, Name, Arity) of
- false ->
- %% Will clobber X registers. Must have a stack frame.
- throw(impossible);
- true ->
- %% The call to this BIF will never return. It is safe
- %% to suppress the stack frame.
- Bif = #k_bif{anno=Anno,
- op=#k_internal{name=guard_error,arity=1},
- args=[Call],ret=[]},
- BrArgs = lists:duplicate(length(BrArgs0), #k_nil{}),
- GB = #k_guard_break{anno=#k{us=[],ns=[],a=[]},args=BrArgs},
- Seq#k_seq{arg=Bif,body=GB}
- end;
- _ ->
- %% Will clobber X registers. Must have a stack frame.
- throw(impossible)
- end;
-avoid_stack_frame_1(#k_seq{arg=A0,body=B0}=Seq) ->
- A = avoid_stack_frame_1(A0),
- B = avoid_stack_frame_1(B0),
- Seq#k_seq{arg=A,body=B};
-avoid_stack_frame_1(#k_test{}=Test) ->
- Test;
-avoid_stack_frame_1(#k_type_clause{values=Values0}=TC) ->
- Values = avoid_stack_frame_list(Values0),
- TC#k_type_clause{values=Values};
-avoid_stack_frame_1(#k_val_clause{body=B0}=VC) ->
- B = avoid_stack_frame_1(B0),
- VC#k_val_clause{body=B};
-avoid_stack_frame_1(_Body) ->
- throw(impossible).
-
-avoid_stack_frame_list([H|T]) ->
- [avoid_stack_frame_1(H)|avoid_stack_frame_list(T)];
-avoid_stack_frame_list([]) -> [].
-
-
%% This pass creates beam format annotated with variable lifetime
%% information. Each thing is given an index and for each variable we
%% store the first and last index for its occurrence. The variable
diff --git a/lib/compiler/test/match_SUITE.erl b/lib/compiler/test/match_SUITE.erl
index e3f842b668..72e5356a8d 100644
--- a/lib/compiler/test/match_SUITE.erl
+++ b/lib/compiler/test/match_SUITE.erl
@@ -378,6 +378,13 @@ untuplify(Config) when is_list(Config) ->
%% We do this to cover sys_core_fold:unalias_pat/1.
{1,2,3,4,alias,{[1,2],{3,4},alias}} = untuplify_1([1,2], {3,4}, alias),
error = untuplify_1([1,2], {3,4}, 42),
+
+ %% Test that a previous bug in v3_codegen is gone. (The sinking of
+ %% stack frames into only the case arms that needed them was not always
+ %% safe.)
+ [33, -1, -33, 1] = untuplify_2(32, 65),
+ {33, 1, -33, -1} = untuplify_2(65, 32),
+
ok.
untuplify_1(A, B, C) ->
@@ -390,6 +397,21 @@ untuplify_1(A, B, C) ->
error
end.
+untuplify_2(V1, V2) ->
+ {D1,D2,D3,D4} =
+ if V1 > V2 ->
+ %% The 1 value was overwritten by the value of V2-V1.
+ {V1-V2, 1, V2-V1, -1};
+ true ->
+ {V2-V1, -1, V1-V2, 1}
+ end,
+ if
+ D2 > D4 ->
+ {D1, D2, D3, D4};
+ true ->
+ [D1, D2, D3, D4]
+ end.
+
%% Coverage of beam_dead:shortcut_boolean_label/4.
shortcut_boolean(Config) when is_list(Config) ->
false = shortcut_boolean_1([0]),