From 9d055f8c13b066952e8b960b3ff7c059d93f8011 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Tue, 12 Sep 2017 19:34:10 -0400 Subject: [PATCH] Fix ordering of arrayed cell wide connections, bug1202 partial. --- Changes | 2 + src/V3AstNodes.h | 17 +++--- src/V3Inst.cpp | 24 ++++---- test_regress/t/t_interface_array_nocolon.pl | 18 ++++++ test_regress/t/t_interface_array_nocolon.v | 64 +++++++++++++++++++++ 5 files changed, 106 insertions(+), 19 deletions(-) create mode 100755 test_regress/t/t_interface_array_nocolon.pl create mode 100644 test_regress/t/t_interface_array_nocolon.v diff --git a/Changes b/Changes index 27ba7325b..e8e3a36d2 100644 --- a/Changes +++ b/Changes @@ -4,6 +4,8 @@ The contributors that suggested a given feature are shown in []. Thanks! * Verilator 3.911 devel +*** Fix ordering of arrayed cell wide connections, bug1202 partial. [Mike Popoloski] + **** Fix enum ranges without colons, bug1204. [Mike Popoloski] diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 0f42a36c9..63cc55dd8 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -127,15 +127,18 @@ public: setOp2p(new AstConst(fl,range.hi())); setOp3p(new AstConst(fl,range.lo())); } ASTNODE_NODE_FUNCS(Range) - AstNode* msbp() const { return op2p(); } // op2 = Msb expression - AstNode* lsbp() const { return op3p(); } // op3 = Lsb expression + AstNode* msbp() const { return op2p(); } // op2 = Msb expression + AstNode* lsbp() const { return op3p(); } // op3 = Lsb expression AstNode* leftp() const { return littleEndian()?lsbp():msbp(); } // How to show a declaration AstNode* rightp() const { return littleEndian()?msbp():lsbp(); } - int msbConst() const { AstConst* constp=msbp()->castConst(); return (constp?constp->toSInt():0); } - int lsbConst() const { AstConst* constp=lsbp()->castConst(); return (constp?constp->toSInt():0); } - int elementsConst() const { return (msbConst()>lsbConst()) ? msbConst()-lsbConst()+1 : lsbConst()-msbConst()+1; } - bool littleEndian() const { return m_littleEndian; } - void littleEndian(bool flag) { m_littleEndian=flag; } + int msbConst() const { AstConst* constp=msbp()->castConst(); return (constp?constp->toSInt():0); } + int lsbConst() const { AstConst* constp=lsbp()->castConst(); return (constp?constp->toSInt():0); } + int elementsConst() const { return (msbConst()>lsbConst()) ? msbConst()-lsbConst()+1 : lsbConst()-msbConst()+1; } + int leftConst() const { AstConst* constp=leftp()->castConst(); return (constp?constp->toSInt():0); } + int rightConst() const { AstConst* constp=rightp()->castConst(); return (constp?constp->toSInt():0); } + int leftToRightInc() const { return littleEndian()?1:-1; } + bool littleEndian() const { return m_littleEndian; } + void littleEndian(bool flag) { m_littleEndian=flag; } virtual void dump(ostream& str); virtual string emitC() { V3ERROR_NA; return ""; } virtual V3Hash sameHash() const { return V3Hash(); } diff --git a/src/V3Inst.cpp b/src/V3Inst.cpp index 9551f841d..be2df4f7d 100644 --- a/src/V3Inst.cpp +++ b/src/V3Inst.cpp @@ -218,8 +218,7 @@ class InstDeVisitor : public AstNVisitor { private: // STATE AstRange* m_cellRangep; // Range for arrayed instantiations, NULL for normal instantiations - int m_instNum; // Current instantiation number - int m_instLsb; // Current instantiation number + int m_instSelNum; // Current instantiation count 0..N-1 InstDeModVarVisitor m_deModVars; // State of variables for current cell module typedef map VarNameMap; @@ -278,8 +277,10 @@ private: && ifaceVarp->dtypep()->castUnpackArrayDType()->subDTypep()->castIfaceRefDType(); // Make all of the required clones - m_instLsb = m_cellRangep->lsbConst(); - for (m_instNum = m_instLsb; m_instNum<=m_cellRangep->msbConst(); m_instNum++) { + for (int i = 0; i < m_cellRangep->elementsConst(); i++) { + m_instSelNum = m_cellRangep->littleEndian() ? (m_cellRangep->elementsConst() - 1 - i) : i; + int instNum = m_cellRangep->lsbConst() + i; + AstCell* newp = nodep->cloneTree(false); nodep->addNextHere(newp); // Remove ranging and fix name @@ -287,8 +288,8 @@ private: // Somewhat illogically, we need to rename the orignal name of the cell too. // as that is the name users expect for dotting // The spec says we add [x], but that won't work in C... - newp->name(newp->name()+"__BRA__"+cvtToStr(m_instNum)+"__KET__"); - newp->origName(newp->origName()+"__BRA__"+cvtToStr(m_instNum)+"__KET__"); + newp->name(newp->name()+"__BRA__"+cvtToStr(instNum)+"__KET__"); + newp->origName(newp->origName()+"__BRA__"+cvtToStr(instNum)+"__KET__"); UINFO(8," CELL loop "<addNextHere(ifaceRefp); ifaceRefp->cellp(newp); ifaceRefp->cellName(newp->name()); - varNewp->name(varNewp->name() + "__BRA__" + cvtToStr(m_instNum) + "__KET__"); - varNewp->origName(varNewp->origName() + "__BRA__" + cvtToStr(m_instNum) + "__KET__"); + varNewp->name(varNewp->name() + "__BRA__" + cvtToStr(instNum) + "__KET__"); + varNewp->origName(varNewp->origName() + "__BRA__" + cvtToStr(instNum) + "__KET__"); varNewp->dtypep(ifaceRefp); newp->addNextHere(varNewp); if (debug()==9) { varNewp->dumpTree(cout, "newintf: "); cout << endl; } @@ -343,7 +344,7 @@ private: // Connection to array, where array dimensions match the instant dimension AstNode* exprp = nodep->exprp()->unlinkFrBack(); exprp = new AstArraySel (exprp->fileline(), exprp, - (m_instNum-m_instLsb)); + m_instSelNum); nodep->exprp(exprp); } else if (expwidth == pinwidth) { // NOP: Arrayed instants: widths match so connect to each instance @@ -358,7 +359,7 @@ private: // Note spec allows more complicated matches such as slices and such } exprp = new AstSel (exprp->fileline(), exprp, - pinwidth*(m_instNum-m_instLsb), + pinwidth*m_instSelNum, pinwidth); nodep->exprp(exprp); } else { @@ -457,8 +458,7 @@ public: // CONSTUCTORS explicit InstDeVisitor(AstNetlist* nodep) { m_cellRangep=NULL; - m_instNum=0; - m_instLsb=0; + m_instSelNum=0; // nodep->accept(*this); } diff --git a/test_regress/t/t_interface_array_nocolon.pl b/test_regress/t/t_interface_array_nocolon.pl new file mode 100755 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_interface_array_nocolon.pl @@ -0,0 +1,18 @@ +#!/usr/bin/perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2003 by Wilson Snyder. This program is free software; you can +# redistribute it and/or modify it under the terms of either the GNU +# Lesser General Public License Version 3 or the Perl Artistic License +# Version 2.0. + +compile ( + ); + +execute ( + check_finished=>1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_array_nocolon.v b/test_regress/t/t_interface_array_nocolon.v new file mode 100644 index 000000000..db1289682 --- /dev/null +++ b/test_regress/t/t_interface_array_nocolon.v @@ -0,0 +1,64 @@ +// DESCRIPTION: Verilator: Functionally demonstrate an array of interfaces +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2017 by Mike Popoloski. + +interface foo_intf + ( + input x + ); +endinterface + +module foo_subm + ( + input x + ); +endmodule + +module t (); + + localparam N = 3; + + wire [2:0] X = 3'b110; + + // Should not cause LITENDIAN warning? + // verilator lint_off LITENDIAN + foo_intf foos [N] (.x(X)); + foo_intf fool [1:3] (.x(X)); + foo_intf foom [3:1] (.x(X)); + + foo_subm subs [N] (.x(X)); + foo_subm subl [1:3] (.x(X)); + foo_subm subm [3:1] (.x(X)); + + initial begin + // Check numbering with 0 first + // NC has a bug here + if (foos[0].x !== 1'b1) $stop; + if (foos[1].x !== 1'b1) $stop; + if (foos[2].x !== 1'b0) $stop; + // + if (fool[1].x !== 1'b1) $stop; + if (fool[2].x !== 1'b1) $stop; + if (fool[3].x !== 1'b0) $stop; + // + if (foom[1].x !== 1'b0) $stop; + if (foom[2].x !== 1'b1) $stop; + if (foom[3].x !== 1'b1) $stop; + // + if (subs[0].x !== 1'b1) $stop; + if (subs[1].x !== 1'b1) $stop; + if (subs[2].x !== 1'b0) $stop; + // + if (subl[1].x !== 1'b1) $stop; + if (subl[2].x !== 1'b1) $stop; + if (subl[3].x !== 1'b0) $stop; + // + if (subm[1].x !== 1'b0) $stop; + if (subm[2].x !== 1'b1) $stop; + if (subm[3].x !== 1'b1) $stop; + // + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule