This PR should be the base for discussion, do not merge it yet!

It correctly detects reg/wire mix and incorrect use on blocking,nonblocking assignments within blocks and assign statements.

What it DOES'T do:
Detect registers connected to output ports of instances.

Where it FAILS:
memorty nonblocking assignments causes spurious (I assume??) errors on yosys-generated "_ADDR", "_DATA", "EN" signals.

You can test it with tests/simple/reg_wire_error.v (look inside for the comments to enable/disable specific lines)

diff --git a/frontends/ast/ast.cc b/frontends/ast/ast.cc
index 037a9f3eec6ed17c1a60c5346e20163ded43b345..25267775a1e5589097dc2e853841db2e69e96489 100644 (file)
--- a/frontends/ast/ast.cc
+++ b/frontends/ast/ast.cc
@@ -191,6 +191,7 @@ AstNode::AstNode(AstNodeType type, AstNode *child1, AstNode *child2, AstNode *ch
        is_input = false;
        is_output = false;
        is_reg = false;
+       is_logic = false;
        is_signed = false;
        is_string = false;
        range_valid = false;
@@ -285,7 +286,9 @@ void AstNode::dumpAst(FILE *f, std::string indent) const
                fprintf(f, " input");
        if (is_output)
                fprintf(f, " output");
-       if (is_reg)
+       if (is_logic)
+               fprintf(f, " logic");
+       if (is_reg) // this is an AST dump, not Verilog - if we see "logic reg" that's fine.
                fprintf(f, " reg");
        if (is_signed)
                fprintf(f, " signed");
@@ -652,6 +655,8 @@ bool AstNode::operator==(const AstNode &other) const
                return false;
        if (is_output != other.is_output)
                return false;
+       if (is_logic != other.is_logic)
+               return false;
        if (is_reg != other.is_reg)
                return false;
        if (is_signed != other.is_signed)

diff --git a/frontends/ast/ast.h b/frontends/ast/ast.h
index d1e2c78d1f306a60102a89b329dce577ef80985d..9b71249346582aa89be418ea0697d55254c80dbc 100644 (file)
--- a/frontends/ast/ast.h
+++ b/frontends/ast/ast.h
@@ -168,7 +168,7 @@ namespace AST
                // node content - most of it is unused in most node types
                std::string str;
                std::vector<RTLIL::State> bits;
-               bool is_input, is_output, is_reg, is_signed, is_string, range_valid, range_swapped;
+               bool is_input, is_output, is_reg, is_logic, is_signed, is_string, range_valid, range_swapped;
                int port_id, range_left, range_right;
                uint32_t integer;
                double realvalue;

diff --git a/frontends/ast/simplify.cc b/frontends/ast/simplify.cc
index a16fdfeebc81a61de318d1a4d50dc8cd4db3655f..c9c5e5263a27d796d456795f757e2280b4b6d2ff 100644 (file)
--- a/frontends/ast/simplify.cc
+++ b/frontends/ast/simplify.cc
@@ -327,6 +327,8 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
                        if (node->type == AST_WIRE) {
                                if (this_wire_scope.count(node->str) > 0) {
                                        AstNode *first_node = this_wire_scope[node->str];
+                                       if (first_node->is_input && node->is_reg)
+                                               goto wires_are_incompatible;
                                        if (!node->is_input && !node->is_output && node->is_reg && node->children.size() == 0)
                                                goto wires_are_compatible;
                                        if (first_node->children.size() == 0 && node->children.size() == 1 && node->children[0]->type == AST_RANGE) {
@@ -361,6 +363,8 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
                                                first_node->is_output = true;
                                        if (node->is_reg)
                                                first_node->is_reg = true;
+                                       if (node->is_logic)
+                                               first_node->is_logic = true;
                                        if (node->is_signed)
                                                first_node->is_signed = true;
                                        for (auto &it : node->attributes) {
@@ -440,6 +444,12 @@ bool AstNode::simplify(bool const_fold, bool at_zero, bool in_lvalue, int stage,
                children[1]->detectSignWidth(width_hint, sign_hint);
                width_hint = max(width_hint, backup_width_hint);
                child_0_is_self_determined = true;
+               if ((type == AST_ASSIGN_LE || type == AST_ASSIGN_EQ) && children[0]->id2ast->is_logic)
+                       children[0]->id2ast->is_reg = true; // if logic type is used in a block asignment
+               if ((type == AST_ASSIGN_LE || type == AST_ASSIGN_EQ) && !children[0]->id2ast->is_reg)
+                       log_warning("wire '%s' is assigned in a block at %s:%d.\n", children[0]->str.c_str(), filename.c_str(), linenum);
+               if (type == AST_ASSIGN && children[0]->id2ast->is_reg)
+                       log_error("reg '%s' is assigned in a continuous assignment at %s:%d.\n", children[0]->str.c_str(), filename.c_str(), linenum);
                break;
 
        case AST_PARAMETER:

diff --git a/frontends/verilog/verilog_lexer.l b/frontends/verilog/verilog_lexer.l
index d6d00c371be0b8d749a136afa37672a64bf8e20e..8aa123e1ed922808c67e6f9872f46b8e0430998d 100644 (file)
--- a/frontends/verilog/verilog_lexer.l
+++ b/frontends/verilog/verilog_lexer.l
@@ -189,7 +189,7 @@ YOSYS_NAMESPACE_END
 "const"      { if (formal_mode) return TOK_CONST; SV_KEYWORD(TOK_CONST); }
 "checker"    { if (formal_mode) return TOK_CHECKER; SV_KEYWORD(TOK_CHECKER); }
 "endchecker" { if (formal_mode) return TOK_ENDCHECKER; SV_KEYWORD(TOK_ENDCHECKER); }
-"logic"      { SV_KEYWORD(TOK_REG); }
+"logic"      { SV_KEYWORD(TOK_LOGIC); }
 "bit"        { SV_KEYWORD(TOK_REG); }
 
 "eventually"   { if (formal_mode) return TOK_EVENTUALLY; SV_KEYWORD(TOK_EVENTUALLY); }

diff --git a/frontends/verilog/verilog_parser.y b/frontends/verilog/verilog_parser.y
index ba2fc036e638071a0257e911aa192f7ca3023942..722febf130841a27319acd482494aa0b047c4c6c 100644 (file)
--- a/frontends/verilog/verilog_parser.y
+++ b/frontends/verilog/verilog_parser.y
@@ -105,7 +105,7 @@ static void free_attr(std::map<std::string, AstNode*> *al)
 %token ATTR_BEGIN ATTR_END DEFATTR_BEGIN DEFATTR_END
 %token TOK_MODULE TOK_ENDMODULE TOK_PARAMETER TOK_LOCALPARAM TOK_DEFPARAM
 %token TOK_PACKAGE TOK_ENDPACKAGE TOK_PACKAGESEP
-%token TOK_INPUT TOK_OUTPUT TOK_INOUT TOK_WIRE TOK_REG
+%token TOK_INPUT TOK_OUTPUT TOK_INOUT TOK_WIRE TOK_REG TOK_LOGIC
 %token TOK_INTEGER TOK_SIGNED TOK_ASSIGN TOK_ALWAYS TOK_INITIAL
 %token TOK_BEGIN TOK_END TOK_IF TOK_ELSE TOK_FOR TOK_WHILE TOK_REPEAT
 %token TOK_DPI_FUNCTION TOK_POSEDGE TOK_NEGEDGE TOK_OR TOK_AUTOMATIC
@@ -394,6 +394,9 @@ wire_type_token:
        TOK_REG {
                astbuf3->is_reg = true;
        } |
+       TOK_LOGIC {
+               astbuf3->is_logic = true;
+       } |
        TOK_INTEGER {
                astbuf3->is_reg = true;
                astbuf3->range_left = 31;
@@ -827,6 +830,7 @@ wire_name:
                                node->port_id = current_function_or_task_port_id++;
                }
                ast_stack.back()->children.push_back(node);
+
                delete $1;
        };
 

diff --git a/tests/simple/reg_wire_error.v b/tests/simple/reg_wire_error.v
new file mode 100644 (file)
index 0000000..ab461b9
--- /dev/null
+++ b/tests/simple/reg_wire_error.v
@@ -0,0 +1,40 @@
+module sub_mod(input i_in, output o_out);
+assign o_out = i_in;
+endmodule
+
+module test(i_clk, i_reg, o_reg, o_wire);
+input i_clk;
+input i_reg;
+output o_reg;
+output o_wire;
+
+// Enable this to see how it doesn't fail on yosys although it should
+//reg o_wire;
+// Enable this instead of the above to see how logic can be mapped to a wire
+logic o_wire;
+// Enable this to see how it doesn't fail on yosys although it should
+//reg i_reg;
+// Disable this to see how it doesn't fail on yosys although it should
+reg o_reg;
+
+logic l_reg;
+
+// Enable this to tst if logic-turne-reg will catch assignments even if done before it turned into a reg
+//assign l_reg = !o_reg;
+initial o_reg = 1'b0;
+always @(posedge i_clk)
+begin
+  o_reg <= !o_reg;
+  l_reg <= !o_reg;
+end
+
+assign o_wire = !o_reg;
+// Uncomment this to see how a logic already turned intoa reg can be freely assigned on yosys
+//assign l_reg = !o_reg;
+
+sub_mod sm_inst (
+  .i_in(1'b1),
+  .o_out(o_reg)
+);
+endmodule
+