Check that the signedness of class properties is handled correctly
* When sign extending
* When passing as a value to a system function
Check this for both when accessing the property from within a class method
as well as accessing it on a class object.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Check that the signedness of the return value of methods is handled
correctly.
* When sign extending
* When passing as a value to a system function
Check this for both methods on user defined class as well as built-in
methods on SystemVerilog types.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
The signedness of an expression can change depending on its context. E.g.
for an arithmetic operation with one unsigned operand all operands are
treated as unsigned.
This is currently not considered when accessing class properties. This can
lead to incorrect behavior with regards to sign extension.
E.g. the following will print 4294967295 rather than 65535.
```
class C;
shortint x = -1;
endclass
...
C c = new;
$display(c.x + 32'h0);
```
Furthermore the return value is not expanded to the width of its context.
This can cause vvp to crash with an exception when it expects a vector on
the stack to have a certain width. E.g.
```
class C;
shortint x = -1;
endclass
...
C c = new;
int x;
bit a = 1'b1;
x = a ? c.x : 64'h0;
```
Solve both of this by using `pad_to_width()` on the property expression if
it is a vectorable type. Since any identifier, not just class properties,
need to have this done insert this on the common path and remove the
`pad_to_width()` call on individual paths.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
The signedness of an expression can change depending on its context. E.g.
for an arithmetic operation with one unsigned operand all operands are
treated as unsigned.
For methods, both on built-in SystemVerilog types as well as user defined
classes, this is currently not considered. This can lead to incorrect behavior
if the value is sign extended.
E.g. the following will print 4294967295 rather than 65535.
```
shortint q[$];
q.push_back(-1);
$display(q.pop_front() + 32'h0);
```
Furthermore the return value is not expanded to the width of its context.
This can cause vvp to crash with an exception when it expects a vector on
the stack to have a certain width.
E.g.
```
int d[];
longint x;
bit a = 1'b1;
x = a ? d.size() : 64'h0;
```
Solve both of this by using `pad_to_width()` on the method return value if
it is a vectorable type. Since any function call, not just methods, needs
to have this done to its return value insert this on the common path.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
When calling a queue method without parenthesis it gets elaborated through
the PEIdent path. On this path the type, width and signdess are not
reported for the method call. This leads to incorrect behavior in contexts
where those are important.
Correctly report the type, the same as when the method is called with
parenthesis.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
The result for the built-in methods for the SystemVerilog types is
currently always unsigned. This can lead to incorrect behavior if the value
is sign extended or passed as an argument to a system function (e.g.
$display).
For most built-in methods this does not matter, since even though they have
a signed return type, they will not return a negative value. E.g. the
string `len()` or queue `size()` functions.
It does make a difference though for the queue `pop_front()` and
`pop_back()` methods. Their return type is the element type of the queue.
If the element type is signed and the value in queue is negative is will be
handled incorrectly.
E.g. the following will print `4294967295` rather than `-1`.
```
int q[$];
q.push_back(-1);
$display(q.pop_front());
```
To correctly support this consistently assign the actual data type of the
built-in method's return value to the `NetESFunc`, rather than just the width
and base type. The width, base type and also the signedness can be derived
from the data type.
Note that this only fixes the default signedness, but not the case where
the signedness of the expression is changed by its context (e.g. in
arithmetic expression). Handling this will require some additional work.
Also note that assigning the actual data type is also required to support type
checking on the return value, e.g. as needed for enum types.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
There are a couple of different NetNet constructors for different data
types. They are all very similar, consolidate them into a single
constructor taking a ivl_type_t.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Check that the behavior of the Verilog AMS `abs()` function is correct when
its argument is a function call. Check this for both vector as well as real
types.
This test is largely a copy of the existing vams_abs2 test, just replacing
the identifier argument with a function call argument.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Currently a `vector_type_t` with the base type set to `IVL_VT_REAL` is used as
the data type for real type signals. But there is also the `real_type_t` data
type, which is used as the data type for function return types and class
properties.
Move signals also over to using `real_type_t`. This ensures consistent
behavior between all sorts of constructs with a data type, makes sure that
`vector_type_t` is only used for vector types.
It also allows to eventually differentiate between `real` and `shortreal`
at the elaboration stage. Currently this information is discarded by the
parser.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
reals are both signed and scalar. But the real_type_t currently reports as
neither.
This isn't much of a problem because most real signals are implemented
using netvector_t with the base type set to IVL_VT_REAL, for which the
signedness is correctly reported. Function return values and class
properties use the netreal_t as their data type, but most places that work
with reals check the base type and assume that the value is signed when the
base type is real.
The only place where this really makes a difference at the moment is the
Verilog-AMS function when being passed a function call as its argument. In
that case the `abs()` function will be optimized away and a negative value
will be passed through as negative.
But going forward netreal_t is also going to be used for the data type of
real type signals.
To fix the `abs()` issue and to be ready to switch real signals over to
using netreal_t as their type implement the appropriate methods on
netreal_t.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Currently only the netvector_t type implements the get_scalar() method. To
check whether a type is scalar it is first cast to netvector_t and then the
method is called.
But there are other types, such as areal that can also be scalar. To
support indicating that a real type is scalar add a virtual get_scalar()
method to ivl_type_s, which is the base class for all types.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
The compiler normally optimises away the enclosing block statement
if a block only contains one statement. But this is not valid for
a fork/join_none block.
Check that static class properties can be accessed for read and write and
that they are shared between all instances of a class type.
Check that this works for the following 3 cases
* accessing the static property in a class function or task
* accessing the static property in a class function or task using `this`
* accessing the static property on a class object instance
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Assigning a value to a static class property in a class task or function
will currently not write to the static signal, but instead to an otherwise
invisible per instance property. E.g. the example below will print 0 when
the task `t` is called.
```
class C;
static int i;
task t;
i = 10;
$display(i);
end
endclass
```
Since static class properties are implemented as normal signals just
fallback to the default signal handling when an assignment to a static
class property is detected.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Check that constant recursive functions are supported. Check both Verilog
style using assignments to the implicit function return signal and
SystemVerilog style using `return`.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Add a regression test that checks that recursive functions using a `return`
statement work correctly.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
A `return` statement in a function gets translated into a vvp `%disable`
instruction. This works fine as long as no recursion is involved. The
`%disable` instruction will stop execution of all active threads of a
particular scope. For recursive functions this means as soon as the inner
most function returns all containing outer function calls get disabled as
well. This results in incorrect behavior.
To make recursive functions using the `return` statement work use the new
vvp `%disable/parent` instruction. This instruction will only disable the
closest thread in the thread hierarchy that matches the target scope.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
The `%disable` instruction will stop the execution of all active
threads of a specific scope. This is what is required to implement
the semantics of the Verilog `disable` statement.
But it is not suited to implement the SystemVerilog flow control
statements such as `return`, `continue` and `break`. These only
affect the thread hierarchy from which it is called, but not other
concurrently running threads from the same scope.
Add a new `%disable/flow` instruction that will only disable the thread
closest to the current thread in the thread hierarchy. This can either be
the thread itself or one of its parents. This will leave other concurrent
threads of the same scope untouched and also allows function recursion
since only the closest parent thread is disabled.
Note that it is not possible to implement this using `%jmp` instructions
since a block in a function with variable declarations will be its own
sub-thread, but using flow control instructions it is possible to exit from
that thread to the parent scope, which is not possible with `%jmp`
instructions.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Embed the Icarus Verilog documentation, and format it so that the
Sphinx tools can process it into html or other formats. This will
make the documentation easier to keep up to date with the actual
software.
Check that it is not possible to declare a variable in a package without an
explicit data type for the variable.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Variable declarations in packages need an explicit data type. Omitting the
data type or using just packed dimensions is not valid syntax. E.g. the
following should not work.
```
package P;
x;
[1:0] y;
endpackage
```
The current implementation does accept this tough. To fix this update the
parser to only allow explicit data types for package variable declarations.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
This simplifies the code by making it independent of the size of 'long', and
fixes the behaviour of urandom_range when the upper limit is > 0x7fffffff.
Check that it is possible to declare the type separately from the direction
for non-ANSI integer, time and atom2 ports. Check that it is possible to
both declare the type before and after the direction.
For integer, time and atom2 types the range specification on the port
direction declaration should be empty, rather than the implicit packed
dimension of the integer type.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
When using non-ANSI style port declarations it is possible to declare the
port direction and the data type for the port in separate statements. E.g.
```
input x;
reg x;
```
When using packed array dimensions they must match for both declarations.
E.g.
```
input [3:0] x;
reg [3:0] x;
```
But this only applies for vector types, i.e. the packed dimension is
explicitly declared. It does not apply to the `integer` and `time` types,
which have an implicit packed dimension.
The current implementation requires that even for `integer` and `time`
types the implicit dimension needs to be explicitly declared in the port
direction. E.g. the following will result in a elaboration error
complaining about a packed dimension mismatch.
```
module test;
output x;
integer x;
endmodule
```
Currently the parser creates a vector_type_t for `time` and `integer`. This
means that e.g. `time` and `reg [63:0]` are indistinguishable during
elaboration, even though they require different behavior.
To fix let the atom2_type_t handle `integer` and `time`. Since it no longer
exclusively handles 2-state types, rename it to atom_type_t.
This also fixes a problem with the vlog95 target unit tests. The vlog95
target translates
```
module test(output integer x);
endmodule
```
to
```
module test(x);
output x;
integer x;
endmodule
```
which then fails when being elaborated again. There were some regression
tests that were failing because of this that will now pass.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
When using non-ANSI style port declarations it is possible to declare the
port direction and the data type for the port in separate statements. E.g.
```
input x;
reg x;
```
When using packed array dimensions they must match for both declarations.
E.g.
```
input [3:0] x;
reg [3:0] x;
```
But this only applies to vector types, i.e. the packed dimension is
explicitly declared. It does not apply to the atom2 types which have an
implicit packed dimension.
The current implementation requires that even for atom2 types the implicit
dimension needs to be explicitly declared in the port direction. E.g. the
following will result in a elaboration error complaining about a packed
dimension mismatch.
```
module test;
output x;
byte x;
endmodule
```
Currently atom2_type_t's are deconstructed into base type, range and
signdness in the parser. That data is then passed to the signal
elaboration, which will then construct a netvector_t from it. This makes it
impossible to e.g. differentiate between `bit signed [31:0]` and `int`
during elaboration.
Instead of breaking the data type apart pass it as the data_type_t of the
signal and use the elaborate_type() method in the signal elaboration to
generate the netvector_t.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Check that it is possible to both declare and call class constructors
without using parenthesis after the `new` keyword.
Check that a non-ANSI port for a class constructor results in an error.
Check that it is possible to invoke a class task through a implicit class
handle (`this` or `super`) without using parenthesis after the task name.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
It is possible to call a class method without parenthesis if no arguments
are specified.
At the moment this works when calling a class method by name. But when
using the implicit class handle `this` or `super` it does not work.
Add support for this.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Class constructors can be declared without parenthesis after the `new` when
no arguments are required. Just like for normal function.
In a similar way the base class constructor can also be invoked without
parenthesis after the `new`.
```
class C extends D;
function new;
super.new;
endfunction
endclass
```
Add support for this by making the parenthesis optional in the parser.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Class constructors don't allow for non-ANSI ports. E.g. the following is
not valid.
```
class C;
function new();
input int i;
endfunction
endclass
```
The parser will currently accept this, but otherwise ignore the non-ANSI
port. Modify the parser rules so that this is a syntax error.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
There are a few places in the grammar that follow the pattern of
`implicit_class_handle '.' hierarchy_identifier` and then splice the two
identifier paths into a single one. Factor this into a common helper rule
to avoid duplicated code.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
There are a few places in the grammar where it is possible to specify a
argument list in parenthesis or nothing. E.g. a task invocation.
```
task t(int a = 10);
endtask
initial begin
// All 3 are valid syntax
t(1);
t();
t;
end
```
Factor this out into a common rule to be able to remove some duplicated
code.
Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
Lars-Peter Clausen
Stephen Williams
Emilio Miralles
Martin Whitaker