From 54956f0f29888177705b584f496bcbface15d141 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 4 Oct 2022 23:21:11 +0200 Subject: [PATCH 1/3] Be consistent on allowing calling non-void function as task When calling non-void functions or non-void methods of built-in types as a task a warning is issued. But when calling a non-void method of a user defined class as a task an error is generated. Be consistent here and generate a warning in both cases. Signed-off-by: Lars-Peter Clausen --- Statement.h | 1 + elaborate.cc | 32 ++++++++++++++++++-------------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/Statement.h b/Statement.h index 72ab46a3d..50ee2f2fd 100644 --- a/Statement.h +++ b/Statement.h @@ -235,6 +235,7 @@ class PCallTask : public Statement { NetProc*elaborate_function_(Design*des, NetScope*scope) const; NetProc*elaborate_void_function_(Design*des, NetScope*scope, NetFuncDef*def) const; + NetProc *elaborate_non_void_function_(Design *des, NetScope *scope) const; NetProc*elaborate_build_call_(Design*des, NetScope*scope, NetScope*task, NetExpr*use_this) const; diff --git a/elaborate.cc b/elaborate.cc index dfd1fb7d2..9d5ef8877 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -3896,16 +3896,8 @@ bool PCallTask::test_task_calls_ok_(Design*des, NetScope*scope) const return true; } -NetProc* PCallTask::elaborate_function_(Design*des, NetScope*scope) const +NetProc *PCallTask::elaborate_non_void_function_(Design *des, NetScope *scope) const { - NetFuncDef*func = des->find_function(scope, path_); - // This is not a function, so this task call cannot be a function - // call with a missing return assignment. - if (! func) return 0; - - if (gn_system_verilog() && func->is_void()) - return elaborate_void_function_(des, scope, func); - // Generate a function call version of this task call. PExpr*rval = new PECallFunction(package_, path_, parms_); rval->set_file(get_file()); @@ -3920,6 +3912,21 @@ NetProc* PCallTask::elaborate_function_(Design*des, NetScope*scope) const return tmp->elaborate(des, scope); } +NetProc* PCallTask::elaborate_function_(Design*des, NetScope*scope) const +{ + NetFuncDef*func = des->find_function(scope, path_); + + // This is not a function, so this task call cannot be a function + // call with a missing return assignment. + if (!func) + return nullptr; + + if (gn_system_verilog() && func->is_void()) + return elaborate_void_function_(des, scope, func); + + return elaborate_non_void_function_(des, scope); +} + NetProc* PCallTask::elaborate_void_function_(Design*des, NetScope*scope, NetFuncDef*def) const { @@ -3957,11 +3964,8 @@ NetProc* PCallTask::elaborate_build_call_(Design*des, NetScope*scope, } else if (task->type() == NetScope::FUNC) { NetFuncDef*tmp = task->func_def(); - if (!tmp->is_void()) { - cerr << get_fileline() << ": error: " - << "Calling a non-void function as a task." << endl; - des->errors += 1; - } + if (!tmp->is_void()) + return elaborate_non_void_function_(des, scope); def = tmp; } From 0e62ff153dfe91ff475890516918b170c68fe152 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Wed, 12 Oct 2022 11:05:57 +0200 Subject: [PATCH 2/3] Add support for void cast function call SystemVerilog has explicit support for calling a function as a statement. This is allowed when the function call is encapsulated in `void'(...)`. E.g. `void'(f(1, 2, 3));` We already support calling function calls as statements without the void cast and emit a warning when doing so. Adding support for void casts only requires to update the parser to handle the void cast and then do not emit the warning if a function is called as a statement as part of a void cast. Void casting a task or void function call is not allowed and will generate an elaboration error. Signed-off-by: Lars-Peter Clausen --- Statement.h | 3 +++ elaborate.cc | 35 +++++++++++++++++++++------ parse.y | 67 ++++++++++++++++++++++++++++++---------------------- 3 files changed, 70 insertions(+), 35 deletions(-) diff --git a/Statement.h b/Statement.h index 50ee2f2fd..613f2d64b 100644 --- a/Statement.h +++ b/Statement.h @@ -226,6 +226,8 @@ class PCallTask : public Statement { bool elaborate_elab(Design*des, NetScope*scope) const; + void void_cast() { void_cast_ = true; } + private: NetProc* elaborate_sys(Design*des, NetScope*scope) const; NetProc* elaborate_usr(Design*des, NetScope*scope) const; @@ -257,6 +259,7 @@ class PCallTask : public Statement { PPackage*package_; pform_name_t path_; std::vector parms_; + bool void_cast_ = false; }; class PCase : public Statement { diff --git a/elaborate.cc b/elaborate.cc index 9d5ef8877..02d782e97 100644 --- a/elaborate.cc +++ b/elaborate.cc @@ -3390,10 +3390,14 @@ NetProc* PCondit::elaborate(Design*des, NetScope*scope) const NetProc* PCallTask::elaborate(Design*des, NetScope*scope) const { - if (peek_tail_name(path_)[0] == '$') - return elaborate_sys(des, scope); - else + if (peek_tail_name(path_)[0] == '$') { + if (void_cast_) + return elaborate_non_void_function_(des, scope); + else + return elaborate_sys(des, scope); + } else { return elaborate_usr(des, scope); + } } /* @@ -3690,8 +3694,10 @@ NetProc* PCallTask::elaborate_method_func_(NetScope*scope, perm_string method_name, const char*sys_task_name) const { - cerr << get_fileline() << ": warning: method function '" - << method_name << "' is being called as a task." << endl; + if (!void_cast_) { + cerr << get_fileline() << ": warning: method function '" + << method_name << "' is being called as a task." << endl; + } // Generate the function. NetESFunc*sys_expr = new NetESFunc(sys_task_name, type, 1); @@ -3906,8 +3912,11 @@ NetProc *PCallTask::elaborate_non_void_function_(Design *des, NetScope *scope) c PAssign*tmp = new PAssign(0, rval); tmp->set_file(get_file()); tmp->set_lineno(get_lineno()); - cerr << get_fileline() << ": warning: User function '" - << peek_tail_name(path_) << "' is being called as a task." << endl; + if (!void_cast_) { + cerr << get_fileline() << ": warning: User function '" + << peek_tail_name(path_) << "' is being called as a task." << endl; + } + // Elaborate the assignment to a dummy variable. return tmp->elaborate(des, scope); } @@ -3962,11 +3971,23 @@ NetProc* PCallTask::elaborate_build_call_(Design*des, NetScope*scope, // that we can catch more errors. test_task_calls_ok_(des, scope); + if (void_cast_) { + cerr << get_fileline() << ": error: void casting user task '" + << peek_tail_name(path_) << "' is not allowed." << endl; + des->errors++; + } + } else if (task->type() == NetScope::FUNC) { NetFuncDef*tmp = task->func_def(); if (!tmp->is_void()) return elaborate_non_void_function_(des, scope); def = tmp; + + if (void_cast_) { + cerr << get_fileline() << ": error: void casting user void function '" + << peek_tail_name(path_) << "' is not allowed." << endl; + des->errors++; + } } /* The caller has checked the parms_ size to make sure it diff --git a/parse.y b/parse.y index 409b359a8..19b1577d8 100644 --- a/parse.y +++ b/parse.y @@ -420,6 +420,8 @@ static void current_function_set_statement(const YYLTYPE&loc, std::vector*wires; + PCallTask *subroutine_call; + PEventStatement*event_statement; Statement*statement; std::vector*statement_list; @@ -685,6 +687,8 @@ static void current_function_set_statement(const YYLTYPE&loc, std::vector analog_statement +%type subroutine_call + %type join_keyword %type spec_polarity @@ -6241,6 +6245,35 @@ spec_notifier { args_after_notifier = 0; delete[]$1; } ; +subroutine_call + : hierarchy_identifier argument_list_parens_opt + { PCallTask*tmp = pform_make_call_task(@1, *$1, *$2); + delete $1; + delete $2; + $$ = tmp; + } + | class_hierarchy_identifier argument_list_parens_opt + { PCallTask*tmp = new PCallTask(*$1, *$2); + FILE_NAME(tmp, @1); + delete $1; + delete $2; + $$ = tmp; + } + | SYSTEM_IDENTIFIER argument_list_parens_opt + { PCallTask*tmp = new PCallTask(lex_strings.make($1), *$2); + FILE_NAME(tmp,@1); + delete[]$1; + delete $2; + $$ = tmp; + } + | hierarchy_identifier '(' error ')' + { yyerror(@3, "error: Syntax error in task arguments."); + listpt; + PCallTask*tmp = pform_make_call_task(@1, *$1, pt); + delete $1; + $$ = tmp; + } + ; statement_item /* This is roughly statement_item in the LRM */ @@ -6592,19 +6625,13 @@ statement_item /* This is roughly statement_item in the LRM */ FILE_NAME(tmp,@1); $$ = tmp; } - | SYSTEM_IDENTIFIER argument_list_parens_opt ';' - { PCallTask*tmp = new PCallTask(lex_strings.make($1), *$2); - FILE_NAME(tmp,@1); - delete[]$1; - delete $2; - $$ = tmp; - } + | K_void '\'' '(' subroutine_call ')' ';' + { $4->void_cast(); + $$ = $4; + } - | hierarchy_identifier argument_list_parens_opt ';' - { PCallTask*tmp = pform_make_call_task(@1, *$1, *$2); - delete $1; - delete $2; - $$ = tmp; + | subroutine_call ';' + { $$ = $1; } | hierarchy_identifier K_with '{' constraint_block_item_list_opt '}' ';' @@ -6622,14 +6649,6 @@ statement_item /* This is roughly statement_item in the LRM */ $$ = tmp; } - | class_hierarchy_identifier argument_list_parens_opt ';' - { PCallTask*tmp = new PCallTask(*$1, *$2); - FILE_NAME(tmp, @1); - delete $1; - delete $2; - $$ = tmp; - } - /* IEEE1800 A.1.8: class_constructor_declaration with a call to parent constructor. Note that the implicit_class_handle must be K_super ("this.new" makes little sense) but that would @@ -6646,14 +6665,6 @@ statement_item /* This is roughly statement_item in the LRM */ delete $1; $$ = tmp; } - | hierarchy_identifier '(' error ')' ';' - { yyerror(@3, "error: Syntax error in task arguments."); - listpt; - PCallTask*tmp = pform_make_call_task(@1, *$1, pt); - delete $1; - $$ = tmp; - } - | error ';' { yyerror(@2, "error: malformed statement"); yyerrok; From 269e1ca88d2b47139f46ffe2b9679b2af554af25 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Thu, 15 Dec 2022 21:14:33 -0800 Subject: [PATCH 3/3] Add regression tests for void casts Check that it is possible to use a function with a return type as a statement by using a void cast. Also check that trying to void cast a void function, a task or an expression results in an error. Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/sv_void_cast1.v | 37 +++++++++++++++++++++++++ ivtest/ivltests/sv_void_cast2.v | 41 ++++++++++++++++++++++++++++ ivtest/ivltests/sv_void_cast3.v | 18 ++++++++++++ ivtest/ivltests/sv_void_cast4.v | 11 ++++++++ ivtest/ivltests/sv_void_cast_fail1.v | 12 ++++++++ ivtest/ivltests/sv_void_cast_fail2.v | 12 ++++++++ ivtest/ivltests/sv_void_cast_fail3.v | 9 ++++++ ivtest/regress-sv.list | 7 +++++ ivtest/regress-vlog95.list | 4 +++ 9 files changed, 151 insertions(+) create mode 100644 ivtest/ivltests/sv_void_cast1.v create mode 100644 ivtest/ivltests/sv_void_cast2.v create mode 100644 ivtest/ivltests/sv_void_cast3.v create mode 100644 ivtest/ivltests/sv_void_cast4.v create mode 100644 ivtest/ivltests/sv_void_cast_fail1.v create mode 100644 ivtest/ivltests/sv_void_cast_fail2.v create mode 100644 ivtest/ivltests/sv_void_cast_fail3.v diff --git a/ivtest/ivltests/sv_void_cast1.v b/ivtest/ivltests/sv_void_cast1.v new file mode 100644 index 000000000..ad271440a --- /dev/null +++ b/ivtest/ivltests/sv_void_cast1.v @@ -0,0 +1,37 @@ +// Check that void casts are supported + +module test; + + int a; + real b; + string c; + + function int f1(int x); + a = x; + return x; + endfunction + + function real f2(real x); + b = x; + return x; + endfunction + + function string f3(string x); + c = x; + return x; + endfunction + + + initial begin + void'(f1(10)); + void'(f2(1.0)); + void'(f3("10")); + + if (a === 10 && b == 1.0 && c == "10") begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_void_cast2.v b/ivtest/ivltests/sv_void_cast2.v new file mode 100644 index 000000000..141996218 --- /dev/null +++ b/ivtest/ivltests/sv_void_cast2.v @@ -0,0 +1,41 @@ +// Check that void casts on class methods are supported + +module test; + + int a; + real b; + string c; + + class C; + function int f1(int x); + a = x; + return x; + endfunction + + function real f2(real x); + b = x; + return x; + endfunction + + function string f3(string x); + c = x; + return x; + endfunction + endclass + + C d; + + initial begin + d = new; + void'(d.f1(10)); + void'(d.f2(1.0)); + void'(d.f3("10")); + + if (a === 10 && b == 1.0 && c == "10") begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_void_cast3.v b/ivtest/ivltests/sv_void_cast3.v new file mode 100644 index 000000000..44237780f --- /dev/null +++ b/ivtest/ivltests/sv_void_cast3.v @@ -0,0 +1,18 @@ +// Check that void casts on methods of built-in types is supported + +module test; + + int q[$]; + + initial begin + q.push_back(1); + void'(q.pop_back()); + + if (q.size() === 0) begin + $display("PASSED"); + end else begin + $display("FAILED"); + end + end + +endmodule diff --git a/ivtest/ivltests/sv_void_cast4.v b/ivtest/ivltests/sv_void_cast4.v new file mode 100644 index 000000000..db23012ff --- /dev/null +++ b/ivtest/ivltests/sv_void_cast4.v @@ -0,0 +1,11 @@ +// Check that void casts on SystemFunctions is supported + +module test; + + initial begin + void'($clog2(10)); + + $display("PASSED"); + end + +endmodule diff --git a/ivtest/ivltests/sv_void_cast_fail1.v b/ivtest/ivltests/sv_void_cast_fail1.v new file mode 100644 index 000000000..c0acd3e09 --- /dev/null +++ b/ivtest/ivltests/sv_void_cast_fail1.v @@ -0,0 +1,12 @@ +// Check that void casting a void function results in an error + +module test; + + function void f(int x); + endfunction + + initial begin + void'(f(10)); + end + +endmodule diff --git a/ivtest/ivltests/sv_void_cast_fail2.v b/ivtest/ivltests/sv_void_cast_fail2.v new file mode 100644 index 000000000..ea3132bda --- /dev/null +++ b/ivtest/ivltests/sv_void_cast_fail2.v @@ -0,0 +1,12 @@ +// Check that void casting a task results in an error + +module test; + + task t(int x); + endtask + + initial begin + void'(t(10)); + end + +endmodule diff --git a/ivtest/ivltests/sv_void_cast_fail3.v b/ivtest/ivltests/sv_void_cast_fail3.v new file mode 100644 index 000000000..814feae69 --- /dev/null +++ b/ivtest/ivltests/sv_void_cast_fail3.v @@ -0,0 +1,9 @@ +// Check that void casting an expression results in an error + +module test; + + initial begin + void'(1+2); + end + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index d002e84a3..c7dea8134 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -803,6 +803,13 @@ sv_var_module_output1 normal,-g2005-sv ivltests sv_var_module_output2 normal,-g2005-sv ivltests sv_var_package normal,-g2005-sv ivltests sv_var_task normal,-g2005-sv ivltests +sv_void_cast1 normal,-g2005-sv ivltests +sv_void_cast2 normal,-g2005-sv ivltests +sv_void_cast3 normal,-g2005-sv ivltests +sv_void_cast4 normal,-g2005-sv ivltests +sv_void_cast_fail1 CE,-g2005-sv ivltests +sv_void_cast_fail2 CE,-g2005-sv ivltests +sv_void_cast_fail3 CE,-g2005-sv ivltests sv_wildcard_import1 normal,-g2009 ivltests sv_wildcard_import2 normal,-g2009 ivltests sv_wildcard_import3 normal,-g2009 ivltests diff --git a/ivtest/regress-vlog95.list b/ivtest/regress-vlog95.list index acd4b9f86..323a0cb1a 100644 --- a/ivtest/regress-vlog95.list +++ b/ivtest/regress-vlog95.list @@ -515,6 +515,9 @@ sv_typedef_queue_base1 CE,-g2009 ivltests # queue sv_typedef_queue_base2 CE,-g2009 ivltests # queue sv_typedef_queue_base3 CE,-g2009 ivltests # queue sv_typedef_queue_base4 CE,-g2009 ivltests # queue +sv_void_cast1 CE,-g2009,-pallowsigned=1 ivltests # string +sv_void_cast2 CE,-g2009,-pallowsigned=1 ivltests # string, class +sv_void_cast3 CE,-g2009,-pallowsigned=1 ivltests # queue wait_fork CE,-g2009 ivltests # wait fork and join_* wild_cmp_err CE,-g2009 ivltests # ==?/!=? wild_cmp_err2 CE,-g2009 ivltests # ==?/!=? @@ -977,6 +980,7 @@ sv_var_module_output1 normal,-g2005-sv,-pallowsigned=1 ivltests sv_var_module_output2 normal,-g2005-sv,-pallowsigned=1 ivltests sv_var_package normal,-g2005-sv,-pallowsigned=1 ivltests sv_var_task normal,-g2005-sv,-pallowsigned=1 ivltests +sv_void_cast4 normal,-g2009,-pallowsigned=1 ivltests test_dispwided normal,-pallowsigned=1 ivltests gold=test_dispwided.gold test_inc_dec normal,-g2009,-pallowsigned=1 ivltests test_enumsystem normal,-g2009,-pallowsigned=1,ivltests/enumsystem.vhd ivltests