From dd4ae939429b44776338e544c02b7345f1b6a9cf Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 13 Sep 2022 13:34:28 +0200 Subject: [PATCH 1/4] Fix port direction for output port declaration lists with default value For output port lists with a default value for the first port declaration all subsequent port declarations are declared as inout ports. This is due to a small typo when setting the `port_declaration_context` port direction. Fix this to make sure all ports in the port declaration list are declared as output ports. Signed-off-by: Lars-Peter Clausen --- parse.y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/parse.y b/parse.y index 1295aaa94..e155d6272 100644 --- a/parse.y +++ b/parse.y @@ -4524,7 +4524,7 @@ port_declaration } ptmp = pform_module_port_reference(@2, name); pform_module_define_port(@2, name, NetNet::POUTPUT, use_type, $4, $1); - port_declaration_context.port_type = NetNet::PINOUT; + port_declaration_context.port_type = NetNet::POUTPUT; port_declaration_context.port_net_type = use_type; port_declaration_context.data_type = $4; From a9bd522fd3248b28b170970b99e2b3161b15b51c Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Mon, 12 Sep 2022 22:17:17 +0200 Subject: [PATCH 2/4] parser: Add a helper rule for optional initializers There are a few places where it is possible to provide an optional initializer in the form of `[ = ]`. There are currently multiple rules that implement this behavior as well as few places with duplicated rules, one with and one without the initializer. Introduce a common helper rule for optional initializers. This allows to remove some duplicated code from the parser. Signed-off-by: Lars-Peter Clausen --- parse.y | 110 ++++++++++++++------------------------------------------ 1 file changed, 27 insertions(+), 83 deletions(-) diff --git a/parse.y b/parse.y index e155d6272..f1b12a4d4 100644 --- a/parse.y +++ b/parse.y @@ -602,7 +602,6 @@ static void current_function_set_statement(const YYLTYPE&loc, std::vector udp_port_list %type udp_port_decl udp_port_decls %type udp_initial udp_init_opt -%type udp_initial_expr_opt %type net_variable %type net_variable_list @@ -621,7 +620,7 @@ static void current_function_set_statement(const YYLTYPE&loc, std::vector list_of_ports module_port_list_opt list_of_port_declarations module_attribute_foreign %type parameter_value_range parameter_value_ranges %type parameter_value_ranges_opt -%type tf_port_item_expr_opt value_range_expression +%type value_range_expression %type enum_name_list enum_name %type enum_data_type enum_base_type @@ -648,8 +647,8 @@ static void current_function_set_statement(const YYLTYPE&loc, std::vector hierarchy_identifier implicit_class_handle class_hierarchy_identifier %type assignment_pattern expression expr_mintypmax %type expr_primary_or_typename expr_primary -%type class_new dynamic_array_new let_default_opt -%type var_decl_initializer_opt +%type class_new dynamic_array_new +%type var_decl_initializer_opt initializer_opt %type inc_or_dec_expression inside_expression lpvalue %type branch_probe_expression streaming_concatenation %type delay_value delay_value_simple @@ -1778,11 +1777,15 @@ list_of_variable_decl_assignments /* IEEE1800-2005 A.2.3 */ } ; -var_decl_initializer_opt +initializer_opt : '=' expression { $$ = $2; } + | { $$ = nullptr; } + ; + +var_decl_initializer_opt + : initializer_opt | '=' class_new { $$ = $2; } | '=' dynamic_array_new { $$ = $2; } - | { $$ = 0; } ; variable_decl_assignment /* IEEE1800-2005 A.2.3 */ @@ -2342,7 +2345,7 @@ tf_port_declaration /* IEEE1800-2005: A.2.7 */ tf_port_item /* IEEE1800-2005: A.2.7 */ - : port_direction_opt K_var_opt data_type_or_implicit IDENTIFIER dimensions_opt tf_port_item_expr_opt + : port_direction_opt K_var_opt data_type_or_implicit IDENTIFIER dimensions_opt initializer_opt { std::vector*tmp; NetNet::PortType use_port_type = $1; if ((use_port_type == NetNet::PIMPLICIT) && (gn_system_verilog() || ($3 == 0))) @@ -2379,6 +2382,7 @@ tf_port_item /* IEEE1800-2005: A.2.7 */ $$ = tmp; if ($6) { + pform_requires_sv(@6, "Task/function default argument"); assert(tmp->size()==1); tmp->front().defe = $6; } @@ -2393,16 +2397,6 @@ tf_port_item /* IEEE1800-2005: A.2.7 */ } ; - /* This rule matches the [ = ] part of the tf_port_item rules. */ - -tf_port_item_expr_opt - : '=' expression - { pform_requires_sv(@$, "Task/function default argument"); - $$ = $2; - } - | { $$ = 0; } - ; - tf_port_list /* IEEE1800-2005: A.2.7 */ : { port_declaration_context.port_type = gn_system_verilog() ? NetNet::PINPUT : NetNet::PIMPLICIT; port_declaration_context.data_type = 0; @@ -2572,21 +2566,13 @@ attribute_list attribute - : IDENTIFIER + : IDENTIFIER initializer_opt { named_pexpr_t*tmp = new named_pexpr_t; tmp->name = lex_strings.make($1); - tmp->parm = 0; + tmp->parm = $2; delete[]$1; $$ = tmp; } - | IDENTIFIER '=' expression - { PExpr*tmp = $3; - named_pexpr_t*tmp2 = new named_pexpr_t; - tmp2->name = lex_strings.make($1); - tmp2->parm = tmp; - delete[]$1; - $$ = tmp2; - } ; @@ -2786,42 +2772,22 @@ pos_neg_number ; enum_name - : IDENTIFIER + : IDENTIFIER initializer_opt { perm_string name = lex_strings.make($1); delete[]$1; - $$ = make_named_number(name); + $$ = make_named_number(name, $2); } - | IDENTIFIER '[' pos_neg_number ']' + | IDENTIFIER '[' pos_neg_number ']' initializer_opt { perm_string name = lex_strings.make($1); long count = check_enum_seq_value(@1, $3, false); + $$ = make_named_numbers(name, 0, count-1, $5); delete[]$1; - $$ = make_named_numbers(name, 0, count-1); delete $3; } - | IDENTIFIER '[' pos_neg_number ':' pos_neg_number ']' + | IDENTIFIER '[' pos_neg_number ':' pos_neg_number ']' initializer_opt { perm_string name = lex_strings.make($1); $$ = make_named_numbers(name, check_enum_seq_value(@1, $3, true), - check_enum_seq_value(@1, $5, true)); - delete[]$1; - delete $3; - delete $5; - } - | IDENTIFIER '=' expression - { perm_string name = lex_strings.make($1); - delete[]$1; - $$ = make_named_number(name, $3); - } - | IDENTIFIER '[' pos_neg_number ']' '=' expression - { perm_string name = lex_strings.make($1); - long count = check_enum_seq_value(@1, $3, false); - $$ = make_named_numbers(name, 0, count-1, $6); - delete[]$1; - delete $3; - } - | IDENTIFIER '[' pos_neg_number ':' pos_neg_number ']' '=' expression - { perm_string name = lex_strings.make($1); - $$ = make_named_numbers(name, check_enum_seq_value(@1, $3, true), - check_enum_seq_value(@1, $5, true), $8); + check_enum_seq_value(@1, $5, true), $7); delete[]$1; delete $3; delete $5; @@ -4317,14 +4283,10 @@ list_of_port_identifiers ; list_of_variable_port_identifiers - : IDENTIFIER dimensions_opt - { $$ = make_port_list($1, $2, 0); } - | IDENTIFIER dimensions_opt '=' expression - { $$ = make_port_list($1, $2, $4); } - | list_of_variable_port_identifiers ',' IDENTIFIER dimensions_opt - { $$ = make_port_list($1, $3, $4, 0); } - | list_of_variable_port_identifiers ',' IDENTIFIER dimensions_opt '=' expression - { $$ = make_port_list($1, $3, $4, $6); } + : IDENTIFIER dimensions_opt initializer_opt + { $$ = make_port_list($1, $2, $3); } + | list_of_variable_port_identifiers ',' IDENTIFIER dimensions_opt initializer_opt + { $$ = make_port_list($1, $3, $4, $5); } ; @@ -5250,19 +5212,12 @@ let_port_list // FIXME: What about the attributes? let_port_item - : attribute_list_opt let_formal_type IDENTIFIER dimensions_opt let_default_opt + : attribute_list_opt let_formal_type IDENTIFIER dimensions_opt initializer_opt { perm_string tmp3 = lex_strings.make($3); $$ = pform_make_let_port($2, tmp3, $4, $5); } ; -let_default_opt - : '=' expression - { $$ = $2; } - | - { $$ = 0; } - ; - let_formal_type : data_type_or_implicit { $$ = $1; } @@ -5436,15 +5391,9 @@ parameter_assign_list ; parameter_assign - : IDENTIFIER parameter_value_ranges_opt + : IDENTIFIER initializer_opt parameter_value_ranges_opt { pform_set_parameter(@1, lex_strings.make($1), param_is_local, - param_data_type, 0, $2); - delete[]$1; - } - | IDENTIFIER '=' expression parameter_value_ranges_opt - { PExpr*tmp = $3; - pform_set_parameter(@1, lex_strings.make($1), param_is_local, - param_data_type, tmp, $4); + param_data_type, $2, $3); delete[]$1; } ; @@ -6976,11 +6925,6 @@ udp_port_list udp_reg_opt: K_reg { $$ = true; } | { $$ = false; }; -udp_initial_expr_opt - : '=' expression { $$ = $2; } - | { $$ = 0; } - ; - udp_input_declaration_list : K_input IDENTIFIER { std::list*tmp = new std::list; @@ -7017,7 +6961,7 @@ udp_primitive names and declarations are all in the parameter list. */ | K_primitive IDENTIFIER - '(' K_output udp_reg_opt IDENTIFIER udp_initial_expr_opt ',' + '(' K_output udp_reg_opt IDENTIFIER initializer_opt ',' udp_input_declaration_list ')' ';' udp_body K_endprimitive label_opt From a19a07254b0ad3683b5aaa6bc46f71d22a096195 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 13 Sep 2022 10:09:18 +0200 Subject: [PATCH 3/4] Support default port values in port declarations lists Both Verilog (2005) and SystemVerilog support default port values for variable output ports. SystemVerilog also supports default port values for input ports. For port declaration lists it is possible to specify the default value for port identifier. E.g. ``` module M ( input integer x, y = 1, output integer z, w = 2 ) ... ``` Currently the parser only supports specifying the default value for the first identifier in the list. Extend the parser to also allow to specify the default value for identifiers in the list. Signed-off-by: Lars-Peter Clausen --- parse.y | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/parse.y b/parse.y index f1b12a4d4..52c67ab34 100644 --- a/parse.y +++ b/parse.y @@ -4332,13 +4332,29 @@ list_of_port_declarations tmp->push_back($3); $$ = tmp; } - | list_of_port_declarations ',' IDENTIFIER + | list_of_port_declarations ',' IDENTIFIER initializer_opt { Module::port_t*ptmp; perm_string name = lex_strings.make($3); ptmp = pform_module_port_reference(@3, name); std::vector*tmp = $1; tmp->push_back(ptmp); + if ($4) { + switch (port_declaration_context.port_type) { + case NetNet::PINOUT: + yyerror(@4, "error: Default port value not allowed for inout ports."); + break; + case NetNet::PINPUT: + pform_requires_sv(@4, "Default port value"); + ptmp->default_value = $4; + break; + case NetNet::POUTPUT: + pform_make_var_init(@3, name, $4); + break; + default: + break; + } + } /* Get the port declaration details, the port type and what not, from context data stored by the last port_declaration rule. */ From 6e4a1ac15ef86ece6f411876fdfc091a16af124a Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 13 Sep 2022 13:06:10 +0200 Subject: [PATCH 4/4] Add regression tests for module port list default values Check that default values are support for module port lists. * For output ports it is supported in both Verilog and SystemVerilog. * For input ports it is only supported in SystemVerilog. * For inout ports it is never supported Signed-off-by: Lars-Peter Clausen --- ivtest/ivltests/module_inout_port_list_def.v | 22 +++++++++++++ ivtest/ivltests/module_input_port_list_def.v | 31 +++++++++++++++++++ ivtest/ivltests/module_output_port_list_def.v | 31 +++++++++++++++++++ ivtest/regress-fsv.list | 1 + ivtest/regress-vlg.list | 3 ++ 5 files changed, 88 insertions(+) create mode 100644 ivtest/ivltests/module_inout_port_list_def.v create mode 100644 ivtest/ivltests/module_input_port_list_def.v create mode 100644 ivtest/ivltests/module_output_port_list_def.v diff --git a/ivtest/ivltests/module_inout_port_list_def.v b/ivtest/ivltests/module_inout_port_list_def.v new file mode 100644 index 000000000..dea3e042c --- /dev/null +++ b/ivtest/ivltests/module_inout_port_list_def.v @@ -0,0 +1,22 @@ +// Check that it is an error to specify a default value for inout port +// declarations. + +module M ( + inout [31:0] x, y = 1 // inout ports do not support default values +); + initial begin + $display("FAILED"); + end + +endmodule + +module test; + + wire [31:0] x, y; + + M i_m ( + .x(x), + .y(y) + ); + +endmodule diff --git a/ivtest/ivltests/module_input_port_list_def.v b/ivtest/ivltests/module_input_port_list_def.v new file mode 100644 index 000000000..54a5dac9f --- /dev/null +++ b/ivtest/ivltests/module_input_port_list_def.v @@ -0,0 +1,31 @@ +// Check that it is possible to specify a default port value for each port in a +// input port declaration list. + +module M ( + input [31:0] x = 1, y = 2 +); + + `define check(val, exp) \ + if (val !== exp) begin \ + $display("FAILED(%0d): %s, expected %0h got %0h", `__LINE__, `"val`", exp, val); \ + failed = 1'b1; \ + end + + reg failed = 1'b0; + + initial begin + `check(x, 1) + `check(y, 2) + + if (!failed) begin + $display("PASSED"); + end + end + +endmodule + +module test; + + M i_m (); + +endmodule diff --git a/ivtest/ivltests/module_output_port_list_def.v b/ivtest/ivltests/module_output_port_list_def.v new file mode 100644 index 000000000..d40f453fe --- /dev/null +++ b/ivtest/ivltests/module_output_port_list_def.v @@ -0,0 +1,31 @@ +// Check that it is possible to specify a default port value for each port in a +// output port declaration list. + +module M ( + output [31:0] x = 1, y = 2 +); + + `define check(val, exp) \ + if (val !== exp) begin \ + $display("FAILED(%0d): %s, expected %0h got %0h", `__LINE__, `"val`", exp, val); \ + failed = 1'b1; \ + end + + reg failed = 1'b0; + + initial begin + `check(x, 1) + `check(y, 2) + + if (!failed) begin + $display("PASSED"); + end + end + +endmodule + +module test; + + M i_m (); + +endmodule diff --git a/ivtest/regress-fsv.list b/ivtest/regress-fsv.list index f8e0625a8..80932efd4 100644 --- a/ivtest/regress-fsv.list +++ b/ivtest/regress-fsv.list @@ -78,6 +78,7 @@ br_gh567 normal ivltests check_constant_3 normal ivltests function4 normal ivltests module_inout_port_type normal ivltests +module_input_port_list_def normal,-g2005-sv ivltests module_input_port_type normal ivltests parameter_in_generate1 normal ivltests parameter_no_default normal ivltests diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index dfaad21fa..95d7c2ecf 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -651,7 +651,9 @@ mixed_width_case normal ivltests modparam normal ivltests top # Override parameter via passed down value module3.12A normal ivltests main module3.12B normal ivltests +module_inout_port_list_def CE ivltests # inout ports do not support default values module_inout_port_type CE ivltests +module_input_port_list_def CE ivltests # input ports only support default values in SV module_input_port_type CE ivltests module_nonansi_integer1 normal ivltests module_nonansi_integer2 normal ivltests @@ -659,6 +661,7 @@ module_nonansi_time1 normal ivltests module_nonansi_time2 normal ivltests module_nonansi_vec1 normal ivltests module_nonansi_vec2 normal ivltests +module_output_port_list_def normal ivltests module_output_port_var1 normal ivltests module_output_port_var2 normal ivltests module_port_range_mismatch CE ivltests