From b6af90fe20bc92631061c48c328f3c6e4760e4df Mon Sep 17 00:00:00 2001 From: Zachary Snow Date: Sun, 21 Feb 2021 14:45:21 -0500 Subject: [PATCH] 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 --- frontends/ast/ast.h | 8 +++- frontends/ast/simplify.cc | 32 +++++++++++---- .../{const_arg_loop.v => const_arg_loop.sv} | 27 ++++++------ tests/various/const_arg_loop.ys | 7 +++- tests/various/{const_func.v => const_func.sv} | 41 +++++++++---------- tests/various/const_func.ys | 8 +++- tests/verilog/func_arg_mismatch_1.ys | 12 ++++++ tests/verilog/func_arg_mismatch_2.ys | 12 ++++++ tests/verilog/func_arg_mismatch_3.ys | 12 ++++++ tests/verilog/func_arg_mismatch_4.ys | 12 ++++++ 10 files changed, 124 insertions(+), 47 deletions(-) rename tests/various/{const_arg_loop.v => const_arg_loop.sv} (78%) rename tests/various/{const_func.v => const_func.sv} (74%) create mode 100644 tests/verilog/func_arg_mismatch_1.ys create mode 100644 tests/verilog/func_arg_mismatch_2.ys create mode 100644 tests/verilog/func_arg_mismatch_3.ys create mode 100644 tests/verilog/func_arg_mismatch_4.ys 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 &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.v b/tests/various/const_arg_loop.sv similarity index 78% rename from tests/various/const_arg_loop.v rename to tests/various/const_arg_loop.sv index 358fb439a..f28d06e68 100644 --- a/tests/various/const_arg_loop.v +++ b/tests/various/const_arg_loop.sv @@ -20,11 +20,11 @@ module top; endfunction function automatic [31:0] operation2; - input [4:0] var; + input [4:0] inp; input integer num; begin - var[0] = var[0] ^ 1; - operation2 = num * var; + inp[0] = inp[0] ^ 1; + operation2 = num * inp; end endfunction @@ -79,15 +79,14 @@ module top; 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 + 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.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.v b/tests/various/const_func.sv similarity index 74% rename from tests/various/const_func.v rename to tests/various/const_func.sv index 541e63b19..af65f5c73 100644 --- a/tests/various/const_func.v +++ b/tests/various/const_func.sv @@ -62,26 +62,25 @@ module top(out); 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); + 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 property (Y == 3); - assert property (Z == ~3); -`endif + assert(Y == 3); + assert(Z == ~3); + end 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 <