From d738b2c1272b02d8799e9feda83b1eae8ba10c07 Mon Sep 17 00:00:00 2001
From: Zachary Snow <zach@zachjs.com>
Date: Tue, 2 Mar 2021 10:43:53 -0500
Subject: [PATCH] sv: support for parameters without default values

- Modules with a parameter without a default value will be automatically
  deferred until the hierarchy pass
- Allows for parameters without defaults as module items, rather than
  just int the `parameter_port_list`, despite being forbidden in the LRM
- Check for parameters without defaults that haven't been overriden
- Add location info to parameter/localparam declarations
---
 frontends/ast/ast.cc                         | 29 ++++++++++-
 frontends/verilog/verilog_parser.y           | 24 +++++++--
 tests/verilog/localparam_no_default_1.ys     | 17 +++++++
 tests/verilog/localparam_no_default_2.ys     | 15 ++++++
 tests/verilog/param_no_default.sv            | 52 ++++++++++++++++++++
 tests/verilog/param_no_default.ys            |  7 +++
 tests/verilog/param_no_default_not_svmode.ys | 26 ++++++++++
 tests/verilog/param_no_default_unbound_1.ys  | 12 +++++
 tests/verilog/param_no_default_unbound_2.ys  | 12 +++++
 tests/verilog/param_no_default_unbound_3.ys  | 12 +++++
 tests/verilog/param_no_default_unbound_4.ys  | 12 +++++
 tests/verilog/param_no_default_unbound_5.ys  | 12 +++++
 12 files changed, 225 insertions(+), 5 deletions(-)
 create mode 100644 tests/verilog/localparam_no_default_1.ys
 create mode 100644 tests/verilog/localparam_no_default_2.ys
 create mode 100644 tests/verilog/param_no_default.sv
 create mode 100644 tests/verilog/param_no_default.ys
 create mode 100644 tests/verilog/param_no_default_not_svmode.ys
 create mode 100644 tests/verilog/param_no_default_unbound_1.ys
 create mode 100644 tests/verilog/param_no_default_unbound_2.ys
 create mode 100644 tests/verilog/param_no_default_unbound_3.ys
 create mode 100644 tests/verilog/param_no_default_unbound_4.ys
 create mode 100644 tests/verilog/param_no_default_unbound_5.ys

diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc
index 57552d86c..7fb61bb34 100644
--- a/frontends/ast/ast.cc
+++ b/frontends/ast/ast.cc
@@ -968,6 +968,14 @@ void AST::set_src_attr(RTLIL::AttrObject *obj, const AstNode *ast)
 	obj->attributes[ID::src] = ast->loc_string();
 }
 
