From: Zachary Snow <zach@zachjs.com>
Date: Sun, 21 Feb 2021 19:45:21 +0000 (-0500)
Subject: verilog: fix sizing of constant args for tasks/functions
X-Git-Tag: working-ls180~42^2
X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=b6af90fe20bc92631061c48c328f3c6e4760e4df;p=yosys.git

verilog: fix sizing of constant args for tasks/functions

- Simplify synthetic localparams for normal calls to update their width
    - This step was inadvertently removed alongside `added_mod_children`
- Support redeclaration of constant function arguments
    - `eval_const_function` never correctly handled this, but the issue
      was not exposed in the existing tests until the recent change to
      always attempt constant function evaluation when all-const args
      are used
- Check asserts in const_arg_loop and const_func tests
- Add coverage for width mismatch error cases
---

diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h
index d8818df31..b793c6c2d 100644
--- a/frontends/ast/ast.h
+++ b/frontends/ast/ast.h
@@ -263,7 +263,13 @@ namespace AST
 		bool detect_latch(const std::string &var);
 
 		// additional functionality for evaluating constant functions
-		struct varinfo_t { RTLIL::Const val; int offset; bool is_signed; };
+		struct varinfo_t {
+			RTLIL::Const val;
+			int offset;
+			bool is_signed;
+			AstNode *arg = nullptr;
+			bool explicitly_sized;
+		};
 		bool has_const_only_constructs();
 		bool replace_variables(std::map<std::string, varinfo_t> &variables, AstNode *fcall, bool must_succeed);
 		AstNode *eval_const_function(AstNode *fcall, bool must_succeed);
diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc
index 402b7257b..df7c9282c 100644
--- a/frontends/ast/simplify.cc
+++ b/frontends/ast/simplify.cc
@@ -3376,6 +3376,8 @@ skip_dynamic_range_lvalue_expansion:;
 								range->children.push_back(mkconst_int(0, true));
 							}
 						}
+						// updates the sizing
+						while (wire->simplify(true, false, false, 1, -1, false, false)) { }
 						continue;
 					}
 					AstNode *wire_id = new AstNode(AST_IDENTIFIER);
@@ -4591,17 +4593,29 @@ AstNode *AstNode::eval_const_function(AstNode *fcall, bool must_succeed)
 				log_file_error(stmt->filename, stmt->location.first_line, "Can't determine size of variable %s\n%s:%d.%d-%d.%d: ... called from here.\n",
 						stmt->str.c_str(), fcall->filename.c_str(), fcall->location.first_line, fcall->location.first_column, fcall->location.last_line, fcall->location.last_column);
 			}
