| Age | Commit message (Collapse) | Author |
|---|
|
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.
|
|
Maps have certain invariants that must be preserved:
(1) A map as a pattern must be represented as #c_map{} record,
never as a literal. The reason is that the pattern '#{}' will
match any map, not just the empty map. The literal '#{}' will
only match the empty map.
(2) In a map pattern, the key must be a literal, a variable, or
data (list or tuple). Keys that are binaries or maps *must* be
represented as literals.
(3) Maps in expressions should be represented as literals if possible.
Nothing is broken if this invariant is broken, but the generated
code will be less efficient.
To preserve invariant (1), cerl:update_c_map/3 must never collapse
a map to a literal. To preserve invariant (3), cerl:update_c_map/3
must collapse a map to a literal if possible.
To preserve both invariants, we need a way for cerl:update_c_map/3 to
know whether the map is used as a pattern or as an expression. The
simplest way is to have an 'is_pat' boolean in the #c_map{} record
which is set when a #c_map{} record is initially created.
We also need to update core_parse.yrl to establish the invariants
in the same way as v3_core, to ensure that compiling from a
.core file will work even if all optimizations on Core Erlang are
disabled.
|
|
|
|
We used to evaluate the body of a 'letrec' in value context, even
if the 'letrec' was being evaluated in effect context. In most
cases, the context does not matter because the body is usually
just an 'apply' which will never be optimized away.
However, in the case of incorrect code described in the previous
commit, it does matter. We can find such bad code by evaluating
the body in effect context. For example, if we have the following
incorrect code:
letrec
f/1 = fun(A) -> ... <use of Var> ...
in let Var = <<2:301>>
in apply(Arg)
If the letrec is evaluated in effect context, the code will be
reduced to:
letrec
f/1 = fun(A) -> ... <use of Var> ...
in seq Var = <<2:301>> do apply(Arg)
Now Var will be unbound and a later compiler pass will crash to
ensure that the bad Core Erlang code is noticed.
Also add a test case to ensure that the compiler crashes if the
bug fixed in the previous commit re-surfaces.
|
|
The translation of list comprehension with a map pattern
with a big literal binary as key such as:
lc(L) ->
[V || #{<<2:301>> := V} <- L].
would generate Core Erlang code where an unbound variable
were referenced:
'lc'/1 =
fun (L) ->
letrec
'lc$^0'/1 = fun (_cor4) ->
case _cor4 of
<[~{~<_cor1,V>}~|_cor3]> when 'true' ->
let <_cor5> = apply 'lc$^0'/1(_cor3)
in [V|_cor5]
<[_cor2|_cor3]> when 'true' ->
apply 'lc$^0'/1(_cor3)
<[]> when 'true' ->
[]
end
in let <_cor1> = #{#<2>(301,1,'integer',['unsigned'|['big']])}#
in apply 'lc$^0'/1(L)
In the map pattern in the 'case' in the 'letrec', the key is the
variable '_cor1' which should be bound in the enclosing environment.
It is not.
There is binding of '_cor1', but in the wrong place (at the end of
the function). Because of the way v3_kernel translates letrecs,
the code *happens* to work.
The code will break if Core Erlang optimizations were strengthened
to more aggressively eliminate variable bindings that are not used,
or if the translation from Core Erlang to Kernel Erlang were changed.
Correct the translation so that '_cor1' is bound in the environment
enclosing the 'letrec':
'lc'/1 =
fun (L) ->
let <_cor1> = #{#<2>(301,1,'integer',['unsigned'|['big']])}#
in letrec
'lc$^0'/1 = fun (_cor4) ->
case _cor4 of
<[~{~<_cor1,V>}~|_cor3]> when 'true' ->
let <_cor5> = apply 'lc$^0'/1(_cor3)
in [V|_cor5]
<[_cor2|_cor3]> when 'true' ->
apply 'lc$^0'/1(_cor3)
<[]> when 'true' ->
[]
end
in apply 'lc$^0'/1(L)
Unfortunately I was not able to come up with a test case that
demonstrates the bug.
|
|
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.
|
|
|
|
|
|
Remove ?line macros. Run test cases in parallel.
|
|
* egil/fix-maps-compiler-coverage/OTP-12425:
compiler: Rename util function to adhere to name policy
compiler: Remove get_map_elements label check in blocks
compiler: Remove unnecassary guard for get_map_elements
compiler: Remove dead code in beam_flatten
compiler: Increase Maps code coverage
|
|
* bjorn/compiler/coverage:
map_SUITE: Ensure recompilation when running cover
Add beam_utils_SUITE to cover more lines in beam_utils
beam_utils: Remove unreachable clauses in live_opt/4
receive_SUITE: Cover handling of recv_mark & recv_set in beam_utils
beam_validator_SUITE: Mend the compiler_bug/1 test case
beam_clean: Remove handling of forgotten instructions
compile_SUITE: Test the 'dialyzer' option
|
|
* maint:
Update primary bootstrap
core_lib: Handle patterns in map values
|
|
|
|
|
|
beam_utils:live_opt() is only invoked on code that has been
blockified by beam_block. Therefore the allocate/3 and
allocate_heap/4 instructions only occur in their transformed
form inside a block.
While we are it, correct a comment. 'asm' has been replaced
by 'from_asm'.
|
|
While we are it, also remove ?line macros in ref_opt_1/1 and correct
the indentation in do_ref_opt/2.
|
|
The compiler_bug/1 test case succeeded for the wrong reason. The
'asm' option is no longer supported (was ignored) and the compiler
looked for a .erl file.
Make sure that we don't fall for this trick again by making sure
that the error is reported from beam_validator.
|
|
core_lib:is_var_used/2 would not consider a variable used in the
value of a map pattern such as:
case Map of
#{key := <<42:N>>} -> ok
end
Here the variable 'N' would not be considered used.
It was assumed that there was no need to check map patterns at
all, since maps currently don't support variables in keys.
|
|
* oliv3/math_log2/OTP-12411:
Add math:log2/1
|
|
Commits b44f86b7 and 97953704 introduced translations of instructions
in beam_a and beam_z, but forgot to remove the handling of them in
beam_clean.
|
|
Cover more code in v3_core.
|
|
* maint:
Update primary bootstrap
beam_bool: Correct live calculation for GC BIFs
beam_bool: Correct indentation for try...catch
sys_core_fold: Correct optimization of 'case'
Conflicts:
bootstrap/bin/start.boot
bootstrap/bin/start_clean.boot
bootstrap/lib/compiler/ebin/beam_asm.beam
bootstrap/lib/stdlib/ebin/io_lib_pretty.beam
|
|
* bjorn/compiler/map-in-record-bug/OTP-12402:
sys_core_fold: Correct optimization of 'case'
|
|
|
|
When optimizing boolean expressions, it is not always possible
to find a number of live registers for a GC BIF that both preserves
all source registers that will be tested and at the same time
does not include registers that are not initialized.
As currently implemented, we have incomplete information about
the register calculated from the free variables. Some registers
are marked as "reserved". Reserved registers means that we don't
know anything about them; they may or may not be initialized.
As a conservative correction (suitable for a maintenance release), we
will abort the optimization if we find any reserved registers when
calculating the number of live registers. We will not attempt to
improve the information about the registers in this commit.
By examining the coverage when running the existing compiler test
suite we find that the optimization is aborted 15 times (before
adding any new test cases). To put that in perspective, the
optimization is successfully applied 4927 times, and aborted for
other reasons 547 times.
Reported-by: Ulf Norell
Reported-by: Anthony Ramine
|
|
Old versions of the Erlang mode for Emacs used to indent try...catch
strangely - the first clause following the 'catch' would be indented
with one space less than the following clauses.
If we are to use the new Erlang mode when we add more clauses, they
would be indented with one space less than the preceding clauses.
That would look silly.
|
|
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
|
|
* beam_utils:joineven/1 -> beam_utils:join_even/1
* beam_utils:split_even/1 -> beam_utils:split_even/1
|
|
The get_map_elements instruction has been removed from all blocks by the
mandatory beam_split pass and thus only needs handling by the outer loop.
|
|
|
|
No need to check for fail label zero for get_map_elements in beam_split.
get_map_elements is always used in pattern matching and never in a body.
|
|
The normalization in beam_flatten can never be reached for get_map_elements
since it is always handled in the mandatory beam_split pass.
|
|
This commit covers 'split_block_label_used' in the beam_bool pass for Maps.
|
|
|
|
While we are, clean up the comments and rearrange the code for
clarity. Also add a test to cover the last uncovered line in
beam_dead.erl.
|
|
Amend the test suite to call beam_dead as originally intended (and not
beam_block), and modify the input data so that the exception will
occur within the try ... catch block in function/2.
|
|
Better optimizations with less code.
|
|
The BEAM compiler translates code such as:
is_hex_digit(D) when $0 =< D, D =< $9 -> true;
is_hex_digit(D) when $a =< D, D =< $z -> true;
is_hex_digit(D) when $A =< D, D =< $Z -> true;
is_hex_digit(_) -> false.
to something like this:
L0: test is_ge L1 {x,0} 48
test is_ge L1 57 {x,0}
move true {x,0}
return.
L1: test is_ge L2 {x,0} 97
test is_ge L2 122 {x,0}
move true {x,0}
return
L2: test is_ge L3 {x,0} 65
test is_ge L3 90 {x,0}
move true {x,0}
return
L3: move false {x,0}
return
We can see that tests will be repeated even if they cannot possibly
succeed. For instance, if we pass in {x,0} equal to 32, the first
test that {x,0} is greater than or equal to 48 at L0 will fail.
The control will transfer to L1, where it will be tested whether
{x,0} is greater than 97. That test will fail and control
will pass to L2, where again the test will fail.
The compiler can do better by short-circuiting repeating tests:
L0: test is_ge L3 {x,0} 48
test is_ge L1 57 {x,0}
move true {x,0}
return.
L1: test is_ge L2 {x,0} 97
test is_ge L3 122 {x,0}
move true {x,0}
return
L2: test is_ge L3 {x,0} 65
test is_ge L3 90 {x,0}
move true {x,0}
return
L3: move false {x,0}
return
|
|
=== OTP-17.4 ===
Changed Applications:
- asn1-3.0.3
- common_test-1.9
- compiler-5.0.3
- crypto-3.4.2
- debugger-4.0.2
- dialyzer-2.7.3
- diameter-1.8
- edoc-0.7.16
- eldap-1.1
- erl_docgen-0.3.7
- erl_interface-3.7.20
- erts-6.3
- eunit-2.2.9
- hipe-3.11.2
- inets-5.10.4
- jinterface-1.5.12
- kernel-3.1
- megaco-3.17.3
- mnesia-4.12.4
- observer-2.0.3
- odbc-2.10.22
- otp_mibs-1.0.10
- parsetools-2.0.12
- percept-0.8.10
- runtime_tools-1.8.15
- snmp-5.1.1
- ssh-3.1
- ssl-5.3.8
- stdlib-2.3
- syntax_tools-1.6.17
- test_server-3.7.2
- tools-2.7.1
- wx-1.3.2
Unchanged Applications:
- cosEvent-2.1.15
- cosEventDomain-1.1.14
- cosFileTransfer-1.1.16
- cosNotification-1.1.21
- cosProperty-1.1.17
- cosTime-1.1.14
- cosTransactions-1.2.14
- et-1.5
- gs-1.5.16
- ic-4.3.6
- orber-3.7.1
- os_mon-2.3
- ose-1.0.2
- public_key-0.22.1
- reltool-0.6.6
- sasl-2.4.1
- typer-0.9.8
- webtool-0.8.10
- xmerl-1.3.7
Conflicts:
OTP_VERSION
erts/vsn.mk
|
|
|
|
This fixes a regression introduced in commit 805f9c89fc01220bc1bb0f27e1b68fd4eca688ba
The problem occured with map keys compiled with dialyzer option turned on.
In OTP 17, map keys needs to be literals.
|
|
* maint:
Fix miscompilation when module contains multiple named funs
Fix locations of shadowing warnings in ms_transform
|
|
Commit 78ce8917d started to use get_anno/1 to extract the line
annotation from filter qualifiers in comprehensions, but this does not
respect the spec of this function and resuls in a dialyzer warning.
To make the code more type-friendly, introduce a get_qual_anno/1
function.
Kostis Sagonas suggested that the function should be implemented
similar to this to also ensure that the qualifiers are of the
appropriate form:
get_qual_anno({call,Line,_,_}) -> Line;
get_qual_anno({op,Line,_,_,_}) -> Line;
.
.
.
get_qual_anno({var,Line,_}) -> Line.
The problem is that it is difficult to know exacly which forms
that may occur and the function will need to be updated if new
abstract forms are added. Thus this implementation would complicate
maintanance without any real payoff.
Reported-by: Kostis Sagonas
|
|
A module containing two named funs bearing the same name and arity could be
miscompiled.
Reported-by: Sam Chapin
|
|
* egil/map-type-opt/OTP-12253:
compiler: Type is_nonempty_list optimization
compiler: Type map optimization
|
|
* tuncer/compiler/finalize-asm-deprecation/OTP-12100:
compiler: finalize 18.x 'asm' deprecation
|
|
|
|
|
|
* egil/maps/variable-keys/OTP-12218: (22 commits)
compiler: Update test for Maps aliasing
compiler: Properly support Map aliasing
compiler: Refactor Map pairs aliasing
compiler: Fix harmless need_heap error for Maps
stdlib: Update Map tests
stdlib: Use environment bindings for Maps keys in erl_eval matching
debugger: Update Map tests
compiler: Update Map tests
compiler: Fix v3_core Maps pair chains
compiler: Use expressions in core patterns
compiler: Use variables in Map cerl inliner
compiler: Reintroduce binary limit for Map keys
compiler: Shameless v3_core hack for variables
compiler: Use variables in Map beam assmebler
compiler: Use variables in Map kernel pass
compiler: Use variables in Map core pass
compiler: Normalize unary ops on Maps key literals
stdlib: Update Map tests
stdlib: erl_lint Map key variables
compiler: Maps are always patterns never values in matching
...
|
|
|
