aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBjörn Gustavsson <[email protected]>2009-12-11 22:25:51 +0100
committerBjörn Gustavsson <[email protected]>2009-12-11 22:43:39 +0100
commit88efa63b733b627934fb7eec6236c71d7acfe082 (patch)
treeda691cf124f127927e85b7f30d8dd597a8e67235
parentaf198c489a7ab431fd1e2b52d16e8e13525915f7 (diff)
downloadotp-88efa63b733b627934fb7eec6236c71d7acfe082.tar.gz
otp-88efa63b733b627934fb7eec6236c71d7acfe082.tar.bz2
otp-88efa63b733b627934fb7eec6236c71d7acfe082.zip
beam_bool: Fix generation of code that does not validate
The following code (by Simon Cornish) bad(XDo1, XDo2, Do3) -> Do1 = (XDo1 =/= []), Do2 = (XDo2 =/= []), CH1 = if Do1 == true; Do1 == false,Do2==false,Do3 == blah -> ch1; true -> no end, CH2 = if Do1 == true; Do1 == false,Do2==false,Do3 == xx -> ch2; true -> no end, {CH1,CH2}. is optimized by beam_bool even though the optimization is not safe. The trouble is that an assignment to {y,0} no longer occurs on all paths leading to its use. The bug is in dst_regs/2 which is supposed to return a set of all registers assigned in a code block, but it ignores registers assigned in 'move' instructions. Fix the bug by taking 'move' instructions into account. This change is safe since it can only cause more registers to be added to the MustBeKilled and MustBeUnused sets in ensure_opt_safe/6, which means that it can only cause the optimization to be turned off for code that used to be optimized.
-rw-r--r--lib/compiler/src/beam_bool.erl12
-rw-r--r--lib/compiler/test/andor_SUITE.erl30
2 files changed, 37 insertions, 5 deletions
diff --git a/lib/compiler/src/beam_bool.erl b/lib/compiler/src/beam_bool.erl
index ffe5cdb501..dcc6ad4c7c 100644
--- a/lib/compiler/src/beam_bool.erl
+++ b/lib/compiler/src/beam_bool.erl
@@ -173,7 +173,7 @@ bopt_block(Reg, Fail, OldIs, [{block,Bl0}|Acc0], St0) ->
%% whether the optimized code is guaranteed to work in the same
%% way as the original code.
%%
-%% Throws an exception if the optmization is not safe.
+%% Throw an exception if the optimization is not safe.
%%
ensure_opt_safe(Bl, NewCode, OldIs, Fail, PreceedingCode, St) ->
%% Here are the conditions that must be true for the
@@ -190,10 +190,10 @@ ensure_opt_safe(Bl, NewCode, OldIs, Fail, PreceedingCode, St) ->
%% by the code that follows.
%%
%% 3. Any register that is assigned a value in the optimized
- %% code must be UNUSED or KILLED in the following code.
- %% (Possible future improvement: Registers that are known
- %% to be assigned the SAME value in the original and optimized
- %% code don't need to be unused in the following code.)
+ %% code must be UNUSED or KILLED in the following code
+ %% (because the register might be assigned the wrong value,
+ %% and even if the value is right it might no longer be
+ %% assigned on *all* paths leading to its use).
InitInPreceeding = initialized_regs(PreceedingCode),
@@ -310,6 +310,8 @@ dst_regs([{set,[D],_,{bif,_,{f,_}}}|Is], Acc) ->
dst_regs(Is, [D|Acc]);
dst_regs([{set,[D],_,{alloc,_,{gc_bif,_,{f,_}}}}|Is], Acc) ->
dst_regs(Is, [D|Acc]);
+dst_regs([{set,[D],_,move}|Is], Acc) ->
+ dst_regs(Is, [D|Acc]);
dst_regs([_|Is], Acc) ->
dst_regs(Is, Acc);
dst_regs([], Acc) -> ordsets:from_list(Acc).
diff --git a/lib/compiler/test/andor_SUITE.erl b/lib/compiler/test/andor_SUITE.erl
index 6e3ac4d4f4..a460d54239 100644
--- a/lib/compiler/test/andor_SUITE.erl
+++ b/lib/compiler/test/andor_SUITE.erl
@@ -390,6 +390,15 @@ before_and_inside_if(Config) when is_list(Config) ->
?line yes = before_and_inside_if([], [], x),
?line yes = before_and_inside_if([], [b], delete),
?line yes = before_and_inside_if([], [b], x),
+
+ ?line {ch1,ch2} = before_and_inside_if_2([a], [b], blah),
+ ?line {ch1,ch2} = before_and_inside_if_2([a], [b], xx),
+ ?line {ch1,ch2} = before_and_inside_if_2([a], [], blah),
+ ?line {ch1,ch2} = before_and_inside_if_2([a], [], xx),
+ ?line {no,no} = before_and_inside_if_2([], [b], blah),
+ ?line {no,no} = before_and_inside_if_2([], [b], xx),
+ ?line {ch1,no} = before_and_inside_if_2([], [], blah),
+ ?line {no,ch2} = before_and_inside_if_2([], [], xx),
ok.
%% Thanks to Simon Cornish and Kostis Sagonas.
@@ -408,6 +417,27 @@ before_and_inside_if(XDo1, XDo2, Do3) ->
yes
end.
+%% Thanks to Simon Cornish.
+%% Used to generate code that would not set {y,0} on
+%% all paths before its use (and therefore fail
+%% validation by the beam_validator).
+before_and_inside_if_2(XDo1, XDo2, Do3) ->
+ Do1 = (XDo1 =/= []),
+ Do2 = (XDo2 =/= []),
+ CH1 = if Do1 == true;
+ Do1 == false,Do2==false,Do3 == blah ->
+ ch1;
+ true ->
+ no
+ end,
+ CH2 = if Do1 == true;
+ Do1 == false,Do2==false,Do3 == xx ->
+ ch2;
+ true ->
+ no
+ end,
+ {CH1,CH2}.
+
%% Utilities.
check(V1, V0) ->