Age | Commit message (Collapse) | Author |
|
* 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
|
|
|
|
When translating a function with map construction:
f(A) ->
B = b,
C = c,
#{A=>1,B=>2,C=>3}.
v3_core would break apart the map construction into three
parts because of the way the map instructions in BEAM work --
variable keys need to be in their own instruction.
In the example, constant propagation will turn two of the
keys to literal keys. But the initial breaking apart will
not be undone, so there will still be three map constructions:
'f'/1 =
fun (_cor0) ->
let <_cor3> = ~{::<_cor0,1>}~
in let <_cor4> = ~{::<'b',2>|_cor3}~
in ~{::<'c',3>|_cor4}~
It would be possible to complicate the sys_core_fold pass
to regroup map operations so that we would get:
'f'/1 =
fun (_cor0) ->
let <_cor3> = ~{::<_cor0,1>}~
in ~{::<'b',2>,::<'c',3>|_cor3}~
A simpler way that allows to simplify the translation is
to skip the grouping in v3_core and translate the function
to:
'f'/1 =
fun (_cor0) ->
~{::<_cor0,1>,::<'b',2>,::<'c',3>}~
We will then let v3_kernel do the grouping while translating
from Core Erlang to Kernel Erlang.
|
|
A map key in a pattern would be incorrectly pretty-printed.
As an example, the pattern in:
x() ->
#{ #{ a => 3 } := 42 } = X.
would be pretty-printed as:
<~{~<~{~<'a',3>}~,42>}~
instead of:
<~{~<~{::<'a',3>}~,42>}~
When this problem has been corrected, the workaround for it in
cerl:ann_c_map/3 can be removed. The workaround was not harmless,
as it would cause the following map update to incorrectly succeed:
(#{})#{a:=1}
|
|
sys_core_fold:eval_element/3 attempts to evaluate calls to element/2
at compile time or to warn when the call will obviously fail. For
example:
element(1, [a])
will obviously fail and eval_element/3 will produce a warning.
eval_element/3 uses the helper functions is_not_integer/1 and
is_not_tuple/1 to test whether the arguments are known to be
incorrect. The clauses that attempt to match #c_map{} in those
helper function will never be executed, because #c_map{} will
never occur directly in an argument for a function call.
For example, code such as:
element(1, #{a=>Val})
will be translated to:
let <NewVar> = #{a=>Val}
in element(1, NewVar)
since maps are not considered safe (some map operations may
cause an exception at run time).
|
|
When we compile from Core Erlang, do it with and without
Core Erlang optimizations to ensure that we are not dependent
on the optimizations always being run.
|
|
v3_core is careful to always create literals whenever possible.
Correct core_parse so it, too, always creates literals out
of literal conses. With that correction, we can remove the
workaround in sys_core_fold (introduced in 26a5dea3cb5e101)
that handles non-literal flags in a binary.
|
|
It was a workaround for a bug that has been fixed.
|
|
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
|