| Age | Commit message (Collapse) | Author |
|---|
|
* bjorn/compiler/beam_jump:
beam_jump: Eliminate pathologically slow compilation
|
|
* bjorn/compiler/beam_validator:
beam_validator: Exit immediately on crashes
beam_validator: Remove the file/1 and files/1 functions
beam_validator: Remove support for all other unsupported instructions
beam_validator: Remove support for unsupported bit syntax instructions
|
|
José Valim noticed that code such as:
match(1) -> 1;
match(2) -> 2;
match(3) -> 3;
...
match(1000) -> 1000.
would compile very slowly. The culprit is opt/3 in beam_jump.
What happens is that opt/3 will rewrite this code:
select_val ...
label 1
jump 1000
label 2
jump 1000
...
label 999
jump 1000
label 1000
return
very slowly to this code:
select_val ...
label 1
label 2
...
label 999
label 1000
return
The reason for the slowness is that when opt/3 sees this
sequence:
label 1
jump 1000
...
it will remove the label (storing it in a dictionary),
and pick up the previously processed instruction from
the accumulator:
select_val ...
jump 1000
label 2
jump 1000
...
That is done in order to process all labels before the
jump and also to get rid of the jump instruction if the
previous instruction is an "unreachable after". In this
case, re-processing the sequence will remove the now
unreachable jump instruction:
select_val ...
label 2
jump 1000
...
The problem is that re-processing the select_val instruction is
expensive. The instruction has a list of 1000 labels, all of which
will be added (again) to the set of referenced labels. The
select_val instruction will be re-processed again and again
until all labels and jumps have been gobbled up.
In the original version of beam_jump, opt/3 was not called
repeatedly until a fixpoint was found, but was expected to do
all its optimizations in one pass. The fixpoint iteration was
added later.
Since we now have the fixpoint iteration, there is no need
to do everything in a single pass. When we encounter a jump, we will
collect all previously seen labels and put them into the dictionary,
and then we will move on.
As a further optimization, we will look for sequences like this:
jump X
label ...
jump X
and replace them with:
label ...
jump X
In the example above, that will avoid 1000 updates of the dictionary.
After applying this optimization, compilation of the
pattern went from roughly 55 s to 0.1 s for the example
above but with 10000 clauses.
Reported-by: José Valim
|
|
The beam_validator catches all exceptions and collect them.
It makes more sense to don't catch 'error' and 'exit' exceptions,
but to just print out the name of the current function and pass
on the exception just as all other compilation passes do. Those
kind of exceptions are the symptoms of the kind of severe but
easily catched bugs that occur during development.
|
|
Before the beam_validator was added as compiler pass, it was a
standalone module that could analyse existing .beam files and .S
files.
Even though beam_validator has been part of the compiler for many
releases, it still supports the analysis of .beam and .S files.
To reduce the code bloat and to improve coverage of beam_validator,
remove the file/1 and files/1 functions and all associated help
functions. We'll need to update the test suite, since some of the
checked in .S files have errors that beam_validator ignores, but
that will not be accepted when running them throught the compiler
using the 'from_asm' option. In particular, we will need to export
all functions that should be validated (since the beam_clean pass
will remove any function that is not possible to call).
|
|
|
|
|
|
The assert_strict_literal_termorder/1 function is used to validate the
get_map_elements and has_map_fields instructions. In neither case is
it useful to allow an empty lists of fields, so we should no longer
allow an empty list.
The mmap/2 function is cute, but it is used in only one place, so it
is much simpler to write a special-purpose function to extract the
keys from the list of map pairs.
|
|
The has_map_fields test was not recognized in is_pure_test/1,
because beam_a has rewritten the {list,_} part of instruction.
|
|
|
|
|
|
|
|
There is no need to always introduce a new variable to hold a map.
Maps are novars (constructs that don't export variables).
|
|
In cd1eaf0116190, opt_simple_let_2/6 was updated to do the same
optimizations in 'value' and 'effect' context.
Coalesce the clauses for 'value' and 'effect' context to one to
make it clear that they do the same thing.
|
|
The code for inlining high-order functions from the lists module
is quite annoying when you try to navigate the sys_core_fold
module. Break out the code into its own module.
|
|
Those functions allow us to clean up some more code.
|
|
Introduce access functions to hide the low-level details of how
type information is implemented.
|
|
|
|
Essentially, core_lib:literal_value/1 became useless when literals
were introduced in R12. Since we always create #c_literal{} records
whenever possible, literal_value/1 would *only* succeed when it was
passed a #c_literal{} argument.
|
|
We are about to deprecate core_lib:get_anno/1 and core_lib:set_anno/2,
so we should stop using them in the compiler.
|
|
Attributes must be literals. Since 1fcdcd50, both core_parse and
v3_core guarantees all Core Erlang terms that may be represented as
literals in fact are represented as literals.
Therefore, we no longer need to call core_lib:is_literal/1, but
can test for a #c_literal{} directly.
|
|
This module is the last that uses -include() instead of
include_lib(). With that change, a quick smoke test can
be done simply like this:
PATH=$ERL_TOP/bin:$PATH erlc -W0 *.erl
Without the change, you would also need to add
-I $ERL_TOP/lib/test_server/include
to the command line.
|
|
Rename add_scope/2 to sub_add_scope/2 to be similar in naming as
the other functions that operates on #sub{} (in particular,
sub_subst_scope/1). Also, move the definition to be near to the
other sub_* functions.
|
|
Compiling the following function:
f(V) when not (bar and V) -> true; %Line 4
f(_) -> false.
would produce the following warnings:
no_file: Warning: the call to is_boolean/1 has no effect
t.erl:4: Warning: the guard for this clause evaluates to 'false'
t.erl:4: Warning: use of operator '=:=' has no effect
Two of the warnings refer to calls to is_boolean/1 and '=:='/2 which
v3_core added when translating the code to Core Erlang. The only
relevant warning is:
t.erl:4: Warning: the guard for this clause evaluates to 'false'
Suppress the other two warning by marking the compiler-generated
calls with a 'compiler_generated' annotation.
|
|
|
|
|
|
Core Erlang annotations are supposed to be a list of terms. v3_core
could temporarily stuff a record in the 'anno' field of a Core Erlang
record. That will cause Dialyzer warnings if we would tighten the
type specs for annotations. (We want to tighten the warnings in order
to catch more real problems.)
Avoid abusing the annotation by wrapping the entire Core Erlang
record in a #isimple{} record.
Reported-by: Kostis Sagonas
|
|
* bjorn/compiler/dup-bug-fix/OTP-12453:
Teach case_opt/3 to avoid unnecessary building
sys_core_fold: Optimize let statements more aggressively
Suppress warnings for expressions that are assigned to '_'
trace_bif_SUITE: Ensure that a call to time/0 is not removed
|
|
* bjorn/compiler/map-core-syntax/OTP-12454:
Make the syntax for map pairs readable
|
|
* maint:
Update primary bootstrap
Correct unsafe optimization of '==' and '/='
Conflicts:
bootstrap/lib/compiler/ebin/sys_core_fold.beam
|
|
Since '=:=' is cheaper than '==', the compiler tries to replace
'==' with '=:=' if the result of comparison will be the same.
As an example:
V == {a,b}
can be rewritten to:
V =:= {a,b}
since the literal on the right side contains no numeric values
that '==' would compare differently to '=:='.
With the introduction of maps, we will need to take them into
account. Since the comparison of maps is planned to change in 18.0,
we will be very conservative and only do the optimization if
both keys and values are non-numeric.
|
|
Given this code:
f(S) ->
F0 = F1 = {S,S},
[F0,F1].
case_opt/3 would "optimize" it like this:
f(S) ->
F1 = {S,S},
F0 = {S,S},
[F0,F1].
Similarly, this code:
g({a,_,_}=T) ->
{b,
[_,_] = [T,none],
x}.
would be rewritten to:
g({a,Tmp1,Tmp2}=T) ->
Tmp3 = {a,Tmp1,Tmp2},
{b,
[Tmp3,none],
x}.
where the tuple is rebuilt instead of using the T variable.
Rewrite case_opt/3 to be more careful while optimizing.
|
|
I originally decided that in 'value' context, rewriting a let statement
where the variables were not in the body to a sequence was not worth
it, because the variables would be unused in only one let in a
thousand lets (roughly).
I have reconsidered.
The main reason is that if we do the rewrite, core_lib:is_var_used/2
will be used much more frequently, which will help us to find bugs
in it sooner.
Another reason is that the way letify/2 is currently implemented
with its own calls to core_lib:is_var_used/2 is only safe as long
as all the bindings are independent of each other. We could make
letify/2 smarter, but if we introduce this new optimization there
is no need.
Measuring compilation speed, I have not seen any significant slowdown.
It seems that although core_lib:is_var_used/2 is called much more
frequently, most calls will be fast because is_var_used/2 will quickly
find a use of the variable.
Also add a test case to cover a line opt_guard_try/1 that was
no longer covered.
|
|
In c34ad2d5, the compiler learned to silence some warnings for
expressions that were explicitly assigned to the '_' variable,
as in this example:
_ = list_to_integer(S),
ok
That commit intentionally only made it possible to silence warnings
for BIFs that could cause an exception. Warnings would still be
produced for:
_ = date(),
ok
because date/0 can never fail and thus making the call completely
useless. The reasoning was that such warnings can always be
eliminated by eliminating the offending code.
While that is true, there is the question about rules and their
consistency. It is surprising that '_' can be used to silence
some warnings, but has no effect on other warnings.
Therefore, we will teach the compiler to silence warnings for
the following constructs:
* Calls to safe BIFs such as date/0
* Expressions that will cause an exception such as 'X/0'
* Terms that are built but not used, such as '{x,X}'
|
|
* 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
|
|
We must not convert map patterns to map expressions.
|
|
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.
|
|
Use the same syntax for map pairs in Core Erlang code as in the
Erlang Code. This Erlang code:
M#{x:=42,y=>1}
will look like this in Core Erlang:
~{'x':=42,'y'=>1|M}~
|
|
* 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.
|
