From 940dc98c667e19996baf094d5bca718448320a5d Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Sat, 1 Dec 2018 10:12:10 -0500 Subject: [PATCH] Add CONTASSREG error on continuous assignments to regs, bug1369. --- Changes | 2 ++ bin/verilator | 12 +++++++++++ src/V3Ast.h | 7 +++++++ src/V3Error.h | 8 +++++--- src/V3Undriven.cpp | 28 +++++++++++++++++++++----- test_regress/t/t_langext_1.v | 33 +++++++++++++++---------------- test_regress/t/t_wire_beh_bad.out | 3 ++- test_regress/t/t_wire_beh_bad.pl | 2 +- test_regress/t/t_wire_beh_bad.v | 2 +- 9 files changed, 69 insertions(+), 28 deletions(-) diff --git a/Changes b/Changes index 99d0f6d00..36c9b5073 100644 --- a/Changes +++ b/Changes @@ -10,6 +10,8 @@ The contributors that suggested a given feature are shown in []. Thanks! *** In --xml-only show the original unmodified names, and add module_files and cells similar to Verilog-Perl, msg2719. [Kanad Kanhere] +**** Add CONTASSREG error on continuous assignments to regs, bug1369. [Peter Gerst] + **** Add PROCASSWIRE error on behavioral assignments to wires, msg2737. [Neil Turton] **** Add IMPORTSTAR warning on import::* inside $unit scope. diff --git a/bin/verilator b/bin/verilator index c8cfc041b..6f63af247 100755 --- a/bin/verilator +++ b/bin/verilator @@ -3500,6 +3500,18 @@ L Ignoring this warning may make Verilator simulations differ from other simulators. +=item CONTASSREG + +Error that a continuous assignment is setting a reg. According to IEEE +Verilog, but not SystemVerilog, a wire must be used as the target of +continuous assignments. + +This error is only reported when "--language 1364-1995", "--language +1364-2001", or "--language 1364-2005" is used. + +Ignoring this error will only suppress the lint check, it will simulate +correctly. + =item DECLFILENAME Warns that a module or other declaration's name doesn't match the filename diff --git a/src/V3Ast.h b/src/V3Ast.h index 39355d0e5..efe1d4edf 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -555,6 +555,13 @@ public: || m_e==SUPPLY0 || m_e==SUPPLY1 || m_e==VAR); } + bool isContAssignable() const { // In Verilog, always ok in SystemVerilog + return (m_e==SUPPLY0 || m_e==SUPPLY1 + || m_e==WIRE || m_e==WREAL || m_e==IMPLICITWIRE + || m_e==TRIWIRE || m_e==TRI0 || m_e==TRI1 || m_e==PORT + || m_e==BLOCKTEMP || m_e==MODULETEMP || m_e==STMTTEMP + || m_e==XTEMP || m_e==IFACEREF); + } bool isProcAssignable() const { return (m_e==GPARAM || m_e==LPARAM || m_e==GENVAR || m_e==VAR diff --git a/src/V3Error.h b/src/V3Error.h index c79c5e3f0..6bc741cb1 100644 --- a/src/V3Error.h +++ b/src/V3Error.h @@ -71,6 +71,7 @@ public: CMPCONST, // Comparison is constant due to limited range COLONPLUS, // :+ instead of +: COMBDLY, // Combinatorial delayed assignment + CONTASSREG, // Continuous assignment on reg DEFPARAM, // Style: Defparam DECLFILENAME, // Declaration doesn't match filename ENDLABEL, // End lable name mismatch @@ -133,8 +134,8 @@ public: " EC_FIRST_WARN", "ALWCOMBORDER", "ASSIGNDLY", "ASSIGNIN", "BLKANDNBLK", "BLKLOOPINIT", "BLKSEQ", "BSSPACE", - "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CDCRSTLOGIC", "CLKDATA", - "CMPCONST", "COLONPLUS", "COMBDLY", + "CASEINCOMPLETE", "CASEOVERLAP", "CASEWITHX", "CASEX", "CDCRSTLOGIC", "CLKDATA", + "CMPCONST", "COLONPLUS", "COMBDLY", "CONTASSREG", "DEFPARAM", "DECLFILENAME", "ENDLABEL", "GENCLK", "IFDEPTH", "IMPERFECTSCH", "IMPLICIT", "IMPORTSTAR", "IMPURE", @@ -160,7 +161,8 @@ public: // Warnings we'll present to the user as errors // Later -Werror- options may make more of these. bool pretendError() const { return ( m_e==ASSIGNIN || m_e==BLKANDNBLK - || m_e==BLKLOOPINIT + || m_e==BLKLOOPINIT + || m_e==CONTASSREG || m_e==IMPURE || m_e==PROCASSWIRE); } // Warnings to mention manual diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp index f6bdb7d5f..5da146b68 100644 --- a/src/V3Undriven.cpp +++ b/src/V3Undriven.cpp @@ -233,6 +233,7 @@ private: // STATE std::vector m_entryps[3]; // Nodes to delete when we are finished bool m_inBBox; // In black box; mark as driven+used + bool m_inContAssign; // In continuous assignment bool m_inProcAssign; // In procedual assignment AstNodeFTask* m_taskp; // Current task AstAlways* m_alwaysCombp; // Current always if combo, otherwise NULL @@ -323,7 +324,14 @@ private: if (nodep->lvalue() && !VN_IS(nodep, VarXRef)) { // Ignore interface variables and similar ugly items if (m_inProcAssign && !nodep->varp()->varType().isProcAssignable()) { - nodep->v3warn(PROCASSWIRE, "Procedural assignment to wire, perhaps intend var (IEEE 2017 6.5): " + nodep->v3warn(PROCASSWIRE, "Procedural assignment to wire, perhaps intended var" + " (IEEE 2017 6.5): " + +nodep->prettyName()); + } + if (m_inContAssign && !nodep->varp()->varType().isContAssignable() + && !nodep->fileline()->language().systemVerilog()) { + nodep->v3warn(CONTASSREG, "Continuous assignment to reg, perhaps intended wire" + " (IEEE 2005 6.1; Verilog only, legal in SV): " +nodep->prettyName()); } } @@ -350,22 +358,31 @@ private: } virtual void visit(AstAssign* nodep) { - bool prevBeh = m_inProcAssign; + bool prevProc = m_inProcAssign; { m_inProcAssign = true; iterateChildren(nodep); m_inProcAssign = false; } - m_inProcAssign = prevBeh; + m_inProcAssign = prevProc; } virtual void visit(AstAssignDly* nodep) { - bool prevBeh = m_inProcAssign; + bool prevProc = m_inProcAssign; { m_inProcAssign = true; iterateChildren(nodep); m_inProcAssign = false; } - m_inProcAssign = prevBeh; + m_inProcAssign = prevProc; + } + virtual void visit(AstAssignW* nodep) { + bool prevCont = m_inProcAssign; + { + m_inContAssign = true; + iterateChildren(nodep); + m_inContAssign = false; + } + m_inProcAssign = prevCont; } virtual void visit(AstAlways* nodep) { AstAlways* prevAlwp = m_alwaysCombp; @@ -406,6 +423,7 @@ public: // CONSTUCTORS explicit UndrivenVisitor(AstNetlist* nodep) { m_inBBox = false; + m_inContAssign = false; m_inProcAssign = false; m_taskp = NULL; m_alwaysCombp = NULL; diff --git a/test_regress/t/t_langext_1.v b/test_regress/t/t_langext_1.v index db02848ed..bd8728f91 100644 --- a/test_regress/t/t_langext_1.v +++ b/test_regress/t/t_langext_1.v @@ -17,35 +17,34 @@ module t (/*AUTOARG*/ ); input clk; - reg [1:0] res; + wire [1:0] res; // Instantiate the test - test test_i (/*AUTOINST*/ - // Outputs - .res (res), - // Inputs - .clk (clk), - .in (1'b1)); + test test_i (// Outputs + .res (res[1:0]), + // Inputs + .clk (clk), + .in (1'b1)); endmodule module test (// Outputs - res, - // Inputs - clk, - in + res, + // Inputs + clk, + in ); - output [1:0] res; - input clk; - input in; + output reg [1:0] res; + input clk; + input in; // This is a Verilog 2001 test generate genvar i; for (i=0; i<2; i=i+1) begin - always @(posedge clk) begin - res[i:i] <= in; - end + always @(posedge clk) begin + res[i:i] <= in; + end end endgenerate endmodule diff --git a/test_regress/t/t_wire_beh_bad.out b/test_regress/t/t_wire_beh_bad.out index 95acb9018..3e8f79329 100644 --- a/test_regress/t/t_wire_beh_bad.out +++ b/test_regress/t/t_wire_beh_bad.out @@ -1,2 +1,3 @@ -%Error-PROCASSWIRE: t/t_wire_beh_bad.v:12: Procedural assignment to wire, perhaps intend var (IEEE 2017 6.5): w +%Error-CONTASSREG: t/t_wire_beh_bad.v:11: Continuous assignment to reg, perhaps intended wire (IEEE 2005 6.1; Verilog only, legal in SV): r +%Error-PROCASSWIRE: t/t_wire_beh_bad.v:12: Procedural assignment to wire, perhaps intended var (IEEE 2017 6.5): w %Error: Exiting due to diff --git a/test_regress/t/t_wire_beh_bad.pl b/test_regress/t/t_wire_beh_bad.pl index 3ce8c1ef2..0f424353d 100755 --- a/test_regress/t/t_wire_beh_bad.pl +++ b/test_regress/t/t_wire_beh_bad.pl @@ -10,7 +10,7 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di scenarios(vlt => 1); compile( - v_flags2 => ["--lint-only"], + v_flags2 => ["--lint-only --language 1364-2001"], fails => 1, expect_filename => $Self->{golden_filename}, ); diff --git a/test_regress/t/t_wire_beh_bad.v b/test_regress/t/t_wire_beh_bad.v index 42696fa9b..b6478afe5 100644 --- a/test_regress/t/t_wire_beh_bad.v +++ b/test_regress/t/t_wire_beh_bad.v @@ -9,6 +9,6 @@ module t (/*AUTOARG*/); reg r; assign r = 1'b1; - always_comb w = 1'b0; + always @ (r) w = 1'b0; endmodule