Set default signedness of return types for built-in methods of SV types

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>
This commit is contained in:
Lars-Peter Clausen 2022-04-03 12:43:31 +02:00
parent 8de2e60d26
commit 156f08a1c5
5 changed files with 33 additions and 55 deletions

View File

@ -249,9 +249,7 @@ class PCallTask : public Statement {
const char*sys_task_name) const;
NetProc*elaborate_method_func_(NetScope*scope,
NetNet*net,
ivl_variable_type_t type,
unsigned width,
bool signed_flag,
ivl_type_t type,
perm_string method_name,
const char*sys_task_name) const;
bool test_task_calls_ok_(Design*des, NetScope*scope) const;

View File

@ -3058,7 +3058,7 @@ NetExpr* PECallFunction::elaborate_expr_method_(Design*des, NetScope*scope,
<< "takes no arguments" << endl;
des->errors += 1;
}
NetESFunc*sys_expr = new NetESFunc("$size", IVL_VT_BOOL, 32, 1);
NetESFunc*sys_expr = new NetESFunc("$size", &netvector_t::atom2u32, 1);
sys_expr->set_line(*this);
sys_expr->parm(0, sub_expr);
return sys_expr;
@ -3076,19 +3076,20 @@ NetExpr* PECallFunction::elaborate_expr_method_(Design*des, NetScope*scope,
// Get the method name that we are looking for.
perm_string method_name = search_results.path_tail.back().name;
if (method_name == "size") {
if (parms_.size() != 0) {
cerr << get_fileline() << ": error: size() method "
<< "takes no arguments" << endl;
des->errors += 1;
}
NetESFunc*sys_expr = new NetESFunc("$size", IVL_VT_BOOL, 32, 1);
NetESFunc*sys_expr = new NetESFunc("$size", &netvector_t::atom2u32, 1);
sys_expr->set_line(*this);
sys_expr->parm(0, sub_expr);
return sys_expr;
}
const netqueue_t*queue = search_results.net->queue_type();
ivl_type_t element_type = queue->element_type();
if (method_name == "pop_back") {
if (parms_.size() != 0) {
cerr << get_fileline() << ": error: pop_back() method "
@ -3096,7 +3097,7 @@ NetExpr* PECallFunction::elaborate_expr_method_(Design*des, NetScope*scope,
des->errors += 1;
}
NetESFunc*sys_expr = new NetESFunc("$ivl_queue_method$pop_back",
expr_type_, expr_width_, 1);
element_type, 1);
sys_expr->set_line(*this);
sys_expr->parm(0, sub_expr);
return sys_expr;
@ -3109,7 +3110,7 @@ NetExpr* PECallFunction::elaborate_expr_method_(Design*des, NetScope*scope,
des->errors += 1;
}
NetESFunc*sys_expr = new NetESFunc("$ivl_queue_method$pop_front",
expr_type_, expr_width_, 1);
element_type, 1);
sys_expr->set_line(*this);
sys_expr->parm(0, sub_expr);
return sys_expr;
@ -3186,35 +3187,35 @@ NetExpr* PECallFunction::elaborate_expr_method_(Design*des, NetScope*scope,
if (method_name == "len") {
NetESFunc*sys_expr = new NetESFunc("$ivl_string_method$len",
IVL_VT_BOOL, 32, 1);
&netvector_t::atom2u32, 1);
sys_expr->parm(0, sub_expr);
return sys_expr;
}
if (method_name == "atoi") {
NetESFunc*sys_expr = new NetESFunc("$ivl_string_method$atoi",
IVL_VT_BOOL, integer_width, 1);
netvector_t::integer_type(), 1);
sys_expr->parm(0, sub_expr);
return sys_expr;
}
if (method_name == "atoreal") {
NetESFunc*sys_expr = new NetESFunc("$ivl_string_method$atoreal",
IVL_VT_REAL, 1, 1);
&netreal_t::type_real, 1);
sys_expr->parm(0, sub_expr);
return sys_expr;
}
if (method_name == "atohex") {
NetESFunc*sys_expr = new NetESFunc("$ivl_string_method$atohex",
IVL_VT_BOOL, integer_width, 1);
netvector_t::integer_type(), 1);
sys_expr->parm(0, sub_expr);
return sys_expr;
}
if (method_name == "substr") {
NetESFunc*sys_expr = new NetESFunc("$ivl_string_method$substr",
IVL_VT_STRING, 1, 3);
&netstring_t::type_string, 3);
sys_expr->set_line(*this);
// First argument is the source string.
@ -4657,7 +4658,9 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope,
ivl_assert(*this, sr.path_tail.size() == 1);
const name_component_t member_comp = sr.path_tail.front();
if (member_comp.name == "size") {
NetESFunc*fun = new NetESFunc("$size", IVL_VT_BOOL, 32, 1);
NetESFunc*fun = new NetESFunc("$size",
&netvector_t::atom2s32,
1);
fun->set_line(*this);
NetESignal*arg = new NetESignal(sr.net);
@ -4773,11 +4776,10 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope,
ivl_assert(*this, sr.path_tail.size() == 1);
const name_component_t member_comp = sr.path_tail.front();
const netqueue_t*queue = sr.net->queue_type();
ivl_variable_type_t qelem_type = queue->element_base_type();
unsigned qelem_width = queue->element_width();
ivl_type_t element_type = queue->element_type();
if (member_comp.name == "pop_back") {
NetESFunc*fun = new NetESFunc("$ivl_queue_method$pop_back",
qelem_type, qelem_width, 1);
element_type, 1);
fun->set_line(*this);
NetESignal*arg = new NetESignal(sr.net);
@ -4789,7 +4791,7 @@ NetExpr* PEIdent::elaborate_expr(Design*des, NetScope*scope,
if (member_comp.name == "pop_front") {
NetESFunc*fun = new NetESFunc("$ivl_queue_method$pop_front",
qelem_type, qelem_width, 1);
element_type, 1);
fun->set_line(*this);
NetESignal*arg = new NetESignal(sr.net);

View File

@ -3676,9 +3676,7 @@ NetProc* PCallTask::elaborate_queue_method_(Design*des, NetScope*scope,
*/
NetProc* PCallTask::elaborate_method_func_(NetScope*scope,
NetNet*net,
ivl_variable_type_t type,
unsigned width,
bool signed_flag,
ivl_type_t type,
perm_string method_name,
const char*sys_task_name) const
{
@ -3686,15 +3684,14 @@ NetProc* PCallTask::elaborate_method_func_(NetScope*scope,
<< method_name << "' is being called as a task." << endl;
// Generate the function.
NetESFunc*sys_expr = new NetESFunc(sys_task_name, type, width, 1);
NetESFunc*sys_expr = new NetESFunc(sys_task_name, type, 1);
sys_expr->set_line(*this);
NetESignal*arg = new NetESignal(net);
arg->set_line(*net);
sys_expr->parm(0, arg);
// Create a L-value that matches the function return type.
NetNet*tmp;
netvector_t*tmp_vec = new netvector_t(type, width-1, 0, signed_flag);
tmp = new NetNet(scope, scope->local_symbol(), NetNet::REG, tmp_vec);
tmp = new NetNet(scope, scope->local_symbol(), NetNet::REG, type);
tmp->set_line(*this);
NetAssign_*lv = new NetAssign_(tmp);
// Generate an assign to the fake L-value.
@ -3782,9 +3779,8 @@ NetProc* PCallTask::elaborate_method_(Design*des, NetScope*scope,
else if (method_name=="size")
// This returns an int. It could be removed, but keep for now.
return elaborate_method_func_(scope, net,
IVL_VT_BOOL, 32,
true, method_name,
"$size");
&netvector_t::atom2s32,
method_name, "$size");
else if (method_name=="reverse") {
cerr << get_fileline() << ": sorry: 'reverse()' "
"array sorting method is not currently supported."
@ -3825,15 +3821,13 @@ NetProc* PCallTask::elaborate_method_(Design*des, NetScope*scope,
"$ivl_queue_method$insert");
else if (method_name == "pop_front")
return elaborate_method_func_(scope, net,
use_darray->element_base_type(),
use_darray->element_width(),
false, method_name,
use_darray->element_type(),
method_name,
"$ivl_queue_method$pop_front");
else if (method_name == "pop_back")
return elaborate_method_func_(scope, net,
use_darray->element_base_type(),
use_darray->element_width(),
false, method_name,
use_darray->element_type(),
method_name,
"$ivl_queue_method$pop_back");
}
@ -5324,12 +5318,12 @@ NetProc* PForeach::elaborate(Design*des, NetScope*scope) const
idx_exp->set_line(*this);
// Make an initialization expression for the index.
NetESFunc*init_expr = new NetESFunc("$low", IVL_VT_BOOL, 32, 1);
NetESFunc*init_expr = new NetESFunc("$low", &netvector_t::atom2s32, 1);
init_expr->set_line(*this);
init_expr->parm(0, array_exp);
// Make a condition expression: idx <= $high(array)
NetESFunc*high_exp = new NetESFunc("$high", IVL_VT_BOOL, 32, 1);
NetESFunc*high_exp = new NetESFunc("$high", &netvector_t::atom2s32, 1);
high_exp->set_line(*this);
high_exp->parm(0, array_exp);

View File

@ -487,28 +487,13 @@ NetESFunc::NetESFunc(const char*n, ivl_variable_type_t t,
}
NetESFunc::NetESFunc(const char*n, ivl_type_t rtype, unsigned np)
: NetExpr(rtype), name_(0), type_(IVL_VT_NO_TYPE), enum_type_(0), parms_(np), is_overridden_(false)
: NetExpr(rtype), name_(0), type_(rtype->base_type()),
enum_type_(dynamic_cast<const netenum_t*>(rtype)), parms_(np),
is_overridden_(false)
{
name_ = lex_strings.add(n);
expr_width(rtype->packed_width());
// FIXME: For now, assume that all uses of this constructor
// are for the IVL_VT_DARRAY type. Eventually, the type_
// member will go away.
if (dynamic_cast<const netdarray_t*>(rtype))
type_ = IVL_VT_DARRAY;
else if (dynamic_cast<const netclass_t*>(rtype))
type_ = IVL_VT_CLASS;
else if (dynamic_cast<const netstring_t*>(rtype))
type_ = IVL_VT_STRING;
else
ivl_assert(*this, 0);
}
NetESFunc::NetESFunc(const char*n, const netenum_t*enum_type, unsigned np)
: name_(0), type_(enum_type->base_type()), enum_type_(enum_type), parms_(np), is_overridden_(false)
{
name_ = lex_strings.add(n);
expr_width(enum_type->packed_width());
cast_signed_base_(rtype->get_signed());
}
NetESFunc::~NetESFunc()

View File

@ -4641,7 +4641,6 @@ class NetESFunc : public NetExpr {
NetESFunc(const char*name, ivl_variable_type_t t,
unsigned width, unsigned nprms, bool is_overridden =false);
NetESFunc(const char*name, ivl_type_t rtype, unsigned nprms);
NetESFunc(const char*name, const netenum_t*enum_type, unsigned nprms);
~NetESFunc();
const char* name() const;