-			variables[stmt->str].val = RTLIL::Const(RTLIL::State::Sx, abs(stmt->range_left - stmt->range_right)+1);
-			variables[stmt->str].offset = min(stmt->range_left, stmt->range_right);
-			variables[stmt->str].is_signed = stmt->is_signed;
+			AstNode::varinfo_t &variable = variables[stmt->str];
+			int width = abs(stmt->range_left - stmt->range_right) + 1;
+			// if this variable has already been declared as an input, check the
+			// sizes match if it already had an explicit size
+			if (variable.arg && variable.explicitly_sized && variable.val.size() != width) {
+				log_file_error(filename, location.first_line, "Incompatible re-declaration of constant function wire %s.\n", stmt->str.c_str());
+			}
+			variable.val = RTLIL::Const(RTLIL::State::Sx, width);
+			variable.offset = min(stmt->range_left, stmt->range_right);
+			variable.is_signed = stmt->is_signed;
+			variable.explicitly_sized = stmt->children.size() &&
+				stmt->children.back()->type == AST_RANGE;
+			// identify the argument corresponding to this wire, if applicable
 			if (stmt->is_input && argidx < fcall->children.size()) {
-				int width = variables[stmt->str].val.bits.size();
-				auto* arg_node = fcall->children.at(argidx++);
-				if (arg_node->type == AST_CONSTANT) {
-					variables[stmt->str].val = arg_node->bitsAsConst(width);
+				variable.arg = fcall->children.at(argidx++);
+			}
+			// load the constant arg's value into this variable
+			if (variable.arg) {
+				if (variable.arg->type == AST_CONSTANT) {
+					variable.val = variable.arg->bitsAsConst(width);
 				} else {
-					log_assert(arg_node->type == AST_REALVALUE);
-					variables[stmt->str].val = arg_node->realAsConst(width);
+					log_assert(variable.arg->type == AST_REALVALUE);
+					variable.val = variable.arg->realAsConst(width);
 				}
 			}
 			current_scope[stmt->str] = stmt;
diff --git a/tests/various/const_arg_loop.sv b/tests/various/const_arg_loop.sv
new file mode 100644
index 000000000..f28d06e68
--- /dev/null
+++ b/tests/various/const_arg_loop.sv
@@ -0,0 +1,92 @@
+module top;
+	function automatic [31:0] operation1;
+		input [4:0] rounds;
+		input integer num;
+		integer i;
+		begin
+			begin : shadow
+				integer rounds;
+				rounds = 0;
+			end
+			for (i = 0; i < rounds; i = i + 1)
+				num = num * 2;
+			operation1 = num;
+		end
+	endfunction
+
+	function automatic [31:0] pass_through;
+		input [31:0] inp;
+		pass_through = inp;
+	endfunction
+
+	function automatic [31:0] operation2;
+		input [4:0] inp;
+		input integer num;
+		begin
+			inp[0] = inp[0] ^ 1;
+			operation2 = num * inp;
+		end
+	endfunction
+
+	function automatic [31:0] operation3;
+		input [4:0] rounds;
+		input integer num;
+		reg [4:0] rounds;
+		integer i;
+		begin
+			begin : shadow
+				integer rounds;
+				rounds = 0;
+			end
+			for (i = 0; i < rounds; i = i + 1)
+				num = num * 2;
+			operation3 = num;
+		end
+	endfunction
+
+	function automatic [16:0] operation4;
+		input [15:0] a;
+		input b;
+		operation4 = {a, b};
+	endfunction
+
+	function automatic integer operation5;
+		input x;
+		integer x;
+		operation5 = x;
+	endfunction
+
+	wire [31:0] a;
+	assign a = 2;
+
+	parameter A = 3;
+
+	wire [31:0] x1;
+	assign x1 = operation1(A, a);
+
+	wire [31:0] x1b;
+	assign x1b = operation1(pass_through(A), a);
+
+	wire [31:0] x2;
+	assign x2 = operation2(A, a);
+
+	wire [31:0] x3;
+	assign x3 = operation3(A, a);
+
+	wire [16:0] x4;
+	assign x4 = operation4(a[15:0], 0);
+
+	wire [31:0] x5;
+	assign x5 = operation5(64);
+
+	always_comb begin
+		assert(a == 2);
+		assert(A == 3);
+		assert(x1 == 16);
+		assert(x1b == 16);
+		assert(x2 == 4);
+		assert(x3 == 16);
+		assert(x4 == a << 1);
+		assert(x5 == 64);
+	end
+endmodule
diff --git a/tests/various/const_arg_loop.v b/tests/various/const_arg_loop.v
deleted file mode 100644
index 358fb439a..000000000
--- a/tests/various/const_arg_loop.v
+++ /dev/null
@@ -1,93 +0,0 @@
-module top;
-	function automatic [31:0] operation1;
-		input [4:0] rounds;
-		input integer num;
-		integer i;
-		begin
-			begin : shadow
-				integer rounds;
-				rounds = 0;
-			end
-			for (i = 0; i < rounds; i = i + 1)
-				num = num * 2;
-			operation1 = num;
-		end
-	endfunction
-
-	function automatic [31:0] pass_through;
-		input [31:0] inp;
-		pass_through = inp;
-	endfunction
-
-	function automatic [31:0] operation2;
-		input [4:0] var;
-		input integer num;
-		begin
-			var[0] = var[0] ^ 1;
-			operation2 = num * var;
-		end
-	endfunction
-
-	function automatic [31:0] operation3;
-		input [4:0] rounds;
-		input integer num;
-		reg [4:0] rounds;
-		integer i;
-		begin
-			begin : shadow
-				integer rounds;
-				rounds = 0;
-			end
-			for (i = 0; i < rounds; i = i + 1)
-				num = num * 2;
-			operation3 = num;
-		end
-	endfunction
-
-	function automatic [16:0] operation4;
-		input [15:0] a;
-		input b;
-		operation4 = {a, b};
-	endfunction
-
-	function automatic integer operation5;
-		input x;
-		integer x;
-		operation5 = x;
-	endfunction
-
-	wire [31:0] a;
-	assign a = 2;
-
-	parameter A = 3;
-
-	wire [31:0] x1;
-	assign x1 = operation1(A, a);
-
-	wire [31:0] x1b;
-	assign x1b = operation1(pass_through(A), a);
-
-	wire [31:0] x2;
-	assign x2 = operation2(A, a);
-
-	wire [31:0] x3;
-	assign x3 = operation3(A, a);
-
-	wire [16:0] x4;
-	assign x4 = operation4(a[15:0], 0);
-
-	wire [31:0] x5;
-	assign x5 = operation5(64);
-
-// `define VERIFY
-`ifdef VERIFY
-    assert property (a == 2);
-    assert property (A == 3);
-    assert property (x1 == 16);
-    assert property (x1b == 16);
-    assert property (x2 == 4);
-    assert property (x3 == 16);
-    assert property (x4 == a << 1);
-    assert property (x5 == 64);
-`endif
-endmodule
diff --git a/tests/various/const_arg_loop.ys b/tests/various/const_arg_loop.ys
index b039bda10..392532213 100644
--- a/tests/various/const_arg_loop.ys
+++ b/tests/various/const_arg_loop.ys
@@ -1 +1,6 @@
-read_verilog const_arg_loop.v
+read_verilog -sv const_arg_loop.sv
+hierarchy
+proc
+opt -full
+select -module top
+sat -verify -seq 1 -tempinduct -prove-asserts -show-all
diff --git a/tests/various/const_func.sv b/tests/various/const_func.sv
new file mode 100644
index 000000000..af65f5c73
--- /dev/null
+++ b/tests/various/const_func.sv
@@ -0,0 +1,86 @@
+module Example(outA, outB, outC, outD);
+    parameter OUTPUT = "FOO";
+    output wire [23:0] outA;
+    output wire [23:0] outB;
+    output reg outC, outD;
+    function automatic [23:0] flip;
+        input [23:0] inp;
+        flip = ~inp;
+    endfunction
+
+    generate
+        if (flip(OUTPUT) == flip("BAR"))
+            assign outA = OUTPUT;
+        else
+            assign outA = 0;
+
+        case (flip(OUTPUT))
+            flip("FOO"): assign outB = OUTPUT;
+            flip("BAR"): assign outB = 0;
+            flip("BAZ"): assign outB = "HI";
+        endcase
+
+        genvar i;
+        initial outC = 0;
+        for (i = 0; i != flip(flip(OUTPUT[15:8])); i = i + 1)
+            if (i + 1 == flip(flip("O")))
+                initial outC = 1;
+    endgenerate
+
+    integer j;
+    initial begin
+        outD = 1;
+        for (j = 0; j != flip(flip(OUTPUT[15:8])); j = j + 1)
+            if (j + 1 == flip(flip("O")))
+                outD = 0;
+    end
+endmodule
+
+module top(out);
+    wire [23:0] a1, a2, a3, a4;
+    wire [23:0] b1, b2, b3, b4;
+    wire        c1, c2, c3, c4;
+    wire        d1, d2, d3, d4;
+    Example          e1(a1, b1, c1, d1);
+    Example #("FOO") e2(a2, b2, c2, d2);
+    Example #("BAR") e3(a3, b3, c3, d3);
+    Example #("BAZ") e4(a4, b4, c4, d4);
+
+    output wire [24 * 8 - 1 + 4 :0] out;
+    assign out = {
+        a1, a2, a3, a4,
+        b1, b2, b3, b4,
+        c1, c2, c3, c4,
+        d1, d2, d3, d4};
+
+    function signed [31:0] negate;
+        input integer inp;
+        negate = ~inp;
+    endfunction
+    parameter W = 10;
+    parameter X = 3;
+    localparam signed Y = $floor(W / X);
+    localparam signed Z = negate($floor(W / X));
+
+    always_comb begin
+        assert(a1 == 0);
+        assert(a2 == 0);
+        assert(a3 == "BAR");
+        assert(a4 == 0);
+        assert(b1 == "FOO");
+        assert(b2 == "FOO");
+        assert(b3 == 0);
+        assert(b4 == "HI");
+        assert(c1 == 1);
+        assert(c2 == 1);
+        assert(c3 == 0);
+        assert(c4 == 0);
+        assert(d1 == 0);
+        assert(d2 == 0);
+        assert(d3 == 1);
+        assert(d4 == 1);
+
+        assert(Y == 3);
+        assert(Z == ~3);
+    end
+endmodule
diff --git a/tests/various/const_func.v b/tests/various/const_func.v
deleted file mode 100644
index 541e63b19..000000000
--- a/tests/various/const_func.v
+++ /dev/null
@@ -1,87 +0,0 @@
-module Example(outA, outB, outC, outD);
-    parameter OUTPUT = "FOO";
-    output wire [23:0] outA;
-    output wire [23:0] outB;
-    output reg outC, outD;
-    function automatic [23:0] flip;
-        input [23:0] inp;
-        flip = ~inp;
-    endfunction
-
-    generate
-        if (flip(OUTPUT) == flip("BAR"))
-            assign outA = OUTPUT;
-        else
-            assign outA = 0;
-
-        case (flip(OUTPUT))
-            flip("FOO"): assign outB = OUTPUT;
-            flip("BAR"): assign outB = 0;
-            flip("BAZ"): assign outB = "HI";
-        endcase
-
-        genvar i;
-        initial outC = 0;
-        for (i = 0; i != flip(flip(OUTPUT[15:8])); i = i + 1)
-            if (i + 1 == flip(flip("O")))
-                initial outC = 1;
-    endgenerate
-
-    integer j;
-    initial begin
-        outD = 1;
-        for (j = 0; j != flip(flip(OUTPUT[15:8])); j = j + 1)
-            if (j + 1 == flip(flip("O")))
-                outD = 0;
-    end
-endmodule
-
-module top(out);
-    wire [23:0] a1, a2, a3, a4;
-    wire [23:0] b1, b2, b3, b4;
-    wire        c1, c2, c3, c4;
-    wire        d1, d2, d3, d4;
-    Example          e1(a1, b1, c1, d1);
-    Example #("FOO") e2(a2, b2, c2, d2);
-    Example #("BAR") e3(a3, b3, c3, d3);
-    Example #("BAZ") e4(a4, b4, c4, d4);
-
-    output wire [24 * 8 - 1 + 4 :0] out;
-    assign out = {
-        a1, a2, a3, a4,
-        b1, b2, b3, b4,
-        c1, c2, c3, c4,
-        d1, d2, d3, d4};
-
-    function signed [31:0] negate;
-        input integer inp;
-        negate = ~inp;
-    endfunction
-    parameter W = 10;
-    parameter X = 3;
-    localparam signed Y = $floor(W / X);
-    localparam signed Z = negate($floor(W / X));
-
-// `define VERIFY
-`ifdef VERIFY
-    assert property (a1 == 0);
-    assert property (a2 == 0);
-    assert property (a3 == "BAR");
-    assert property (a4 == 0);
-    assert property (b1 == "FOO");
-    assert property (b2 == "FOO");
-    assert property (b3 == 0);
-    assert property (b4 == "HI");
-    assert property (c1 == 1);
-    assert property (c2 == 1);
-    assert property (c3 == 0);
-    assert property (c4 == 0);
-    assert property (d1 == 0);
-    assert property (d2 == 0);
-    assert property (d3 == 1);
-    assert property (d4 == 1);
-
-    assert property (Y == 3);
-    assert property (Z == ~3);
-`endif
-endmodule
diff --git a/tests/various/const_func.ys b/tests/various/const_func.ys
index 5e3c04105..2f60acfe6 100644
--- a/tests/various/const_func.ys
+++ b/tests/various/const_func.ys
@@ -1 +1,7 @@
-read_verilog const_func.v
+read_verilog -sv const_func.sv
+hierarchy
+proc
+flatten
+opt -full
+select -module top
+sat -verify -seq 1 -tempinduct -prove-asserts -show-all
diff --git a/tests/verilog/func_arg_mismatch_1.ys b/tests/verilog/func_arg_mismatch_1.ys
new file mode 100644
index 000000000..a0e82db0c
--- /dev/null
+++ b/tests/verilog/func_arg_mismatch_1.ys
@@ -0,0 +1,12 @@
+logger -expect error "Incompatible re-declaration of wire" 1
+read_verilog -sv <<EOT
+module top;
+    function automatic integer f;
+        input [0:0] inp;
+        integer inp;
+        f = inp;
+    endfunction
+    integer x, y;
+    initial x = f(y);
+endmodule
+EOT
diff --git a/tests/verilog/func_arg_mismatch_2.ys b/tests/verilog/func_arg_mismatch_2.ys
new file mode 100644
index 000000000..c2c29c1fb
--- /dev/null
+++ b/tests/verilog/func_arg_mismatch_2.ys
@@ -0,0 +1,12 @@
+logger -expect error "Incompatible re-declaration of constant function wire" 1
+read_verilog -sv <<EOT
+module top;
+    function automatic integer f;
+        input [0:0] inp;
+        integer inp;
+        f = inp;
+    endfunction
+    integer x;
+    initial x = f(0);
+endmodule
+EOT
diff --git a/tests/verilog/func_arg_mismatch_3.ys b/tests/verilog/func_arg_mismatch_3.ys
new file mode 100644
index 000000000..892824c09
--- /dev/null
+++ b/tests/verilog/func_arg_mismatch_3.ys
@@ -0,0 +1,12 @@
+logger -expect error "Incompatible re-declaration of wire" 1
+read_verilog -sv <<EOT
+module top;
+    function automatic integer f;
+        input [1:0] inp;
+        integer inp;
+        f = inp;
+    endfunction
+    integer x, y;
+    initial x = f(y);
+endmodule
+EOT
diff --git a/tests/verilog/func_arg_mismatch_4.ys b/tests/verilog/func_arg_mismatch_4.ys
new file mode 100644
index 000000000..87ec1c299
--- /dev/null
+++ b/tests/verilog/func_arg_mismatch_4.ys
@@ -0,0 +1,12 @@
+logger -expect error "Incompatible re-declaration of constant function wire" 1
+read_verilog -sv <<EOT
+module top;
+    function automatic integer f;
+        input [1:0] inp;
+        integer inp;
+        f = inp;
+    endfunction
+    integer x;
+    initial x = f(0);
+endmodule
+EOT