+static bool param_has_no_default(const AstNode *param) {
+	const auto &children = param->children;
+	log_assert(param->type == AST_PARAMETER);
+	log_assert(children.size() <= 2);
+	return children.empty() ||
+		(children.size() == 1 && children[0]->type == AST_RANGE);
+}
+
 // create a new AstModule from an AST_MODULE AST node
 static AstModule* process_module(AstNode *ast, bool defer, AstNode *original_ast = NULL, bool quiet = false)
 {
@@ -1006,6 +1014,10 @@ static AstModule* process_module(AstNode *ast, bool defer, AstNode *original_ast
 
 	if (!defer)
 	{
+		for (const AstNode *node : ast->children)
+			if (node->type == AST_PARAMETER && param_has_no_default(node))
+				log_file_error(node->filename, node->location.first_line, "Parameter `%s' has no default value and has not been overridden!\n", node->str.c_str());
+
 		bool blackbox_module = flag_lib;
 
 		if (!blackbox_module && !flag_noblackbox) {
@@ -1229,7 +1241,18 @@ void AST::process(RTLIL::Design *design, AstNode *ast, bool dump_ast1, bool dump
 			if (flag_icells && (*it)->str.compare(0, 2, "\\$") == 0)
 				(*it)->str = (*it)->str.substr(1);
 
-			if (defer)
+			bool defer_local = defer;
+			if (!defer_local)
+				for (const AstNode *node : (*it)->children)
+					if (node->type == AST_PARAMETER && param_has_no_default(node))
+					{
+						log("Deferring `%s' because it contains parameter(s) without defaults.\n", ast->str.c_str());
+						defer_local = true;
+						break;
+					}
+
+
+			if (defer_local)
 				(*it)->str = "$abstract" + (*it)->str;
 
 			if (design->has((*it)->str)) {
@@ -1248,7 +1271,7 @@ void AST::process(RTLIL::Design *design, AstNode *ast, bool dump_ast1, bool dump
 				}
 			}
 
-			design->add(process_module(*it, defer));
+			design->add(process_module(*it, defer_local));
 			current_ast_mod = nullptr;
 		}
 		else if ((*it)->type == AST_PACKAGE) {
@@ -1619,6 +1642,8 @@ std::string AstModule::derive_common(RTLIL::Design *design, const dict<RTLIL::Id
 		}
 		continue;
 	rewrite_parameter:
+		if (param_has_no_default(child))
+			child->children.insert(child->children.begin(), nullptr);
 		delete child->children.at(0);
 		if ((it->second.flags & RTLIL::CONST_FLAG_REAL) != 0) {
 			child->children[0] = new AstNode(AST_REALVALUE);
diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y
index bcba9b76a..ea8cc0765 100644
--- a/frontends/verilog/verilog_parser.y
+++ b/frontends/verilog/verilog_parser.y
@@ -1462,7 +1462,26 @@ param_decl_list:
 	single_param_decl | param_decl_list ',' single_param_decl;
 
 single_param_decl:
-	TOK_ID '=' expr {
+	single_param_decl_ident '=' expr {
+		AstNode *decl = ast_stack.back()->children.back();
+		log_assert(decl->type == AST_PARAMETER || decl->type == AST_LOCALPARAM);
+		delete decl->children[0];
+		decl->children[0] = $3;
+	} |
+	single_param_decl_ident {
+		AstNode *decl = ast_stack.back()->children.back();
+		if (decl->type != AST_PARAMETER) {
+			log_assert(decl->type == AST_LOCALPARAM);
+			frontend_verilog_yyerror("localparam initialization is missing!");
+		}
+		if (!sv_mode)
+			frontend_verilog_yyerror("Parameter defaults can only be omitted in SystemVerilog mode!");
+		delete decl->children[0];
+		decl->children.erase(decl->children.begin());
+	};
+
+single_param_decl_ident:
+	TOK_ID {
 		AstNode *node;
 		if (astbuf1 == nullptr) {
 			if (!sv_mode)
@@ -1473,10 +1492,9 @@ single_param_decl:
 			node = astbuf1->clone();
 		}
 		node->str = *$1;
-		delete node->children[0];
-		node->children[0] = $3;
 		ast_stack.back()->children.push_back(node);
 		delete $1;
+		SET_AST_NODE_LOC(node, @1, @1);
 	};
 
 defparam_decl:
diff --git a/tests/verilog/localparam_no_default_1.ys b/tests/verilog/localparam_no_default_1.ys
new file mode 100644
index 000000000..426a48a1c
--- /dev/null
+++ b/tests/verilog/localparam_no_default_1.ys
@@ -0,0 +1,17 @@
+logger -expect-no-warnings
+read_verilog -sv <<EOF
+module Module #(
+    localparam X = 1
+);
+endmodule
+EOF
+
+design -reset
+
+logger -expect error "localparam initialization is missing!" 1
+read_verilog <<EOF
+module Module #(
+    localparam X
+);
+endmodule
+EOF
diff --git a/tests/verilog/localparam_no_default_2.ys b/tests/verilog/localparam_no_default_2.ys
new file mode 100644
index 000000000..b7b2622ad
--- /dev/null
+++ b/tests/verilog/localparam_no_default_2.ys
@@ -0,0 +1,15 @@
+logger -expect-no-warnings
+read_verilog -sv <<EOF
+module Module;
+    localparam X = 1;
+endmodule
+EOF
+
+design -reset
+
+logger -expect error "localparam initialization is missing!" 1
+read_verilog <<EOF
+module Module;
+    localparam X;
+endmodule
+EOF
diff --git a/tests/verilog/param_no_default.sv b/tests/verilog/param_no_default.sv
new file mode 100644
index 000000000..cc35bd2ea
--- /dev/null
+++ b/tests/verilog/param_no_default.sv
@@ -0,0 +1,52 @@
+module example #(
+    parameter w,
+    parameter x = 1,
+    parameter byte y,
+    parameter byte z = 3
+) (
+    output a, b,
+    output byte c, d
+);
+    assign a = w;
+    assign b = x;
+    assign c = y;
+    assign d = z;
+endmodule
+
+module top;
+    wire a1, b1;
+    wire a2, b2;
+    wire a3, b3;
+    wire a4, b4;
+    byte c1, d1;
+    byte c2, d2;
+    byte c3, d3;
+    byte c4, d4;
+
+    example #(0, 1, 2) e1(a1, b1, c1, d1);
+    example #(.w(1), .y(4)) e2(a2, b2, c2, d2);
+    example #(.x(0), .w(1), .y(5)) e3(a3, b3, c3, d3);
+    example #(1, 0, 9, 10) e4(a4, b4, c4, d4);
+
+    always @* begin
+        assert (a1 == 0);
+        assert (b1 == 1);
+        assert (c1 == 2);
+        assert (d1 == 3);
+
+        assert (a2 == 1);
+        assert (b2 == 1);
+        assert (c2 == 4);
+        assert (d3 == 3);
+
+        assert (a3 == 1);
+        assert (b3 == 0);
+        assert (c3 == 5);
+        assert (d3 == 3);
+
+        assert (a4 == 1);
+        assert (b4 == 0);
+        assert (c4 == 9);
+        assert (d4 == 10);
+    end
+endmodule
diff --git a/tests/verilog/param_no_default.ys b/tests/verilog/param_no_default.ys
new file mode 100644
index 000000000..7f161a909
--- /dev/null
+++ b/tests/verilog/param_no_default.ys
@@ -0,0 +1,7 @@
+read_verilog -sv param_no_default.sv
+hierarchy
+proc
+flatten
+opt -full
+select -module top
+sat -verify -seq 1 -tempinduct -prove-asserts -show-all
diff --git a/tests/verilog/param_no_default_not_svmode.ys b/tests/verilog/param_no_default_not_svmode.ys
new file mode 100644
index 000000000..1ded84e9c
--- /dev/null
+++ b/tests/verilog/param_no_default_not_svmode.ys
@@ -0,0 +1,26 @@
+logger -expect-no-warnings
+read_verilog -sv <<EOF
+module Module;
+    parameter X;
+endmodule
+EOF
+
+design -reset
+
+logger -expect-no-warnings
+read_verilog -sv <<EOF
+module Module #(
+    parameter X
+);
+endmodule
+EOF
+
+design -reset
+
+logger -expect error "Parameter defaults can only be omitted in SystemVerilog mode!" 1
+read_verilog <<EOF
+module Module #(
+    parameter X
+);
+endmodule
+EOF
diff --git a/tests/verilog/param_no_default_unbound_1.ys b/tests/verilog/param_no_default_unbound_1.ys
new file mode 100644
index 000000000..4aab85ab5
--- /dev/null
+++ b/tests/verilog/param_no_default_unbound_1.ys
@@ -0,0 +1,12 @@
+read_verilog -sv <<EOF
+module Example #(
+    parameter X
+);
+endmodule
+module top;
+    Example e();
+endmodule
+EOF
+
+logger -expect error "Parameter `\\X' has no default value and has not been overridden!" 1
+hierarchy -top top
diff --git a/tests/verilog/param_no_default_unbound_2.ys b/tests/verilog/param_no_default_unbound_2.ys
new file mode 100644
index 000000000..4b7f3b028
--- /dev/null
+++ b/tests/verilog/param_no_default_unbound_2.ys
@@ -0,0 +1,12 @@
+read_verilog -sv <<EOF
+module Example #(
+    parameter X, Y
+);
+endmodule
+module top;
+    Example e();
+endmodule
+EOF
+
+logger -expect error "Parameter `\\X' has no default value and has not been overridden!" 1
+hierarchy -top top
diff --git a/tests/verilog/param_no_default_unbound_3.ys b/tests/verilog/param_no_default_unbound_3.ys
new file mode 100644
index 000000000..f32b879a5
--- /dev/null
+++ b/tests/verilog/param_no_default_unbound_3.ys
@@ -0,0 +1,12 @@
+read_verilog -sv <<EOF
+module Example #(
+    parameter X, Y
+);
+endmodule
+module top;
+    Example #(1) e();
+endmodule
+EOF
+
+logger -expect error "Parameter `\\Y' has no default value and has not been overridden!" 1
+hierarchy -top top
diff --git a/tests/verilog/param_no_default_unbound_4.ys b/tests/verilog/param_no_default_unbound_4.ys
new file mode 100644
index 000000000..3a8d69d78
--- /dev/null
+++ b/tests/verilog/param_no_default_unbound_4.ys
@@ -0,0 +1,12 @@
+read_verilog -sv <<EOF
+module Example #(
+    parameter X, Y
+);
+endmodule
+module top;
+    Example #(.Y(1)) e();
+endmodule
+EOF
+
+logger -expect error "Parameter `\\X' has no default value and has not been overridden!" 1
+hierarchy -top top
diff --git a/tests/verilog/param_no_default_unbound_5.ys b/tests/verilog/param_no_default_unbound_5.ys
new file mode 100644
index 000000000..30282eba7
--- /dev/null
+++ b/tests/verilog/param_no_default_unbound_5.ys
@@ -0,0 +1,12 @@
+read_verilog -sv <<EOF
+module Example #(
+    parameter X, Y = 2
+);
+endmodule
+module top;
+    Example #(.Y(1)) e();
+endmodule
+EOF
+
+logger -expect error "Parameter `\\X' has no default value and has not been overridden!" 1
+hierarchy -top top
-- 
2.30.2