From 673b40b78ce0bb33adb974d24c44069e61090d8f Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Sat, 5 Feb 2022 12:04:55 +0100 Subject: [PATCH 1/2] Elaborate `parameter` as non-overridable where required For modules, programs and interfaces that have a parameter port list, a parameter declared inside the scope's body is supposed to be elaborated as a local parameter. TThis is described in the Verilog LRM (1364-2005) section 4.10.1 ("Module parameters") and the SystemVerilog LRM (1800-2017) section 6.20.1 ("Parameter declaration syntax"). Implement this by marking a parameter declared in such a way as non-overridable. Note that a parameter declared within a named block, function or task can still be overridden, even if the module has a parameter port list. Signed-off-by: Lars-Peter Clausen --- PScope.h | 1 + pform.cc | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/PScope.h b/PScope.h index 037974860..b9118d7ff 100644 --- a/PScope.h +++ b/PScope.h @@ -116,6 +116,7 @@ class LexicalScope { SymbolType symbol_type() const; }; std::mapparameters; + bool has_parameter_port_list; // Defined types in the scope. std::maptypedefs; diff --git a/pform.cc b/pform.cc index dd171773c..10b8d9f37 100644 --- a/pform.cc +++ b/pform.cc @@ -1384,6 +1384,7 @@ void pform_startmodule(const struct vlltype&loc, const char*name, void pform_start_parameter_port_list() { pform_in_parameter_port_list = true; + pform_peek_scope()->has_parameter_port_list = true; } void pform_end_parameter_port_list() @@ -3264,6 +3265,13 @@ void pform_set_parameter(const struct vlltype&loc, overridable = false; } + bool in_module = dynamic_cast(scope) && + scope == pform_cur_module.front(); + + if (!pform_in_parameter_port_list && in_module && + scope->has_parameter_port_list) + overridable = false; + Module::param_expr_t*parm = new Module::param_expr_t(); FILE_NAME(parm, loc); @@ -3278,8 +3286,7 @@ void pform_set_parameter(const struct vlltype&loc, scope->parameters[name] = parm; // Only a module keeps the position of the parameter. - if (overridable && - (dynamic_cast(scope)) && (scope == pform_cur_module.front())) + if (overridable && in_module) pform_cur_module.front()->param_names.push_back(name); } From bb74c6412d6110e23394e0678036f02dc16640b3 Mon Sep 17 00:00:00 2001 From: Lars-Peter Clausen Date: Tue, 15 Feb 2022 09:53:45 +0100 Subject: [PATCH 2/2] Refactor test for invalid parameter overrides Split the test into multiple tests that each check one type of invalid override rather than having one big tests that checks for everything. This allow to check whether the test passes or fails by seeing if it compiles or not. The one big test on the other hand relies on seeing the exact error messages as recorded in the gold file. Signed-off-by: Lars-Peter Clausen --- ivtest/gold/parameter_invalid_override.gold | 21 ------- ivtest/ivltests/parameter_invalid_override.v | 63 ------------------- ivtest/ivltests/parameter_override_invalid1.v | 19 ++++++ ivtest/ivltests/parameter_override_invalid2.v | 21 +++++++ ivtest/ivltests/parameter_override_invalid3.v | 17 +++++ ivtest/ivltests/parameter_override_invalid4.v | 17 +++++ ivtest/ivltests/parameter_override_invalid5.v | 22 +++++++ ivtest/ivltests/parameter_override_invalid6.v | 20 ++++++ ivtest/ivltests/parameter_override_invalid7.v | 18 ++++++ ivtest/ivltests/parameter_override_invalid8.v | 19 ++++++ ivtest/regress-sv.list | 3 +- ivtest/regress-vlg.list | 6 ++ 12 files changed, 161 insertions(+), 85 deletions(-) delete mode 100644 ivtest/gold/parameter_invalid_override.gold delete mode 100644 ivtest/ivltests/parameter_invalid_override.v create mode 100644 ivtest/ivltests/parameter_override_invalid1.v create mode 100644 ivtest/ivltests/parameter_override_invalid2.v create mode 100644 ivtest/ivltests/parameter_override_invalid3.v create mode 100644 ivtest/ivltests/parameter_override_invalid4.v create mode 100644 ivtest/ivltests/parameter_override_invalid5.v create mode 100644 ivtest/ivltests/parameter_override_invalid6.v create mode 100644 ivtest/ivltests/parameter_override_invalid7.v create mode 100644 ivtest/ivltests/parameter_override_invalid8.v diff --git a/ivtest/gold/parameter_invalid_override.gold b/ivtest/gold/parameter_invalid_override.gold deleted file mode 100644 index 029fc3262..000000000 --- a/ivtest/gold/parameter_invalid_override.gold +++ /dev/null @@ -1,21 +0,0 @@ -./ivltests/parameter_invalid_override.v:28: error: Cannot override localparam `C` in `test.i_a`. -./ivltests/parameter_invalid_override.v:29: error: Cannot override localparam `D` in `test.i_a`. -./ivltests/parameter_invalid_override.v:31: error: Cannot override localparam `F` in `test.i_a`. -./ivltests/parameter_invalid_override.v:32: error: Cannot override localparam `G` in `test.i_a`. -./ivltests/parameter_invalid_override.v:33: error: Cannot override localparam `H` in `test.i_a`. -./ivltests/parameter_invalid_override.v:34: error: Cannot override localparam `I` in `test.i_a`. -./ivltests/parameter_invalid_override.v:35: error: parameter `Z` not found in `test.i_a`. -./ivltests/parameter_invalid_override.v:52: error: Cannot override localparam `C` in `test.i_b`. -./ivltests/parameter_invalid_override.v:53: error: Cannot override localparam `D` in `test.i_b`. -./ivltests/parameter_invalid_override.v:54: error: parameter `Z` not found in `test.i_b`. -./ivltests/parameter_invalid_override.v:40: error: Cannot override localparam `C` in `test.i_a`. -./ivltests/parameter_invalid_override.v:41: error: Cannot override localparam `D` in `test.i_a`. -./ivltests/parameter_invalid_override.v:43: error: Cannot override localparam `F` in `test.i_a`. -./ivltests/parameter_invalid_override.v:44: error: Cannot override localparam `G` in `test.i_a`. -./ivltests/parameter_invalid_override.v:45: error: Cannot override localparam `H` in `test.i_a`. -./ivltests/parameter_invalid_override.v:46: error: Cannot override localparam `I` in `test.i_a`. -./ivltests/parameter_invalid_override.v:47: error: parameter `Z` not found in `test.i_a`. -./ivltests/parameter_invalid_override.v:59: error: Cannot override localparam `C` in `test.i_b`. -./ivltests/parameter_invalid_override.v:60: error: Cannot override localparam `D` in `test.i_b`. -./ivltests/parameter_invalid_override.v:61: error: parameter `Z` not found in `test.i_b`. -21 error(s) during elaboration. diff --git a/ivtest/ivltests/parameter_invalid_override.v b/ivtest/ivltests/parameter_invalid_override.v deleted file mode 100644 index 32843690c..000000000 --- a/ivtest/ivltests/parameter_invalid_override.v +++ /dev/null @@ -1,63 +0,0 @@ -// Check that invalid parameter overrides generate an error - -module a #( - parameter A = 1, B = 2, - localparam C = 3, localparam D = 4, // TODO: D should be localparam even when the keyword omitted - parameter E = 5 -); - - // TODO: parameter here should be treated as a localparam since the module has a - // parameter port list - /*parameter*/ localparam F = 6, G = 7; - localparam H = 8, I = 9; - -endmodule - -module b; - - parameter A = 1, B = 2; - localparam C = 3, D = 4; - -endmodule - -module test; - - a #( - .A(10), - .B(20), - .C(30), - .D(40), - .E(50), - .F(60), - .G(70), - .H(80), - .I(90), - .Z(99) - ) i_a(); - - defparam i_a.A = 100; - defparam i_a.B = 200; - defparam i_a.C = 300; - defparam i_a.D = 400; - defparam i_a.E = 500; - defparam i_a.F = 600; - defparam i_a.G = 700; - defparam i_a.H = 800; - defparam i_a.I = 900; - defparam i_a.Z = 999; - - b #( - .A(10), - .B(20), - .C(30), - .D(40), - .Z(99) - ) i_b(); - - defparam i_b.A = 100; - defparam i_b.B = 200; - defparam i_b.C = 300; - defparam i_b.D = 400; - defparam i_b.Z = 999; - -endmodule diff --git a/ivtest/ivltests/parameter_override_invalid1.v b/ivtest/ivltests/parameter_override_invalid1.v new file mode 100644 index 000000000..37828f07b --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid1.v @@ -0,0 +1,19 @@ +// Check that trying to override a parameter that does not exist results in an +// error + +module a #( + parameter A = 1 +); + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a #( + .Z(10) // Error + ) i_a(); + +endmodule diff --git a/ivtest/ivltests/parameter_override_invalid2.v b/ivtest/ivltests/parameter_override_invalid2.v new file mode 100644 index 000000000..b31828ea0 --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid2.v @@ -0,0 +1,21 @@ +// Check that trying to override a parameter that does not exist results in an +// error + +module a #( + parameter A = 1 +); + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a #( + .A(10) + ) i_a(); + + defparam i_a.Z = 10; // Error + +endmodule diff --git a/ivtest/ivltests/parameter_override_invalid3.v b/ivtest/ivltests/parameter_override_invalid3.v new file mode 100644 index 000000000..4b11d0b14 --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid3.v @@ -0,0 +1,17 @@ +// Check that localparam can not be overridden + +module a; + localparam A = 1; + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a #( + .A(10) // Error + ) i_a(); + +endmodule diff --git a/ivtest/ivltests/parameter_override_invalid4.v b/ivtest/ivltests/parameter_override_invalid4.v new file mode 100644 index 000000000..ee25717eb --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid4.v @@ -0,0 +1,17 @@ +// Check that localparam can not be overridden by defparam + +module a; + localparam A = 1; + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a i_a(); + + defparam i_a.A = 10; // Error + +endmodule diff --git a/ivtest/ivltests/parameter_override_invalid5.v b/ivtest/ivltests/parameter_override_invalid5.v new file mode 100644 index 000000000..1031321f9 --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid5.v @@ -0,0 +1,22 @@ +// Check that parameter declared in the module body can not be overridden if the +// module has a parameter port list. + +module a #( + parameter A = 1 +); + // This behaves like a localparam + parameter B = 1; + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a #( + .A(10), + .B(20) // Error + ) i_a(); + +endmodule diff --git a/ivtest/ivltests/parameter_override_invalid6.v b/ivtest/ivltests/parameter_override_invalid6.v new file mode 100644 index 000000000..cf24fce9e --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid6.v @@ -0,0 +1,20 @@ +// Check that parameter declared in the module body can not be overridden by a +// defparam if the module has a parameter port list. + +module a #(parameter A = 1); + // This behaves like a localparam + parameter B = 2; + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a i_a(); + + defparam i_a.A = 10; + defparam i_a.B = 20; // Error + +endmodule diff --git a/ivtest/ivltests/parameter_override_invalid7.v b/ivtest/ivltests/parameter_override_invalid7.v new file mode 100644 index 000000000..3d45475ee --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid7.v @@ -0,0 +1,18 @@ +// Check that localparam declared in parameter port list can not be overridden + +module a #( + localparam B = 2 +); + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a #( + .A(10) // Error + ) i_a(); + +endmodule diff --git a/ivtest/ivltests/parameter_override_invalid8.v b/ivtest/ivltests/parameter_override_invalid8.v new file mode 100644 index 000000000..caf3ceab7 --- /dev/null +++ b/ivtest/ivltests/parameter_override_invalid8.v @@ -0,0 +1,19 @@ +// Check that localparam declared in parameter port list can not be overridden +// by defparam + +module a #( + localparam A = 1 +); + + initial begin + $display("FAILED"); + end +endmodule + +module test; + + a i_a(); + + defparam i_a.A = 10; // Error + +endmodule diff --git a/ivtest/regress-sv.list b/ivtest/regress-sv.list index b0e865eb5..6ac0eda0a 100644 --- a/ivtest/regress-sv.list +++ b/ivtest/regress-sv.list @@ -317,11 +317,12 @@ named_fork_fail CE,-g2009 ivltests packeda normal,-g2009 ivltests packeda2 normal,-g2009 ivltests parameter_in_generate2 CE,-g2005-sv ivltests -parameter_invalid_override CE,-g2005-sv ivltests gold=parameter_invalid_override.gold parameter_no_default CE,-g2005-sv ivltests parameter_no_default_fail1 CE ivltests parameter_no_default_fail2 CE ivltests parameter_no_default_toplvl normal,-g2005-sv ivltests +parameter_override_invalid7 CE,-g2005-sv ivltests +parameter_override_invalid8 CE,-g2005-sv ivltests parameter_type2 normal,-g2009 ivltests parpkg_test normal,-g2009 ivltests parpkg_test2 normal,-g2009 ivltests diff --git a/ivtest/regress-vlg.list b/ivtest/regress-vlg.list index 127c60b07..e49a9afd4 100644 --- a/ivtest/regress-vlg.list +++ b/ivtest/regress-vlg.list @@ -703,6 +703,12 @@ parameter_omit3 CE ivltests parameter_omit_invalid1 CE ivltests parameter_omit_invalid2 CE ivltests parameter_omit_invalid3 CE ivltests +parameter_override_invalid1 CE ivltests +parameter_override_invalid2 CE ivltests +parameter_override_invalid3 CE ivltests +parameter_override_invalid4 CE ivltests +parameter_override_invalid5 CE ivltests +parameter_override_invalid6 CE ivltests patch1268 normal ivltests pca1 normal ivltests # Procedural Continuous Assignment in a mux pic normal contrib pictest gold=pic.gold