From 40f4411b699b3cda40ad24e2ac69f06b1801e17b Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Thu, 26 Apr 2012 21:11:48 -0400 Subject: [PATCH] Fix tristate connection to unconnected input, bug494, bug495. --- src/V3Inline.cpp | 2 +- src/V3Inst.cpp | 6 +- src/V3Inst.h | 3 +- src/V3Link.cpp | 2 +- src/V3Tristate.cpp | 62 +++++++++--- test_regress/t/t_tri_pin0_bad.v | 23 ----- .../t/{t_tri_pin0_bad.pl => t_tri_unconn.pl} | 10 +- test_regress/t/t_tri_unconn.v | 99 +++++++++++++++++++ 8 files changed, 161 insertions(+), 46 deletions(-) delete mode 100644 test_regress/t/t_tri_pin0_bad.v rename test_regress/t/{t_tri_pin0_bad.pl => t_tri_unconn.pl} (64%) create mode 100644 test_regress/t/t_tri_unconn.v diff --git a/src/V3Inline.cpp b/src/V3Inline.cpp index 7a5351179..bf25ce902 100644 --- a/src/V3Inline.cpp +++ b/src/V3Inline.cpp @@ -115,7 +115,7 @@ private: // this loop as it clone()s itself. for (AstPin* pinp = nodep->pinsp(); pinp; pinp=pinp->nextp()->castPin()) { if (!pinp->exprp()) continue; - V3Inst::pinReconnectSimple(pinp, nodep, m_modp); + V3Inst::pinReconnectSimple(pinp, nodep, m_modp, false); } // Clone original module diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index 9b3c33d88..ef98f2baf 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -87,7 +87,7 @@ private: // Use user1p on the PIN to indicate we created an assign for this pin if (!nodep->user1Inc()) { // Simplify it - V3Inst::pinReconnectSimple(nodep, m_cellp, m_modp); + V3Inst::pinReconnectSimple(nodep, m_cellp, m_modp, false); // Make a ASSIGNW (expr, pin) AstNode* exprp = nodep->exprp()->cloneTree(false); if (nodep->width() != nodep->modVarp()->width()) @@ -237,7 +237,7 @@ public: //###################################################################### // Inst class functions -AstAssignW* V3Inst::pinReconnectSimple(AstPin* pinp, AstCell* cellp, AstNodeModule* modp) { +AstAssignW* V3Inst::pinReconnectSimple(AstPin* pinp, AstCell* cellp, AstNodeModule* modp, bool forTristate) { // If a pin connection is "simple" leave it as-is // Else create a intermediate wire to perform the interconnect // Return the new assignment, if one was made @@ -261,7 +261,7 @@ AstAssignW* V3Inst::pinReconnectSimple(AstPin* pinp, AstCell* cellp, AstNodeModu && pinp->width() == pinVarp->width() && 1) { // Done. One to one interconnect won't need a temporary variable. - } else if (pinp->exprp()->castConst()) { + } else if (!forTristate && pinp->exprp()->castConst()) { // Done. Constant. } else { // Make a new temp wire diff --git a/src/V3Inst.h b/src/V3Inst.h index 222883a92..710c76057 100644 --- a/src/V3Inst.h +++ b/src/V3Inst.h @@ -33,7 +33,8 @@ class V3Inst { public: static void instAll(AstNetlist* nodep); static void dearrayAll(AstNetlist* nodep); - static AstAssignW* pinReconnectSimple(AstPin* nodep, AstCell* cellp, AstNodeModule* modp); + static AstAssignW* pinReconnectSimple(AstPin* nodep, AstCell* cellp, AstNodeModule* modp, + bool forTristate); }; #endif // Guard diff --git a/src/V3Link.cpp b/src/V3Link.cpp index 608cfa419..73b47c57b 100644 --- a/src/V3Link.cpp +++ b/src/V3Link.cpp @@ -634,7 +634,7 @@ private: virtual void visit(AstPin* nodep, AstNUser*) { // Pin: Link to submodule's port - // ONLY CALLED by AstCell during ID_RESOLVE and ID_PARAM state + // ONLY CALLED by visit(AstCell) during ID_RESOLVE and ID_PARAM state if (m_idState==ID_RESOLVE && !nodep->modVarp()) { if (!m_cellVarsp) nodep->v3fatalSrc("Pin not under cell?\n"); AstVar* refp = m_cellVarsp->findIdFlat(nodep->name())->castVar(); diff --git a/src/V3Tristate.cpp b/src/V3Tristate.cpp index 594014581..7caa3efea 100644 --- a/src/V3Tristate.cpp +++ b/src/V3Tristate.cpp @@ -160,6 +160,16 @@ class TristateVisitor : public TristateBaseVisitor { return invarp->user4p()->castNode()->castVar(); } + AstVar* getCreateUnconnVarp(AstNode* fromp) { + AstVar* newp = new AstVar(fromp->fileline(), + AstVarType::MODULETEMP, + "__Vtriunconn"+cvtToStr(m_unique++), + VFlagLogicPacked(), fromp->width()); + if (!m_modp) { newp->v3error("Unsupported: Creating tristate signal not underneath a module"); } + else m_modp->addStmtp(newp); + return newp; + } + AstNode* newEnableDeposit(AstSel* selp, AstNode* enp) { // Form a "deposit" instruction for given enable, using existing select as a template. // Would be nicer if we made this a new AST type @@ -196,6 +206,17 @@ class TristateVisitor : public TristateBaseVisitor { virtual void visit(AstConst* nodep, AstNUser*) { UINFO(9,(m_alhs?"alhs":"")<<" "<user1p()) { + // A pin with 1'b0 or similar connection results in an assign with constant on LHS + // due to the pinReconnectSimple call in visit AstPin. + // We can ignore the output override by making a temporary + AstVar* varp = getCreateUnconnVarp(nodep); + AstNode* newp = new AstVarRef(nodep->fileline(), varp, true); + UINFO(9," const->"<replaceWith(newp); + pushDeletep(nodep); nodep = NULL; + return; + } if (nodep->num().hasZ()) { FileLine* fl = nodep->fileline(); V3Number numz (fl,nodep->width()); numz.opBitsZ(nodep->num()); //Z->1, else 0 @@ -388,7 +409,7 @@ class TristateVisitor : public TristateBaseVisitor { nodep->rhsp()->user1p(NULL); UINFO(9," enp<-rhs "<lhsp()->user1p()<lhsp()->iterateAndNext(*this); m_alhs = false; } @@ -467,15 +488,25 @@ class TristateVisitor : public TristateBaseVisitor { } } + // .tri(SEL(trisig,x)) becomes + // INPUT: -> (VARREF(trisig__pinin)), + // trisig__pinin = SEL(trisig,x) // via pinReconnectSimple + // OUTPUT: -> (VARREF(trisig__pinout)) + // ENABLE: -> (VARREF(trisig__pinen) + // SEL(trisig,x) = BUFIF1(enable__temp, trisig__pinen) + // Added complication is the signal may be an output/inout or just input with tie off (or not) up top + // PIN PORT NEW PORTS AND CONNECTIONS + // N/C input in(from-resolver), __out(to-resolver-only), __en(to-resolver-only) + // N/C inout Spec says illegal + // N/C output Unsupported; Illegal? + // wire input in(from-resolver-with-wire-value), __out(to-resolver-only), __en(to-resolver-only) + // wire inout in, __out, __en + // wire output in, __out, __en + // const input in(from-resolver-with-const-value), __out(to-resolver-only), __en(to-resolver-only) + // const inout Spec says illegal + // const output Unsupported; Illegal? virtual void visit(AstPin* nodep, AstNUser*) { - // .tri(SEL(trisig,x)) becomes - // INPUT: -> (VARREF(trisig__pinin)), - // trisig__pinin = SEL(trisig,x) // via pinReconnectSimple - // OUTPUT: -> (VARREF(trisig__pinout)) - // ENABLE: -> (VARREF(trisig__pinen) - // SEL(trisig,x) = BUFIF1(enable__temp, trisig__pinen) UINFO(9," "<exprp()) return; // No-connect AstVar* enModVarp = (AstVar*) nodep->modVarp()->user1p(); if (!enModVarp) { // no __en signals on this pin nodep->iterateChildren(*this); @@ -488,7 +519,15 @@ class TristateVisitor : public TristateBaseVisitor { if (debug()>=9) nodep->dumpTree(cout,"-pin-pre: "); // pinReconnectSimple needs to presume input or output behavior; we need both - // Therefore, create the BUFIF1 on output and separate input pin, then pinReconnectSimple both + // Therefore, create the enable, output and separate input pin, then pinReconnectSimple all + + if (!nodep->exprp()) { // No-connect; covert to empty connection + UINFO(5,"Unconnected pin terminate "<exprp(new AstVarRef(nodep->fileline(), ucVarp, + nodep->modVarp()->isOutput())); + // We don't need a driver on the wire; the lack of one will default to tristate + } // Create the output enable pin, connect to new signal AstNode* enrefp; @@ -524,14 +563,15 @@ class TristateVisitor : public TristateBaseVisitor { outpinp->user2(true); // mark this visited m_cellp->addPinsp(outpinp); // Simplify - outAssignp = V3Inst::pinReconnectSimple(outpinp, m_cellp, m_modp); // Note may change outpinp->exprp() + if (debug()>=9) outpinp->dumpTree(cout,"-pin-opr: "); + outAssignp = V3Inst::pinReconnectSimple(outpinp, m_cellp, m_modp, true); // Note may change outpinp->exprp() if (debug()>=9) outpinp->dumpTree(cout,"-pin-out: "); if (debug()>=9 && outAssignp) outAssignp->dumpTree(cout,"-pin-out: "); } // Existing pin becomes an input TristateInPinVisitor visitor (nodep->exprp()); - V3Inst::pinReconnectSimple(nodep, m_cellp, m_modp); // Note may change nodep->exprp() + V3Inst::pinReconnectSimple(nodep, m_cellp, m_modp, true); // Note may change nodep->exprp() if (debug()>=9) nodep->dumpTree(cout,"-pin-in: "); // Connect enable to output signal diff --git a/test_regress/t/t_tri_pin0_bad.v b/test_regress/t/t_tri_pin0_bad.v deleted file mode 100644 index b162133ba..000000000 --- a/test_regress/t/t_tri_pin0_bad.v +++ /dev/null @@ -1,23 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed into the Public Domain, for any use, -// without warranty, 2012 by Wilson Snyder. - -module t (/*AUTOARG*/ - // Inputs - clk - ); - input clk; - - t_tri4 t_tri4 (.t4(1'b0)); - -endmodule - -module t_tri4 (/*AUTOARG*/ - // Inputs - t4 - ); - input t4; - tri0 t4; - initial if (t4 !== 1'b0) $stop; -endmodule diff --git a/test_regress/t/t_tri_pin0_bad.pl b/test_regress/t/t_tri_unconn.pl similarity index 64% rename from test_regress/t/t_tri_pin0_bad.pl rename to test_regress/t/t_tri_unconn.pl index 26989db72..f91289753 100755 --- a/test_regress/t/t_tri_pin0_bad.pl +++ b/test_regress/t/t_tri_unconn.pl @@ -8,13 +8,11 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Version 2.0. compile ( - fails=>$Self->{v3}, - expect=> -qr{%Error: t/t_tri_pin0_bad.v:\d+: Unsupported tristate port expression: CONST '1'h0' -%Error: t/t_tri_pin0_bad.v:\d+: Output port is connected to a constant pin, electrical short -%Error: Exiting due to}, + ); + +execute ( + check_finished=>1, ); ok(1); 1; - diff --git a/test_regress/t/t_tri_unconn.v b/test_regress/t/t_tri_unconn.v new file mode 100644 index 000000000..074a29462 --- /dev/null +++ b/test_regress/t/t_tri_unconn.v @@ -0,0 +1,99 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2012 by Wilson Snyder. + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + integer cyc=0; + + wire one = '1; + wire z0 = 'z; + wire z1 = 'z; + wire z2 = 'z; + wire z3 = 'z; + + // verilator lint_off PINMISSING + t_tri0 tri0a (); // Error/warning + t_tri0 tri0b (.tn()); + t_tri0 tri0z (.tn(z0)); + t_tri0 #(.EXPECT(1'b0)) tri0c (.tn(1'b0)); + t_tri0 #(.EXPECT(1'b1)) tri0d (.tn(1'b1)); // Warning would be reasonable given tri0 connect + t_tri0 #(.EXPECT(1'b0)) tri0e (.tn(~one)); + t_tri0 #(.EXPECT(1'b1)) tri0f (.tn(one)); + + t_tri1 tri1a (); + t_tri1 tri1b (.tn()); + t_tri1 tri1z (.tn(z1)); + t_tri1 #(.EXPECT(1'b0)) tri1c (.tn(1'b0)); // Warning would be reasonable given tri1 connect + t_tri1 #(.EXPECT(1'b1)) tri1d (.tn(1'b1)); + t_tri1 #(.EXPECT(1'b0)) tri1e (.tn(~one)); + t_tri1 #(.EXPECT(1'b1)) tri1f (.tn(one)); + + t_tri2 tri2a (); + t_tri2 tri2b (.tn()); + t_tri2 tri2z (.tn(z2)); + t_tri2 #(.EXPECT(1'b0)) tri2c (.tn(1'b0)); + t_tri2 #(.EXPECT(1'b1)) tri2d (.tn(1'b1)); + t_tri2 #(.EXPECT(1'b0)) tri2e (.tn(~one)); + t_tri2 #(.EXPECT(1'b1)) tri2f (.tn(one)); + + t_tri3 tri3a (); + t_tri3 tri3b (.tn()); + t_tri3 tri3z (.tn(z3)); + t_tri3 #(.EXPECT(1'b0)) tri3c (.tn(1'b0)); + t_tri3 #(.EXPECT(1'b1)) tri3d (.tn(1'b1)); + t_tri3 #(.EXPECT(1'b0)) tri3e (.tn(~one)); + t_tri3 #(.EXPECT(1'b1)) tri3f (.tn(one)); + // verilator lint_on PINMISSING + + // Test loop + always @ (posedge clk) begin + cyc <= cyc + 1; + if (cyc==99) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule + +module t_tri0 + #(parameter EXPECT=1'b0) + (tn); + input tn; // Illegal to be inout; spec requires net connection to any inout + tri0 tn; + wire clk = t.clk; + always @(posedge clk) if (tn !== EXPECT) $stop; +endmodule + +module t_tri1 + #(parameter EXPECT=1'b1) + (tn); + input tn; + tri1 tn; + wire clk = t.clk; + always @(posedge clk) if (tn !== EXPECT) $stop; +endmodule + +module t_tri2 + #(parameter EXPECT=1'b0) + (tn); + input tn; + pulldown(tn); + wire clk = t.clk; + always @(posedge clk) if (tn !== EXPECT) $stop; +endmodule + +module t_tri3 + #(parameter EXPECT=1'b1) + (tn); + input tn; + pullup(tn); + wire clk = t.clk; + always @(posedge clk) if (tn !== EXPECT) $stop; +endmodule