Age | Commit message (Collapse) | Author |
|
The compiler would crash when compiling code such as:
(A / B) band 16#ff
The type for the expression would be 'none', but beam_type:verified_type/1
did not handle 'none'.
https://bugs.erlang.org/browse/ERL-829
|
|
beam_type assumed that the operand for the bs_context_to_binary
instruction must be a binary. That is not correct;
bs_context_to_binary accepts anything. Based on the incorrect
assumption, beam_type would remove other test instructions.
The bug was introduced in eee8655788d2, which was supposed
to be just a refactoring commit.
https://bugs.erlang.org/browse/ERL-655
|
|
|
|
This complements the `map_get/2` guard BIF introduced in #1784.
Rationale.
`map_get/2` allows accessing map fields in guards, but it might be
problematic in more complex guard expressions, for example:
foo(X) when map_get(a, X) =:= 1 or is_list(X) -> ...
The `is_list/1` part of the guard could never succeed since the
`map_get/2` guard would fail the whole guard expression. In this
situation, this could be solved by using `;` instead of `or` to separate
the guards, but it is not possible in every case.
To solve this situation, this PR proposes a `is_map_key/2` guard that
allows to check if a map has key inside a guard before trying to access
that key. When combined with `is_map/1` this allows to construct a
purely boolean guard expression testing a value of a key in a map.
Implementation.
Given the use case motivating the introduction of this function, the PR
contains compiler optimisations that produce optimial code for the
following guard expression:
foo(X) when is_map(X) and is_map_key(a, X) and map_get(a, X) =:= 1 -> ok;
foo(_) -> error.
Given all three tests share the failure label, the `is_map_key/2` and
`is_map/2` tests are optimised away.
As with `map_get/2` the `is_map_key/2` BIF is allowed in match specs.
|
|
Make sure to kill all dependencies when a register is killed. For example,
in the following code, when the type information for {x,0} is killed in
the last instruction, there will still be type information for {x,1} referring
to {x,0}:
{get_tuple_element,{x,0},0,{x,1}}.
{test,is_eq_exact,{f,5},[{x,1},{atom,tag}]}.
{get_tuple_element,{x,0},1,{x,2}}.
{get_tuple_element,{x,0},2,{x,0}}.
This does not seems to have caused any problems in the past, but it may
cause problems in the future with a register allocator that reuses register
more aggressively.
|
|
|
|
The function tdb_update/2 is problematic. It does not distinguish
between assigning a register with a new value and updating information
about a register that is used a as source in a test instruction.
That was not a problem in practice when there were very few types,
but bugs started to be noticed as more types were added. (For example,
when a register was overwritten with a new value, the type for the
old value stored in the same register could linger in some cases.)
Introduce separate functions tdb_store/3 and tdb_meet/3 for assigning
a new value to a register and for updating type information for a
register referenced as as source, respectively. Also stricten
verification of the types that gets stored into the type database.
|
|
Eliminate get_list/3 internally in the compiler
|
|
* maint:
Fix incorrect type interference of integer ranges
Conflicts:
lib/compiler/src/beam_type.erl
|
|
|
|
Instructions that produce more than one result complicate
optimizations. get_list/3 is one of two instructions that
produce multiple results (get_map_elements/3 is the other).
Introduce the get_hd/2 and get_tl/2 instructions
that return the head and tail of a cons cell, respectively,
and use it internally in all optimization passes.
For efficiency, we still want to use get_list/3 if both
head and tail are used, so we will translate matching pairs
of get_hd and get_tl back to get_list instructions.
|
|
Optimize away unnecessary test_unit instructions that verify that
binaries are byte-aligned. In a tight loop, eliminating an
instruction can have a small but measurable improvement of the
execution time.
|
|
Separate the simplification of instructions from updating of the
type data base.
|
|
The annotations in the optimizing passes currently looks like this:
{'%live',NumRegistersUsed,RegistersUsedBitmap}
{'%def',RegistersDefinedBitmap}
(NumRegistersUsed is no longer used.)
When I attempted to extend some optimizations, I found that I had to
add additional clauses to tolerate/handle both types of
annotations. That problem would only get worse if any more annotations
are added in the future.
To simplify annotation handling, this commit wraps both types of
annotations in a {'%anno',_} tuple:
{'%anno',{used,RegistersUsedBitmap}}
{'%anno',{def,RegistersDefinedBitmap}}
The '%live' annotation has been renamed to 'used' to make it somewhat
clearer what it means, and the unused NumRegistersUsed part of the
old annotation has been removed.
Alternatives considered: My first attempt was to wrap the annotation
in a 'set' tuple so that there would only be 'set' tuples in a block.
For example:
{set,[],[],{anno,{live,RegistersUsedBitmap}}}
It was not as convenient as expected. Annotations often need to be
handled specially from other instructions in a block. When they are
wrapped in a 'set' tuple, they can very easily be handled incorrectly
or passed on to the next pass. That causes subtle errors or worse
code, and it can be difficult to debug.
Therefore, my conclusion is that annotations should be distinct from
other instructions, to make it obvious when one have missed to handle
an annotation.
|
|
jhogberg/john/compiler/reintroduce-tuple-arity-optimizations/OTP-14857
Reintroduce the tuple arity optimizations removed in PR #1673
|
|
An 'allocate' or 'allocate_zero' instruction should not be
shortly followed by a 'test_heap' instruction. For example,
we don't want this type of code:
{allocate_zero,3,4}.
{line,...}.
{test_heap,7,4}.
{bif,element,{f,0},...,...}.
While the code is safe because 'allocate_zero' has initialized the
stack frame, it is wasteful. Also note that the code would become
unsafe if the 'allocate_zero' instruction were to be replaced with
an 'allocate' instruction.
What we want to see is this:
{allocate_heap_zero,3,7,4}.
{line,...}.
{bif,element,{f,0},...,...}.
|
|
* maint:
beam_validator: Strengthen validation of GC instructions
|
|
beam_validator: Strengthen validation of GC instructions
OTP-14863
|
|
We can safely tell when a test_arity or is_record instruction is
superflous by keeping track of whether the size is exactly known
or not.
|
|
|
|
beam_validator did not verify that the Y registers were initialized
before executing the following instructions that could cause a GC:
bs_append/8
bs_init2/6
bs_init_bits/6
gc_bif1/5
gc_bif2/6
gc_bif3/7
test_heap/2
That means that, for example, an incorrect optimization that replaced
an 'allocate_zero' instruction with an 'allocate' instruction when it
was not safe, would not be rejected by beam_validtor, but would
instead cause a crash or other undefined behavior at runtime.
Also fix a minor bug in beam_type exposed by the stronger checking.
When compiling from .S files, beam_type did not handle the
init/1 instruction and could produce unsafe code.
|
|
The type optimizations for is_record and test_arity checked whether
the arity was equal to the size stored in the type information,
which is incorrect since said size is the *minimum* size of the
tuple (as determined by previous instructions) and not its exact
size.
A future patch to the 'master' branch will restore these
optimizations in a safe manner.
|
|
|
|
|
|
* kill type information only for affected registers in get_map_elements
* bs_get_utf* will produce integers of unicode range
This optimises code created by Elixir compiler, where:
<<x::utf8,_::binary>> when x in 1..10
will compile the guard to
is_integer(X) andalso X >= 1 andalso X =< 10
This allows us to eliminate the is_integer check.
* bs_get_float will produce a float
* allow to carry type information over other bs instructions killing
only the affected registers
* kill only x registers after call_fun and apply instructions
|
|
Code such as the following:
-record(x, {a}).
f(R, N0) ->
N = N0 / 100,
if element(1, R#x.a) =:= 0 ->
N
end.
would fail to compile with the following message:
m: function f/2+19:
Internal consistency check failed - please report this bug.
Instruction: {fmove,{fr,0},{x,1}}
Error: {uninitialized_reg,{fr,0}}:
This bug was introduced in 348b5e6bee2f.
Basically, the beam_type pass placed the fmove instruction in the
wrong place. Instructions that store to floating point registers and
instructions that read from floating point registers are supposed to
be in the same basic block.
Fix the problem by flushing all floating points instruction
before a call the pseudo-BIF is_record/3, thus making sure that
the fmove instruction is placed in the correct block.
Here is an annotated listing of the relevant part of the .S
file (before the fix):
{test_heap,{alloc,[{words,0},{floats,1}]},2}.
{fconv,{x,1},{fr,0}}.
{fmove,{float,100.0},{fr,1}}.
fclearerror.
{bif,fdiv,{f,0},[{fr,0},{fr,1}],{fr,0}}.
{fcheckerror,{f,0}}.
%% The instruction {fmove,{fr,0},{x,1}} should have
%% been here.
%% Block of instructions expanded from a call to
%% the pseudo-BIF is_record/3. (Expanded in a later
%% compiler pass.)
{test,is_tuple,{f,3},[{x,0}]}.
{test,test_arity,{f,3},[{x,0},2]}.
{get_tuple_element,{x,0},0,{x,2}}.
{test,is_eq_exact,{f,3},[{x,2},{atom,x}]}.
{move,{atom,true},{x,2}}.
{jump,{f,4}}.
{label,3}.
{move,{atom,false},{x,2}}.
{label,4}.
%% End of expansion.
%% The fmove instruction that beam_validator complains
%% about.
{fmove,{fr,0},{x,1}}.
Reported-by: Richard Carlsson
|
|
|
|
The beam_type may pass move and recalculates test_heap instructions.
The number of live registers are not always the lowest. Minimize the
number of registers by running beam_utils:live_opt/1 one more time.
|
|
|
|
Add math:floor/1 and math:ceil/1 to avoid unnecessary conversions
in floating point expressions. That is, instead of having to write
float(floor(X)) as part of a floating point expressions, we can
write simply math:floor(X).
|
|
The following code:
simple() ->
case try 0 after [] end of
0 -> college;
1 -> 0
end.
would crash the compiler like this:
crash reason: {case_clause,
{'EXIT',
{function_clause,
[{beam_type,simplify_select_val_int,
[{select,select_val,
{x,0},
{f,7},
[{integer,1},{f,9},{integer,0},{f,8}]},
0],
[{file,"beam_type.erl"},{line,169}]},
{beam_type,simplify_basic_1,3,
[{file,"beam_type.erl"},{line,155}]},
{beam_type,opt,3,[{file,"beam_type.erl"},{line,57}]},
{beam_type,function,1,[{file,"beam_type.erl"},{line,36}]},
{beam_type,'-module/2-lc$^0/1-0-',1,
[{file,"beam_type.erl"},{line,30}]},
{beam_type,module,2,[{file,"beam_type.erl"},{line,30}]},
{compile,'-select_passes/2-anonymous-2-',2,
[{file,"compile.erl"},{line,521}]},
{compile,'-internal_comp/4-anonymous-1-',2,
[{file,"compile.erl"},{line,306}]}]}}}
The root cause is that the type representation is not well-defined.
Integers could be represented in three different ways:
integer
{integer,{1,10}}
{integer,0}
However, only the first two forms were handled.
To avoid similar problems in the future:
* Make the type representation stricter. Make sure that integers are
only represented as 'integer' or {integer,{Min,Max}}.
* Call verify_type/1 whenever a new type is added (not only when
merging types) to ensure that only the supported types are added
to the type database).
(ERL-150)
|
|
We must be careful how we treat the type info for the result of:
setelement(Index, Tuple, NewValue)
If Tuple had type information, the result of setelement/3 (in x(0))
would be assigned the same type information. But that is not safe
for:
setelement(1, Tuple, NewValue)
since the type for the first element will be changed.
Therefore, we must take care to remove the type information for
the first element of the tuple if might have been modified by
setelement/3.
|
|
|
|
There is an optimization in beam_block to simplify a select_val
on a known boolean value. We can implement this optimization
in a cleaner way in beam_type and it will also be applicable
in more situations. (When I added the optimization to beam_type
without removing the optimization from beam_block, the optimization
was applied 66 times.)
|
|
The ASN.1 compiler often generates code similar to:
f(<<0:1,...>>) -> ...;
f(<<1:1,...>>) -> ....
Internally that will be rewritten to (conceptually):
f(<<B:1,Tail/binary>>) ->
case B of
0 ->
case Tail of ... end;
1 ->
case Tail of ... end;
_ ->
error(case_clause)
end.
Since B comes from a bit field of one bit, we know that the only
possible values are 0 and 1. Therefore the error clause can be
eliminated like this:
f(<<B:1,Tail/binary>>) ->
case B of
0 ->
case Tail of ... end;
_ ->
case Tail of ... end
end.
Similarly, we can also a deduce the range for an integer from
a 'band' operation with a literal integer.
While we are at it, also add a test case to improve the coverage.
|
|
The clause cannot possibly match, because there will always be
a {bif,...} clause that will match before reaching the fclearerror
instruction.
|
|
30cc5c90 changed the internal representation of catch and
try...catch, but beam_type was not updated in one place.
|
|
Put 'try' instructions inside block to improve the optimization
of allocation instructions. Currently, the compiler only looks
at initialization of y registers inside blocks when determining
which y registers that will be "naturally" initialized.
|
|
|
|
|
|
Profiling shows that the excution time for checkerror_1/2 could
be be near the top even for modules without any floating point
operations.
It turns out that the complexity of simplify_float_1/4 is quadratic.
checkerror/1 is called with the growing accumulator for each
iteration. checkerror/1 will traverse the entire accumulated list
*unless* some floating point operations are used.
We can avoid this situation if we only call checkerror/1 when there
are live floating point registers. We can also avoid calling flush/3
if there are no live floating point registers.
|
|
When calculating the number of live registers for allocation
instruction, it is not always safe to start with the number of live
registers at the start of the block. We will need to use the register
map to know whether there are any holes (dead registers) that are not
subsequently filled.
If the allocation instruction already has a number of live registers
calculated, there is nothing to be gained by raising it.
|
|
As a preparation for fixing a bug, introduce a complete register
map in the '%live' annotations.
|
|
|
|
|
|
|
|
The compiler shouldn't crash when fed an already-optimised BEAM assembly file.
|
|
files as delimiters.
While working on a tool that processes Erlang code and testing it against this repo,
I found out about those little sneaky 0xff. I thought it may be of help to other
people build such tools to remove non-conforming-to-standard characters.
|
|
|
|
In code such as:
X / 2
the following code would be output from beam_type for the division:
{fconv,{x,0},{fr,0}}.
{fconv,{integer,2},{fr,1}}.
fclearerror.
{bif,fdiv,{f,0},[{fr,0},{fr,1}],{fr,0}}.
That is, the integer 2 would be converted to the float 2.0 at
run-time by "{fconv,{integer,2},{fr,1}}". Make sure that we do
the conversion at compile time.
Noticed-by: Richard O'Keefe
|