verilog: fix sizing of constant args for tasks/functions
authorZachary Snow <zach@zachjs.com>
Sun, 21 Feb 2021 19:45:21 +0000 (14:45 -0500)
committerZachary Snow <zach@zachjs.com>
Sun, 21 Feb 2021 20:44:43 +0000 (15:44 -0500)
- 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

12 files changed:
frontends/ast/ast.h
frontends/ast/simplify.cc
tests/various/const_arg_loop.sv [new file with mode: 0644]
tests/various/const_arg_loop.v [deleted file]
tests/various/const_arg_loop.ys
tests/various/const_func.sv [new file with mode: 0644]
tests/various/const_func.v [deleted file]
tests/various/const_func.ys
tests/verilog/func_arg_mismatch_1.ys [new file with mode: 0644]
tests/verilog/func_arg_mismatch_2.ys [new file with mode: 0644]
tests/verilog/func_arg_mismatch_3.ys [new file with mode: 0644]
tests/verilog/func_arg_mismatch_4.ys [new file with mode: 0644]

index d8818df31b060974a6201973200d205fa90b6fbf..b793c6c2d047655e3ccd2bbe33d5a69b49472928 100644 (file)
@@ -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);
index 402b7257b612db1b35496988ddf25b21417a1767..df7c9282ceb8946903cd86405e5ccce606809ee0 100644 (file)
@@ -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 (file)
index 0000000..f28d06e
--- /dev/null
@@ -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 (file)
index 358fb43..0000000
+++ /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
index b039bda10534638e1078a437564e4fe6db3d7820..392532213b47ec539f5afda9cd73f9fc3fc7e662 100644 (file)
@@ -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 (file)
index 0000000..af65f5c
--- /dev/null
@@ -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 (file)
index 541e63b..0000000
+++ /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
index 5e3c0410522c6fbac59e24fa12d6942b81ac9ac0..2f60acfe6706ecd54de7f032fa240594218eedb4 100644 (file)
@@ -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 (file)
index 0000000..a0e82db
--- /dev/null
@@ -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 (file)
index 0000000..c2c29c1
--- /dev/null
@@ -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 (file)
index 0000000..892824c
--- /dev/null
@@ -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 (file)
index 0000000..87ec1c2
--- /dev/null
@@ -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