From 4436dc41ab26c4c3b20a4f79acb5ef0010df08b4 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 28 Dec 2021 21:33:49 +0100 Subject: [PATCH 01/10] Report an error for size casts with a negative value Size casts are only allowed if the value is positive. For cases where it is 0 negative or undefined an error should be reported. Currently the negative case is not handled. Extend the test to also check for negative values. Signed-off-by: Lars-Peter Clausen --- elab_expr.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/elab_expr.cc b/elab_expr.cc index c084b8029..6ee81e04f 100644 --- a/elab_expr.cc +++ b/elab_expr.cc @@ -3363,9 +3363,12 @@ unsigned PECastSize::test_width(Design*des, NetScope*scope, width_mode_t&) ivl_assert(*this, size_); ivl_assert(*this, base_); + expr_width_ = 0; + NetExpr*size_ex = elab_and_eval(des, scope, size_, -1, true); NetEConst*size_ce = dynamic_cast(size_ex); - expr_width_ = size_ce ? size_ce->value().as_ulong() : 0; + if (size_ce && !size_ce->value().is_negative()) + expr_width_ = size_ce->value().as_ulong(); delete size_ex; if (expr_width_ == 0) { cerr << get_fileline() << ": error: Cast size expression " From 5685eac1beae3cecb436dcc3ea30e6e3c7b2a2e0 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Dec 2022 19:21:15 -0800 Subject: [PATCH 02/10] Add regression tests for invalid size casts Check that an error is reported for size casts with either a value of 0, a negative value or an undefined value. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/size_cast_fail1.v | 12 ++++++++++++ ivtest/ivltests/size_cast_fail2.v | 12 ++++++++++++ ivtest/ivltests/size_cast_fail3.v | 12 ++++++++++++ ivtest/regress-sv.list | 3 +++ 4 files changed, 39 insertions(+) create mode 100644 ivtest/ivltests/size_cast_fail1.v create mode 100644 ivtest/ivltests/size_cast_fail2.v create mode 100644 ivtest/ivltests/size_cast_fail3.v diff --git a/ivtest/ivltests/size_cast_fail1.v b/ivtest/ivltests/size_cast_fail1.v new file mode 100644 index 000000000..dbd520355 --- /dev/null +++ b/ivtest/ivltests/size_cast_fail1.v @@ -0,0 +1,12 @@ +// Check that an error is reported when using a zero value for a size cast. + +module test; + + localparam integer N = 0; + + initial begin + int x, y; + y = N'(x); // This should fail, N is zero + end + +endmodule diff --git a/ivtest/ivltests/size_cast_fail2.v b/ivtest/ivltests/size_cast_fail2.v new file mode 100644 index 000000000..c7a23ad87 --- /dev/null +++ b/ivtest/ivltests/size_cast_fail2.v @@ -0,0 +1,12 @@ +// Check that an error is reported when using a negative value for a size cast. + +module test; + + localparam integer N = -1; + + initial begin + int x, y; + y = N'(x); // This should fail, N is negative + end + +endmodule diff --git a/ivtest/ivltests/size_cast_fail3.v b/ivtest/ivltests/size_cast_fail3.v new file mode 100644 index 000000000..986c05281 --- /dev/null +++ b/ivtest/ivltests/size_cast_fail3.v @@ -0,0 +1,12 @@ +// Check that an error is reported when using an undefined value for a size cast. + +module test; + + localparam integer N = 32'hx; + + initial begin + int x, y; + y = N'(x); // This should fail, N is undefined + end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index be291eb87..168f4b82c 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -465,6 +465,9 @@ size_cast2 normal,-g2005-sv ivltests size_cast3 normal,-g2009 ivltests size_cast4 normal,-g2009 ivltests size_cast5 normal,-g2009 ivltests +size_cast_fail1 CE,-g2009 ivltests +size_cast_fail2 CE,-g2009 ivltests +size_cast_fail3 CE,-g2009 ivltests slongint_test normal,-g2005-sv ivltests sshortint_test normal,-g2005-sv ivltests string_events normal,-g2009 ivltests gold=string_events.gold From a659ca09cef7ad7662f781bb740c7ee3d6ad4418 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Thu, 13 Oct 2022 14:47:24 +0200 Subject: [PATCH 03/10] Let void functions in always_comb block contribute to sensitivity list Internally void function calls are modeled as task calls. But unlike task calls, access to a signal inside a void function is supposed to contribute to the sensitivity list of an always_comb block. Make sure that the same logic applied for non-void functions is also applied for void functions. Signed-off-by: Lars-Peter Clausen --- net_nex_input.cc | 64 ++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 27 deletions(-) diff --git a/net_nex_input.cc b/net_nex_input.cc index 7d68229ef..8c45056c8 100644 --- a/net_nex_input.cc +++ b/net_nex_input.cc @@ -255,6 +255,30 @@ NexusSet* NetETernary::nex_input(bool rem_out, bool always_sens, bool nested_fun return result; } +// Get the contribution of a function call in a always_comb block +static void func_always_sens(NetFuncDef *func, NexusSet *result, + bool rem_out, bool nested_func) +{ + // Avoid recursive function calls. + static set func_set; + if (!nested_func) + func_set.clear(); + + if (!func_set.insert(func).second) + return; + + std::unique_ptr tmp(func->proc()->nex_input(rem_out, true, true)); + // Remove the function inputs + std::unique_ptr in(new NexusSet); + for (unsigned idx = 0; idx < func->port_count(); idx++) { + NetNet *net = func->port(idx); + assert(net->pin_count() == 1); + in->add(net->pin(0).nexus(), 0, net->vector_width()); + } + tmp->rem(*in); + result->add(*tmp); +} + NexusSet* NetEUFunc::nex_input(bool rem_out, bool always_sens, bool nested_func) const { NexusSet*result = new NexusSet; @@ -265,31 +289,8 @@ NexusSet* NetEUFunc::nex_input(bool rem_out, bool always_sens, bool nested_func) delete tmp; } - if (always_sens) { - NetFuncDef*func = func_->func_def(); - - // Avoid recursive function calls. - static set func_set; - if (!nested_func) - func_set.clear(); - - if (!func_set.insert(func).second) - return result; - - NexusSet*tmp = func->proc()->nex_input(rem_out, always_sens, true); - // Remove the function inputs - NexusSet*in = new NexusSet; - for (unsigned idx = 0 ; idx < func->port_count() ; idx += 1) { - NetNet*net = func->port(idx); - assert(net->pin_count() == 1); - in->add(net->pin(0).nexus(), 0, net->vector_width()); - } - tmp->rem(*in); - delete in; - - result->add(*tmp); - delete tmp; - } + if (always_sens) + func_always_sens(func_->func_def(), result, rem_out, nested_func); return result; } @@ -613,9 +614,18 @@ NexusSet* NetSTask::nex_input(bool rem_out, bool always_sens, bool nested_func) * parameters to consider, because the compiler already removed them * and converted them to blocking assignments. */ -NexusSet* NetUTask::nex_input(bool, bool, bool) const +NexusSet* NetUTask::nex_input(bool rem_out, bool always_sens, bool nested_func) const { - return new NexusSet; + NexusSet *result = new NexusSet; + + /* + * Let the contents of void functions contribute to the sensitivity list + * of always_comb blocks + */ + if (always_sens && task_->type() == NetScope::FUNC) + func_always_sens(task_->func_def(), result, rem_out, nested_func); + + return result; } NexusSet* NetWhile::nex_input(bool rem_out, bool always_sens, bool nested_func) const From 9a96ba62e46bb65060a60416b1d7222890f4f003 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Dec 2022 17:34:53 -0800 Subject: [PATCH 04/10] Add regression test for using void function in always_comb block Check that variables used in void functions contribute to the sensitivity list in a always_comb block. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/always_comb_void_func.v | 51 +++++++++++++++++++++++++ ivtest/regress-sv.list | 1 + ivtest/regress-vlog95.list | 1 + 3 files changed, 53 insertions(+) create mode 100644 ivtest/ivltests/always_comb_void_func.v diff --git a/ivtest/ivltests/always_comb_void_func.v b/ivtest/ivltests/always_comb_void_func.v new file mode 100644 index 000000000..0272393b9 --- /dev/null +++ b/ivtest/ivltests/always_comb_void_func.v @@ -0,0 +1,51 @@ +// Check that variables referenced in a void function contribute to the +// sensitivity list of a always_comb block. + +module top; + logic passed; + logic [7:0] value; + integer counter; + + function automatic void count(bit what); + counter = 0; + for (integer i = 0; i < $bits(value); i++) begin + if (value[i] == what) + counter += 1; + end + endfunction + + always_comb begin + count(1'b1); + end + + initial begin + passed = 1'b1; + + value = 8'b0000_0000; + #1; + if (counter !== 0) begin + $display("Expected 0, got %d", counter); + passed = 1'b0; + end + + value = 8'b0011_1100; + #1; + if (counter !== 4) begin + $display("Expected 4, got %d", counter); + passed = 1'b0; + end + + value = 8'b1011_1101; + #1; + if (counter !== 6) begin + $display("Expected 6, got %d", counter); + passed = 1'b0; + end + + if (passed) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index be291eb87..2965ae64d 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -76,6 +76,7 @@ always_comb_fail4 CE,-g2005-sv ivltests always_comb_no_sens nornal,-g2005-sv ivltests gold=always_comb_no_sens.gold always_comb_rfunc nornal,-g2005-sv ivltests always_comb_trig normal,-g2005-sv ivltests +always_comb_void_func normal,-g2005-sv ivltests always_comb_warn normal,-g2005-sv ivltests gold=always_comb_warn.gold always_ff normal,-g2005-sv ivltests always_ff_fail CE,-g2005-sv ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index abb56ce33..87407d155 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -69,6 +69,7 @@ # Verilog 95 does not support automatic tasks or functions. always_comb_rfunc CE ivltests +always_comb_void_func CE ivltests automatic_error11 CE ivltests automatic_error12 CE ivltests automatic_error13 CE ivltests From d12a74beec5d0f4504fb39e64de88cdd711afacb Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 26 Dec 2022 08:52:38 -0800 Subject: [PATCH 05/10] ivlpp: Handle '*' or '/' following C-style comment in macro A '*' or '/' directly following a C-style comment in a macro currently triggers the detection of the start of another comment. Fix this by first looking for a '/' that should start the comment. Signed-off-by: Lars-Peter Clausen --- ivlpp/lexor.lex | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/ivlpp/lexor.lex b/ivlpp/lexor.lex index bc34a1fac..fda434a19 100644 --- a/ivlpp/lexor.lex +++ b/ivlpp/lexor.lex @@ -1215,9 +1215,7 @@ static void do_define(void) } *cp = 0; break; - } - - if (cp[1] == '*') { + } else if (cp[1] == '*') { tail = strstr(cp+2, "*/"); if (tail == 0) { @@ -1229,10 +1227,11 @@ static void do_define(void) } memmove(cp, tail+2, strlen(tail+2)+1); - continue; + } else { + cp++; } - cp = strchr(cp+1, '/'); + cp = strchr(cp, '/'); } /* Trim trailing white space. */ From c4daf11facf965f47ad6cb01f1ddd1cb3c489b75 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 17 Dec 2022 10:30:18 -0800 Subject: [PATCH 06/10] ivlpp: Handle multi-line comments in macros Make sure that comments spanning multiple lines are supported in multi-line macros. Since the lexer parses line by line we need a flag to track whether a multi-line comment is currently active. Signed-off-by: Lars-Peter Clausen --- ivlpp/lexor.lex | 50 +++++++++++++++++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/ivlpp/lexor.lex b/ivlpp/lexor.lex index fda434a19..c4987af7a 100644 --- a/ivlpp/lexor.lex +++ b/ivlpp/lexor.lex @@ -1130,12 +1130,14 @@ void free_macros(void) * variables. When the define is over, the def_finish() function * executes the define and clears this text. The define_continue_flag * is set if do_define detects that the definition is to be continued - * on the next line. + * on the next line. The define_comment_flag is set when a multi-line comment is + * active in a define. */ static char* define_text = 0; static size_t define_cnt = 0; static int define_continue_flag = 0; +static int define_comment_flag = 0; /* * The do_magic function puts the expansions of magic macros into @@ -1189,6 +1191,33 @@ static char *find_arg(char*ptr, char*head, char*arg) return cp; } +/* + * Returns 1 if the comment continues on the next line + */ +static int do_define_multiline_comment(char *replace_start, + const char *search_start) +{ + char *tail = strstr(search_start, "*/"); + + if (!tail) { + if (search_start[strlen(search_start) - 1] == '\\') { + define_continue_flag = 1; + define_comment_flag = 1; + *replace_start++ = '\\'; + } else { + define_comment_flag = 0; + fprintf(stderr, "%s:%u: Unterminated comment in define\n", + istack->path, istack->lineno+1); + } + *replace_start = '\0'; + return 1; + } + define_comment_flag = 0; + tail += 2; + memmove(replace_start, tail, strlen(tail) + 1); + return 0; +} + /* * Collect the definition. Normally, this returns 0. If there is a * continuation, then return 1 and this function may be called again @@ -1204,6 +1233,12 @@ static void do_define(void) define_continue_flag = 0; + /* Are we in an multi-line comment? Look for the end */ + if (define_comment_flag) { + if (do_define_multiline_comment(yytext, yytext)) + return; + } + /* Look for comments in the definition, and remove them. */ cp = strchr(yytext, '/'); @@ -1216,20 +1251,11 @@ static void do_define(void) *cp = 0; break; } else if (cp[1] == '*') { - tail = strstr(cp+2, "*/"); - - if (tail == 0) { - *cp = 0; - fprintf(stderr, "%s:%u: Unterminated comment in define\n", - istack->path, istack->lineno+1 - ); + if (do_define_multiline_comment(cp, cp + 2)) break; - } - - memmove(cp, tail+2, strlen(tail+2)+1); } else { cp++; - } + } cp = strchr(cp, '/'); } From 109b794253eecf9d270d875662835ac5cb72d9a3 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 26 Dec 2022 11:37:44 -0800 Subject: [PATCH 07/10] Add regression tests for comments in macros Add regression tests for some corner cases for handling comments in macros. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/macro_comment1.v | 16 ++++++++++++++++ ivtest/ivltests/macro_comment2.v | 17 +++++++++++++++++ ivtest/ivltests/macro_comment3.v | 16 ++++++++++++++++ ivtest/regress-vlg.list | 3 +++ 4 files changed, 52 insertions(+) create mode 100644 ivtest/ivltests/macro_comment1.v create mode 100644 ivtest/ivltests/macro_comment2.v create mode 100644 ivtest/ivltests/macro_comment3.v diff --git a/ivtest/ivltests/macro_comment1.v b/ivtest/ivltests/macro_comment1.v new file mode 100644 index 000000000..c1004fb61 --- /dev/null +++ b/ivtest/ivltests/macro_comment1.v @@ -0,0 +1,16 @@ +// Check that a '*' or '/' following a C-style comment is supported in a macro + +`define x(a, b) a /* comment */ * b +`define y(a, b) a /* comment */ / b + + module test; + + initial begin + if (`x(2, 3) === 6 && `y(8, 2) === 4) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + + endmodule diff --git a/ivtest/ivltests/macro_comment2.v b/ivtest/ivltests/macro_comment2.v new file mode 100644 index 000000000..3908da21b --- /dev/null +++ b/ivtest/ivltests/macro_comment2.v @@ -0,0 +1,17 @@ +// Check that another comment directly following a C-style comment is +// supported in a macro. + +`define x(a, b) a /* comment */// * b +`define y(a, b) a /* comment *//*/ b*/ + + module test; + + initial begin + if (`x(2, 3) === 2 && `y(8, 2) === 8) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + + endmodule diff --git a/ivtest/ivltests/macro_comment3.v b/ivtest/ivltests/macro_comment3.v new file mode 100644 index 000000000..422d0d97f --- /dev/null +++ b/ivtest/ivltests/macro_comment3.v @@ -0,0 +1,16 @@ +// Check that a '/' directly after opening a C-style comment gets handled +// correctly in a macro. + +`define x(a, b) a /*/ b */ + b + + module test; + + initial begin + if (`x(2, 3) === 5) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + + endmodule diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index ef165b190..d3fb4870d 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -635,6 +635,9 @@ localparam_type normal ivltests gold=parameter_type.gold long_div normal ivltests gold=long_div.gold macro2 normal ivltests macro_args CO,-yivltests ivltests +macro_comment1 normal ivltests +macro_comment2 normal ivltests +macro_comment3 normal ivltests macro_redefinition normal,-Wmacro-redefinition ivltests gold=macro_redefinition.gold macro_replacement normal,-Wmacro-replacement ivltests gold=macro_replacement.gold macsub normal ivltests From 8187bf58f7fa74ea407f9ce47aec8c48aa534b91 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sun, 25 Dec 2022 19:26:52 -0800 Subject: [PATCH 08/10] Add regression test for macros with multi-line comments Check that multi-line comments are supported in macros. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/macro_comment_multiline.v | 23 +++++++++++++++++++++++ ivtest/regress-vlg.list | 1 + 2 files changed, 24 insertions(+) create mode 100644 ivtest/ivltests/macro_comment_multiline.v diff --git a/ivtest/ivltests/macro_comment_multiline.v b/ivtest/ivltests/macro_comment_multiline.v new file mode 100644 index 000000000..3a7c4b18a --- /dev/null +++ b/ivtest/ivltests/macro_comment_multiline.v @@ -0,0 +1,23 @@ +// Check that multi-line comments in macros are supported. Including some corner +// cases like another comment start in the comment. + +`define test(a, b, c) \ + (a + /* these is some \ + multi line */ b /* comment \ + that goes on \ + and on */ ) /* and some more \ +\ + // and even has a comments \ + /* in the comment */ * c + +module test; + + initial begin + if (`test(1, 2, 3) === 9) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index d3fb4870d..977970c83 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -638,6 +638,7 @@ macro_args CO,-yivltests ivltests macro_comment1 normal ivltests macro_comment2 normal ivltests macro_comment3 normal ivltests +macro_comment_multiline normal ivltests macro_redefinition normal,-Wmacro-redefinition ivltests gold=macro_redefinition.gold macro_replacement normal,-Wmacro-replacement ivltests gold=macro_replacement.gold macsub normal ivltests From 6713b343cc0fb3cda442e98031b355d87836ba48 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 27 Dec 2022 08:38:38 -0800 Subject: [PATCH 09/10] Report error for packed struct default member values SystemVerilog allows struct members to have default values specified as part of the struct declaration. But this is only allowed for unpacked structs. For packed structs an error should be reported. This is defined in section 7.2.2 ("Assigning to structures") of the LRM (1800-2017). Currently default member values are just silently discarded if specified for a packed struct. Make sure to report an error instead. Signed-off-by: Lars-Peter Clausen --- elab_type.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/elab_type.cc b/elab_type.cc index 2e5fbdf74..b611f655b 100644 --- a/elab_type.cc +++ b/elab_type.cc @@ -242,6 +242,13 @@ ivl_type_t struct_type_t::elaborate_type_raw(Design*des, NetScope*scope) const ; cur_name != curp->names->end() ; ++ cur_name) { decl_assignment_t*namep = *cur_name; + if (packed_flag && namep->expr) { + cerr << namep->expr->get_fileline() << " error: " + << "Packed structs must not have default member values." + << endl; + des->errors++; + } + netstruct_t::member_t memb; memb.name = namep->name; memb.net_type = elaborate_array_type(des, scope, *this, From e83c0211b2d66962256f2170a1a47f1456c98437 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 27 Dec 2022 08:45:48 -0800 Subject: [PATCH 10/10] Add regression test for default member values in packed structs Default member values are not allowed in packed structs. Check that an error is reported for them. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/struct_packed_member_def.v | 13 +++++++++++++ ivtest/regress-sv.list | 1 + 2 files changed, 14 insertions(+) create mode 100644 ivtest/ivltests/struct_packed_member_def.v diff --git a/ivtest/ivltests/struct_packed_member_def.v b/ivtest/ivltests/struct_packed_member_def.v new file mode 100644 index 000000000..1e9b58129 --- /dev/null +++ b/ivtest/ivltests/struct_packed_member_def.v @@ -0,0 +1,13 @@ +// Check that an error is reported when specifing a default member value for a +// packed struct. + +module test; + struct packed { + // This should fail, default member value is not allowed for packed struct + integer x = 10; + } s; + + initial begin + $display("FAILED"); + end +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index be291eb87..c2c1a8bad 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -485,6 +485,7 @@ struct_member_signed normal,-g2009 ivltests struct_packed_array normal,-g2009 ivltests struct_packed_array2 normal,-g2009 ivltests struct_packed_darray_fail CE,-g2009 ivltests +struct_packed_member_def CE,-g2009 ivltests struct_packed_queue_fail CE,-g2009 ivltests struct_packed_sysfunct normal,-g2009 ivltests struct_packed_sysfunct2 normal,-g2009 ivltests