| Age | Commit message (Collapse) | Author |
|---|
|
The following sequence would be wrongly optimized into a
i_get_tuple_element2 instruction, reading an element from the
wrong tuple:
{get_tuple_element,{x,0},1,{x,0}}.
{get_tuple_element,{x,0},2,{x,1}}.
|
|
Prior to 294d66a295f6c2101fe3c2da630979ad4e736c08 there wasn't much
point to keeping track of tuple element types; they were only known
when we had inserted or extracted values from a tuple, and in
neither case was it likely that we'd extract the same values again.
It makes a lot more sense to do so now that type optimizations are
applied across functions; if we return a tuple it's very likely
that its elements will be extracted soon after, and knowing their
types lets us eliminate more type checks.
Co-authored-by: Björn Gustavsson <[email protected]>
|
|
Introduce subtraction of types to allow some redundant tests to be
eliminated.
Consider this function:
foo(L) when is_list(L) ->
case L of
[_|_] -> non_empty;
[] -> empty
end.
After entering the body of the function, L is known to be either
a cons cell or nil (otherwise the is_list/1 guard would have failed).
If the L is not a cons cell, it must be nil. Therefore, the test
for nil in the second clause of the case can be eliminated.
Here is the SSA code with some additonal comments for the function
before the optimization:
function t:foo(_0) {
0:
@ssa_bool = bif:is_list _0
br @ssa_bool, label 4, label 3
4:
%% _0 is now a list (cons or nil).
@ssa_bool:8 = is_nonempty_list _0
br @ssa_bool:8, label 9, label 7
9:
ret literal non_empty
7:
%% _0 is not a cons (or we wouldn't be here).
%% Subtracting cons from the previously known type list
%% gives that _0 must be nil.
@ssa_bool:10 = bif:'=:=' _0, literal []
br @ssa_bool:10, label 11, label 6
11:
ret literal empty
6:
_6 = put_tuple literal case_clause, _0
%% t.erl:5
@ssa_ret:12 = call remote (literal erlang):(literal error)/1, _6
ret @ssa_ret:12
3:
_9 = put_list _0, literal []
%% t.erl:4
@ssa_ret:13 = call remote (literal erlang):(literal error)/2, literal function_clause, _9
ret @ssa_ret:13
}
Type subtraction gives us that _0 must be nil in block 7, allowing us to
remove the comparison of _0 with nil. The code for the function can be simplified
to:
function t:foo(_0) {
0:
@ssa_bool = bif:is_list _0
br @ssa_bool, label 4, label 3
4:
@ssa_bool:8 = is_nonempty_list _0
br @ssa_bool:8, label 9, label 11
9:
ret literal non_empty
11:
ret literal empty
3:
_9 = put_list _0, literal []
%% t.erl:4
@ssa_ret:13 = call remote (literal erlang):(literal error)/2, literal function_clause, _9
ret @ssa_ret:13
}
|
|
* maint:
Remove unsafe optimization for delaying creation of stackframe
Conflicts:
lib/compiler/src/v3_codegen.erl
|
|
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
|
|
|
|
|
|
Some lines in beam_peep were no longer covered when the sharing optimization
was added to beam_ssa_opt. Also remove some code from beam_peep that no
longer seems possible to cover.
|
|
Add beam_ssa_dead to perform the main optimizations done
by beam_dead:
* Shortcut branches that jump to another block with a branch.
If it can be seen that the second branch will always branch
to a specific block, replace the target of the first branch.
* Combined nested sequences of '=:=' tests and switch
instructions operating on the same variable to a single switch.
Diffing the compiler output, it seems that beam_ssa_dead finds
many more opportunities for optimizations than beam_dead,
although it does not find all opportunities that beam_dead does.
In total, beam_ssa_dead is such improvement over beam_dead that
there is no reason to keep beam_dead as well as beam_ssa_dead.
Note that beam_ssa_dead does not attempt to optimize away
redundant bs_context_binary instructions, because that instruction
will be superseded by new instructions in the near future.
|
|
Add more instructions to the list of functions that can be safely
removed if their values are not used. This is necessary for
correctness when doing more aggressive optimizations. Without this
change, the 'succeeded' instruction could be optimized away leaving
just the instruction followed by an unconditional branch, which the
beam_ssa_codegen does not know how to handle. Here is an example:
_3 = bs_start_match _1
br label 13
By adding bs_start_match to the list, the bs_start_match instruction
will be removed too. (If the result of bs_start_match is actually
used, the succeeded instruction would not be removed.)
While we are it, rename the misnamed function is_pure/1 to
no_side_effect/1 and move it to beam_ssa. is_pure/1 is a bad name
because bif:get has no side effect, but is not pure.
|
|
Call test_lib:recompile/1 from init_per_suite/1 instead of
from all/0. That makes it easy to find the log from the
compilation in the log file for the init_per_suite/1 test
case.
|
|
|
|
|
|
The uncovered line was added in 6753bbcc3fdb0.
|
|
When cleaning selects, it might happen we're left with only one pair. In such
case convert to a regular test + jump.
|
|
Don't only test the case that failed; test it exhaustively.
|
|
The following regression was introduced in 19.0:
foo(bar, <<"x">>) -> 1;
foo(_, <<"x">>) -> 2;
foo(_, <<"y">>) -> 3;
foo(_, _) -> fail.
The call foo(bar,<<"y">>) would errorneous return 'fail' instead of 3.
A testcase in match_SUITE has been added to verify this.
|
|
a3ec2644f5 attempted to teach v3_core not to generate code with
unbound variables. The approach taken in that commit is to
discard all expressions following a badmatch. That does not
work if the badmatch is nested:
{[V] = [] = foo,V},
V
That would be rewritten to:
{error({badmatch,foo})},
V
where V is unbound.
If we were to follow the same approach, the tuple construction
code would have to look out for a badmatch. As would list construction,
begin...end, and so on.
Therefore, as it is impractical to discard all expressions that
follow a badmatch, the only other solution is to ensure that the
variables that the pattern binds will somehow be bound. That can
be arranged by rewriting the pattern to a pattern that binds the
same variables. Thus:
error({badmatch,foo}),
E = foo,
case E of
{[V],[]} ->
V;
Other ->
error({badmatch,Other}
end
|
|
v3_core would generate unsafe code for the following example:
f() ->
{ok={error,E}} = foo(),
E.
Internally, the code would look similar to:
f() ->
Var = foo(),
error({badmatch,Var}),
E.
That is, there would remain a reference to an unbound variable.
Normally, sys_core_fold would remove the reference to 'E', but if
if optimization was disabled the compiler would crash.
|
|
|
|
|
|
|
|
As a first step to removing the test_server application as
as its own separate application, change the inclusion of
test_server.hrl to an inclusion of ct.hrl and remove the
inclusion of test_server_line.hrl.
|
|
|
|
|
|
Duplicated variables as aliases in patterns, such as:
f({_,_}=Dup=Dup) -> ...
will work, but produce sub-optimal code similar to:
f({_,_}=Dup=NewVar) when Dup =:= NewVar -> ...
with one extra guard test for each duplicated variable.
Rewrite pat_alias/2 to eliminate all duplicated variables. While
we are at it, also simplify handling of tuples, conses, and literals
by using the data functions in the cerl module.
|
|
* maint:
Update primary bootstrap
Be more careful about map patterns when evalutating element/2
Do not convert map patterns to map expressions
Conflicts:
bootstrap/lib/compiler/ebin/sys_core_fold.beam
lib/compiler/test/match_SUITE.erl
|
|
In code such as:
case {a,Map} of
{a,#{}}=T ->
T
end
we must NOT rewrite a map pattern to a map expression like this:
case Map of
#{} ->
{a,#{}}
end
because the pattern '#{}' will match any map, but the expression
'#{}' will construct an empty map.
|
|
* bjorn/compiler/map-fixes:
cerl: Remove a clause in fold_map_pairs/3 that will never be reached
Move grouping of map constructions from v3_core to v3_kernel
core_pp: Correct printing of map literals
Strengthen and modernize compile_SUITE
core_parse: Always fold literal conses
cerl: Make sure that we preserve the invariants for maps
cerl_clauses: Fix indentation
sys_core_fold: Strengthen optimization of letrecs in effect context
Fix handling of binary map keys in comprehensions
core_lib: Teach is_var_used/2 to handle keys in map patterns
warnings_SUITE: Eliminate compiler warning for a shadowed variable
lc_SUITE: Add shadow/1
Modernize lc_SUITE
|
|
I have spent too much time lately waiting for 'cover' to finish,
so now its time to optimize the running time of the tests suite
in coverage mode.
Basically, when 'cover' is running, the test suites would not
run any tests in parallel. The reason is that using too many
parallel processes when running 'cover' would be slower than
running them sequentially. But those measurements were made
several years ago, and many improvements have been made to
improve the parallelism of the run-time system.
Experimenting with the test_lib:p_run/2 function, I found that
increasing the number of parallel processes would speed up the
self_compile tests cases in compilation_SUITE. The difference
between using 3 processes or 4 processes was slight, though,
so it seems that we should not use more than 4 processes when
running 'cover'.
We don't want to change test_lib:parallel/0, because there is
no way to limit the number of test cases that will be run in
parallel by common_test. However, there as test suites (such as
andor_SUITE) that don't invoke the compiler at run-time. We can
run the cases in such test suites in parallel even if 'cover'
is running.
|
|
is_var_used/2 did not notice that variable keys in map patterns
were used, which could cause sys_core_fold to do unsafe
optimizations.
|
|
The optimization of a 'case' statement could lead to incorrect
code that would cause an exception at run-time.
Here is an example to show how the optimization went wrong. Start
with the following code:
f({r,#{key:=Val},X}=S) ->
case S of
{r,_,_} ->
setelement(3, Val, S)
end.
(The record operations have already been translated to the
corresponding tuple operations.) The first step in case_opt/3 is
to substitute S to obtain:
f({r,#{key:=Val},X}=S) ->
case {r,#{key:=Val},X} of
{r,_,_} ->
setelement(3, Val, S)
end.
After that substitution the 'case' can be simplified to:
f({r,#{key:=Val},_}=S) ->
case #{key:=Val} of
NewVar ->
setelement(3, Val, S)
end.
That is the result from case_opt/3. Now eval_case/2 notices
that since there is only one clause left in the 'case', the
'case' can eliminated:
f({r,#{key:=Val},_}=S) ->
NewVar = #{key:=Val},
setelement(3, Val, S).
Since the map construction may have a side effect, it was not
eliminated, but assigned to a variable that is never used.
The problem is that '#{key:=Val}' is fine as a pattern, but in a
construction of a new map, the '=>' operator must be used. So the
map construction will fail, generating an exception.
As a conservative correction for a maintenance release, we will
abort the 'case' optimization if the substitution into the 'case'
expression is anything but data items (tuples, conses, or
literals) or variables.
Reported-by: Dmitry Aleksandrov
|
|
|
|
Run testcases in parallel will make the test suite run slightly
faster. Another reason for this change is that we want more testing
of parallel testcase support in common_test.
|
|
|
|
In 3d0f4a3085f11389e5b22d10f96f0cbf08c9337f (an update to conform
with common_test), in all test_lib:recompile(?MODULE) calls, ?MODULE
was changed to the actual name of the module. That would cause
test_lib:recompile/1 to compile the module with the incorrect
compiler options in cloned modules such as record_no_opt_SUITE,
causing worse coverage.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
* bg/compiler-suppress-result-ignored:
compiler tests: Eliminate "result of expression is ignored" warnings
Silence warnings for expressions that are assigned to "_"
OTP-8602 bg/compiler-suppress-result-ignored
It is now possible to suppress the warning in code such as
"list_to_integer(S), ok" by assigning the ignored value "_" like this: "_ =
list_to_integer(S), ok".
|
|
There is currently no zero-cost way to silence the warning
"the result of the expression is ignored", which is issued
for code such as:
list_to_integer(S),
ok
Such code can be useful for assertions or input validation.
Teach the compiler to silence the warning for expressions
that are explicitly assigned to to the "_" variable,
such as:
_ = list_to_integer(S),
ok
Implement it by having the v3_core pass annotate calls in
Core Erlang like this:
let <_> = ( call 'erlang':'list_to_integer'(S) -| ['result_not_wanted'] )
in 'ok'
and modifiy sys_core_fold to suppress the warning for any call
having the annotation.
We deliberately do not make it possible to silence the warnings
for expressions like:
{build,an,unnecessary,term}, ok
or
is_list(L), ok
because we don't know of any real-world scenarios in which that would
be useful.
|
|
|
