From 8127a79cb1d1d391088375f6682ffebe22d82721 Mon Sep 17 00:00:00 2001 From: Wilson Snyder Date: Mon, 14 Jan 2013 21:49:22 -0500 Subject: [PATCH] Fix nested packed arrays and structs, bug600. IMPORTANT: Packed arrays are now represented as a single linear vector in Verilated models this may affect packed arrays that are public or accessed via the VPI. --- Changes | 7 +++ src/V3Ast.h | 4 +- src/V3AstNodes.cpp | 11 ++++- src/V3AstNodes.h | 15 +++++- src/V3Changed.cpp | 2 +- src/V3Const.cpp | 39 +++++---------- src/V3Coverage.cpp | 4 +- src/V3EmitC.cpp | 45 ++++++++--------- src/V3EmitCSyms.cpp | 13 ++--- src/V3LinkResolve.cpp | 2 +- src/V3Slice.cpp | 18 +++---- src/V3Table.cpp | 3 +- src/V3TraceDecl.cpp | 6 +-- src/V3Width.cpp | 48 ++++++++++--------- src/V3WidthSel.cpp | 41 ++++++++++++++-- test_regress/t/t_bitsel_struct.pl | 18 +++++++ test_regress/t/t_bitsel_struct.v | 32 +++++++++++++ test_regress/t/t_bitsel_struct2.pl | 18 +++++++ test_regress/t/t_bitsel_struct2.v | 36 ++++++++++++++ ...d_assign_bad.pl => t_mem_packed_assign.pl} | 11 ++--- ...ked_assign_bad.v => t_mem_packed_assign.v} | 2 + test_regress/t/t_mem_packed_bad.pl | 5 +- test_regress/t/t_mem_packed_bad.v | 2 +- 23 files changed, 268 insertions(+), 114 deletions(-) create mode 100755 test_regress/t/t_bitsel_struct.pl create mode 100644 test_regress/t/t_bitsel_struct.v create mode 100755 test_regress/t/t_bitsel_struct2.pl create mode 100644 test_regress/t/t_bitsel_struct2.v rename test_regress/t/{t_mem_packed_assign_bad.pl => t_mem_packed_assign.pl} (69%) rename test_regress/t/{t_mem_packed_assign_bad.v => t_mem_packed_assign.v} (96%) diff --git a/Changes b/Changes index 5fe680b56..2a8143189 100644 --- a/Changes +++ b/Changes @@ -3,6 +3,13 @@ Revision history for Verilator The contributors that suggested a given feature are shown in []. [by ...] indicates the contributor was also the author of the fix; Thanks! +* Verilator 3.8** devel + +*** Fix nested packed arrays and structs, bug600. [Jeremy Bennett] + Packed arrays are now represented as a single linear vector in + Verilated models this may affect packed arrays that are public or + accessed via the VPI. + * Verilator 3.844 2013/01/09 *** Support "unsigned int" DPI import functions, msg966. [Alex Lee] diff --git a/src/V3Ast.h b/src/V3Ast.h index 8f0aee3a7..18dc50a1b 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -560,8 +560,10 @@ struct VNumRange { int lsb() const { return m_lsb; } int left() const { return littleEndian()?lsb():msb(); } // How to show a declaration int right() const { return littleEndian()?msb():lsb(); } + int elements() const { return msb()-lsb()+1; } bool ranged() const { return m_ranged; } bool littleEndian() const { return m_littleEndian; } + int msbMaxSelect() const { return (lsb()<0 ? msb()-lsb() : msb()); } // Maximum value a [] select may index bool representableByWidth() const // Could be represented by just width=1, or [width-1:0] { return (!m_ranged || (m_lsb==0 && m_msb>=1 && !m_littleEndian)); } }; @@ -1515,7 +1517,7 @@ public: void generic(bool flag) { m_generic = flag; } AstNodeDType* dtypeDimensionp(int depth); pair dimensions(); - uint32_t arrayElements(); // 1, or total multiplication of all dimensions + uint32_t arrayUnpackedElements(); // 1, or total multiplication of all dimensions static int uniqueNumInc() { return ++s_uniqueNum; } }; diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 3be5efbf3..39ae5e6be 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -342,11 +342,11 @@ AstNodeDType* AstNodeDType::dtypeDimensionp(int dimension) { return NULL; } -uint32_t AstNodeDType::arrayElements() { +uint32_t AstNodeDType::arrayUnpackedElements() { uint32_t entries=1; for (AstNodeDType* dtypep=this; dtypep; ) { dtypep = dtypep->skipRefp(); // Skip AstRefDType/AstTypedef, or return same node - if (AstNodeArrayDType* adtypep = dtypep->castNodeArrayDType()) { + if (AstUnpackArrayDType* adtypep = dtypep->castUnpackArrayDType()) { entries *= adtypep->elementsConst(); dtypep = adtypep->subDTypep(); } @@ -754,6 +754,13 @@ void AstPackageImport::dump(ostream& str) { this->AstNode::dump(str); str<<" -> "<AstNode::dump(str); + if (declRange().ranged()) { + str<<" decl["<AstNode::dump(str); for (int i=0; i<(int)(AstBasicDTypeKwd::_ENUM_MAX); ++i) { diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index 454501e2b..4747852ab 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -717,8 +717,13 @@ struct AstSel : public AstNodeTriop { // Children: varref|arraysel, math, constant math // Tempting to have an lvalue() style method here as LHS selects are quite // different, but that doesn't play well with V3Inst and bidirects which don't know direction +private: + VNumRange m_declRange; // Range of the 'from' array if isRanged() is set, else invalid + int m_declElWidth; // If a packed array, the number of bits per element +public: AstSel(FileLine* fl, AstNode* fromp, AstNode* lsbp, AstNode* widthp) :AstNodeTriop(fl, fromp, lsbp, widthp) { + m_declElWidth = 1; if (widthp->castConst()) { dtypeSetLogicSized(widthp->castConst()->toUInt(), widthp->castConst()->toUInt(), @@ -728,9 +733,11 @@ struct AstSel : public AstNodeTriop { AstSel(FileLine* fl, AstNode* fromp, int lsb, int bitwidth) :AstNodeTriop(fl, fromp, new AstConst(fl,lsb), new AstConst(fl,bitwidth)) { + m_declElWidth = 1; dtypeSetLogicSized(bitwidth,bitwidth,AstNumeric::UNSIGNED); } ASTNODE_NODE_FUNCS(Sel, SEL) + virtual void dump(ostream& str); virtual void numberOperate(V3Number& out, const V3Number& from, const V3Number& bit, const V3Number& width) { out.opSel(from, bit.toUInt()+width.toUInt()-1, bit.toUInt()); } virtual string emitVerilog() { V3ERROR_NA; return ""; } // Implemented specially @@ -752,6 +759,10 @@ struct AstSel : public AstNodeTriop { int widthConst() const { return widthp()->castConst()->toSInt(); } int lsbConst() const { return lsbp()->castConst()->toSInt(); } int msbConst() const { return lsbConst()+widthConst()-1; } + VNumRange& declRange() { return m_declRange; } + void declRange(const VNumRange& flag) { m_declRange = flag; } + int declElWidth() const { return m_declElWidth; } + void declElWidth(int flag) { m_declElWidth = flag; } }; struct AstMemberSel : public AstNodeMath { @@ -2593,11 +2604,11 @@ public: , m_showname(showname) { dtypeFrom(varp); m_code = 0; - m_codeInc = varp->dtypep()->arrayElements() * varp->dtypep()->widthWords(); + m_codeInc = varp->dtypep()->arrayUnpackedElements() * varp->dtypep()->widthWords(); AstBasicDType* bdtypep = varp->basicp(); m_left = bdtypep ? bdtypep->left() : 0; m_right = bdtypep ? bdtypep->right() : 0; - if (AstNodeArrayDType* adtypep = varp->dtypeSkipRefp()->castNodeArrayDType()) { + if (AstUnpackArrayDType* adtypep = varp->dtypeSkipRefp()->castUnpackArrayDType()) { m_arrayLsb = adtypep->lsb(); m_arrayMsb = adtypep->msb(); } else { diff --git a/src/V3Changed.cpp b/src/V3Changed.cpp index 287a684d8..538f00790 100644 --- a/src/V3Changed.cpp +++ b/src/V3Changed.cpp @@ -86,7 +86,7 @@ private: #endif AstVar* varp = vscp->varp(); vscp->v3warn(IMPERFECTSCH,"Imperfect scheduling of variable: "<dtypeSkipRefp()->castNodeArrayDType(); + AstUnpackArrayDType* arrayp = varp->dtypeSkipRefp()->castUnpackArrayDType(); bool isArray = arrayp; int msb = isArray ? arrayp->msb() : 0; int lsb = isArray ? arrayp->lsb() : 0; diff --git a/src/V3Const.cpp b/src/V3Const.cpp index 4e2be9fa4..e0d5abd88 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -377,30 +377,12 @@ private: } // Find range of dtype we are selecting from // Similar code in V3Unknown::AstSel - int declMsbMaxSelect = -1; - int declLsb = -1; - bool doit = false; - AstNodeDType* dtypep = nodep->fromp()->dtypep()->skipRefp(); - if (!dtypep) nodep->v3fatalSrc("Select of non-selectable type"); - if (AstNodeArrayDType* adtypep = dtypep->castNodeArrayDType()) { - declMsbMaxSelect = adtypep->msbMaxSelect(); - declLsb = adtypep->lsb(); - doit = true; - } else if (AstBasicDType* adtypep = dtypep->castBasicDType()) { - declMsbMaxSelect = adtypep->msbMaxSelect(); - declLsb = adtypep->lsb(); - doit = (!adtypep->isRanged() || adtypep->msb()); // else it's non-resolvable parameterized - } else if (AstNodeClassDType* adtypep = dtypep->castNodeClassDType()) { - declMsbMaxSelect = adtypep->msbMaxSelect(); - declLsb = adtypep->lsb(); - doit = true; - } else { - nodep->v3error("Select from non-selectable "<prettyTypeName()); - } + bool doit = true; if (m_warn && nodep->lsbp()->castConst() && nodep->widthp()->castConst() && doit) { + int maxDeclBit = nodep->declRange().msbMaxSelect()*nodep->declElWidth() + (nodep->declElWidth()-1); if (nodep->lsbp()->castConst()->num().isFourState() || nodep->widthp()->castConst()->num().isFourState()) { nodep->v3error("Selection index is constantly unknown or tristated: " @@ -408,14 +390,19 @@ private: // Replacing nodep will make a mess above, so we replace the offender replaceZero(nodep->lsbp()); } - else if ((nodep->msbConst() > declMsbMaxSelect) - || (nodep->lsbConst() > declMsbMaxSelect)) { + else if (nodep->declRange().ranged() + && (nodep->msbConst() > maxDeclBit + || nodep->lsbConst() > maxDeclBit)) { // See also warning in V3Width + // Must adjust by element width as declRange() is in number of elements nodep->v3warn(SELRANGE, "Selection index out of range: " - <msbConst()<<":"<lsbConst() - <<" outside "<=0 ? "" - :" (adjusted +"+cvtToStr(-declLsb)+" to account for negative lsb)")); + <<(nodep->msbConst()/nodep->declElWidth()) + <<":"<<(nodep->lsbConst()/nodep->declElWidth()) + <<" outside "<declRange().msbMaxSelect()<<":0" + <<(nodep->declRange().lsb()>=0 ? "" + :(" (adjusted +"+cvtToStr(-nodep->declRange().lsb()) + +" to account for negative lsb)"))); + UINFO(1," Related Raw index is "<msbConst()<<":"<lsbConst()<width()*nodep->dtypep()->arrayElements()) > 256) return "Wide bus/array > 256 bits"; + if ((nodep->width()*nodep->dtypep()->arrayUnpackedElements()) > 256) return "Wide bus/array > 256 bits"; // We allow this, though tracing doesn't // if (nodep->arrayp(1)) return "Unsupported: Multi-dimensional array"; return NULL; @@ -209,7 +209,7 @@ private: varp, chgVarp); } } - else if (AstNodeArrayDType* adtypep = dtypep->castNodeArrayDType()) { + else if (AstUnpackArrayDType* adtypep = dtypep->castUnpackArrayDType()) { for (int index_docs=adtypep->lsb(); index_docs<=adtypep->msb()+1; ++index_docs) { int index_code = index_docs - adtypep->lsb(); ToggleEnt newent (above.m_comment+string("[")+cvtToStr(index_docs)+"]", diff --git a/src/V3EmitC.cpp b/src/V3EmitC.cpp index 3897e4e3e..56340555a 100644 --- a/src/V3EmitC.cpp +++ b/src/V3EmitC.cpp @@ -321,12 +321,12 @@ public: { AstVarRef* varrefp = nodep->memp()->castVarRef(); if (!varrefp) { nodep->v3error("Readmem loading non-variable"); } - else if (AstNodeArrayDType* adtypep = varrefp->varp()->dtypeSkipRefp()->castNodeArrayDType()) { - puts(cvtToStr(varrefp->varp()->dtypep()->arrayElements())); + else if (AstUnpackArrayDType* adtypep = varrefp->varp()->dtypeSkipRefp()->castUnpackArrayDType()) { + puts(cvtToStr(varrefp->varp()->dtypep()->arrayUnpackedElements())); array_lsb = adtypep->lsb(); } else { - nodep->v3error("Readmem loading non-arrayed variable"); + nodep->v3error("Readmem loading other than unpacked-array variable"); } } putbs(", "); @@ -889,8 +889,8 @@ void EmitCStmts::emitVarDecl(AstVar* nodep, const string& prefixIfImp) { } puts(nodep->name()); if (isArray) { - for (AstNodeArrayDType* arrayp=nodep->dtypeSkipRefp()->castNodeArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castNodeArrayDType()) { + for (AstUnpackArrayDType* arrayp=nodep->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { puts("["+cvtToStr(arrayp->elementsConst())+"]"); } } @@ -910,8 +910,8 @@ void EmitCStmts::emitVarDecl(AstVar* nodep, const string& prefixIfImp) { if (isArray) { if (nodep->isWide()) puts("W"); puts("("+nodep->name()); - for (AstNodeArrayDType* arrayp=nodep->dtypeSkipRefp()->castNodeArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castNodeArrayDType()) { + for (AstUnpackArrayDType* arrayp=nodep->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { puts("["+cvtToStr(arrayp->elementsConst())+"]"); } puts(","+cvtToStr(basicp->msb())+","+cvtToStr(basicp->lsb())); @@ -932,8 +932,8 @@ void EmitCStmts::emitVarDecl(AstVar* nodep, const string& prefixIfImp) { // strings and other fundamental c types puts(nodep->vlArgType(true,false)); // This isn't very robust and may need cleanup for other data types - for (AstNodeArrayDType* arrayp=nodep->dtypeSkipRefp()->castNodeArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castNodeArrayDType()) { + for (AstUnpackArrayDType* arrayp=nodep->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { puts("["+cvtToStr(arrayp->elementsConst())+"]"); } puts(";\n"); @@ -958,12 +958,13 @@ void EmitCStmts::emitVarDecl(AstVar* nodep, const string& prefixIfImp) { if (prefixIfImp!="") { puts(prefixIfImp); puts("::"); } puts(nodep->name()); // This isn't very robust and may need cleanup for other data types - for (AstNodeArrayDType* arrayp=nodep->dtypeSkipRefp()->castNodeArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castNodeArrayDType()) { + for (AstUnpackArrayDType* arrayp=nodep->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { puts("["+cvtToStr(arrayp->elementsConst())+"]"); } - puts(","+cvtToStr(basicp->msb())+","+cvtToStr(basicp->lsb())); - if (basicp->isWide()) puts(","+cvtToStr(basicp->widthWords())); + // If it's a packed struct/array then nodep->width is the whole thing, msb/lsb is just lowest dimension + puts(","+cvtToStr(basicp->lsb()+nodep->width())+","+cvtToStr(basicp->lsb())); + if (nodep->isWide()) puts(","+cvtToStr(nodep->widthWords())); puts(");\n"); } } @@ -1328,7 +1329,7 @@ void EmitCImp::emitVarResets(AstNodeModule* modp) { } else if (AstInitArray* initarp = varp->valuep()->castInitArray()) { AstConst* constsp = initarp->initsp()->castConst(); - if (AstNodeArrayDType* arrayp = varp->dtypeSkipRefp()->castNodeArrayDType()) { + if (AstUnpackArrayDType* arrayp = varp->dtypeSkipRefp()->castUnpackArrayDType()) { for (int i=0; ielementsConst(); i++) { if (!constsp) initarp->v3fatalSrc("Not enough values in array initalizement"); emitSetVarConstant(varp->name()+"["+cvtToStr(i)+"]", constsp); @@ -1341,8 +1342,8 @@ void EmitCImp::emitVarResets(AstNodeModule* modp) { else { int vects = 0; // This isn't very robust and may need cleanup for other data types - for (AstNodeArrayDType* arrayp=varp->dtypeSkipRefp()->castNodeArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castNodeArrayDType()) { + for (AstUnpackArrayDType* arrayp=varp->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { int vecnum = vects++; if (arrayp->msb() < arrayp->lsb()) varp->v3fatalSrc("Should have swapped msb & lsb earlier."); string ivar = string("__Vi")+cvtToStr(vecnum); @@ -1508,8 +1509,8 @@ void EmitCImp::emitSavableImp(AstNodeModule* modp) { else { int vects = 0; // This isn't very robust and may need cleanup for other data types - for (AstNodeArrayDType* arrayp=varp->dtypeSkipRefp()->castNodeArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castNodeArrayDType()) { + for (AstUnpackArrayDType* arrayp=varp->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { int vecnum = vects++; if (arrayp->msb() < arrayp->lsb()) varp->v3fatalSrc("Should have swapped msb & lsb earlier."); string ivar = string("__Vi")+cvtToStr(vecnum); @@ -1599,8 +1600,8 @@ void EmitCImp::emitSensitives() { if (varp->isInput() && (varp->isScSensitive() || varp->isUsedClock())) { int vects = 0; // This isn't very robust and may need cleanup for other data types - for (AstNodeArrayDType* arrayp=varp->dtypeSkipRefp()->castNodeArrayDType(); arrayp; - arrayp = arrayp->subDTypep()->skipRefp()->castNodeArrayDType()) { + for (AstUnpackArrayDType* arrayp=varp->dtypeSkipRefp()->castUnpackArrayDType(); arrayp; + arrayp = arrayp->subDTypep()->skipRefp()->castUnpackArrayDType()) { int vecnum = vects++; if (arrayp->msb() < arrayp->lsb()) varp->v3fatalSrc("Should have swapped msb & lsb earlier."); string ivar = string("__Vi")+cvtToStr(vecnum); @@ -1697,7 +1698,7 @@ void EmitCStmts::emitVarList(AstNode* firstp, EisWhich which, const string& pref int sigbytes = varp->dtypeSkipRefp()->widthAlignBytes(); int sortbytes = sortmax-1; if (varp->isUsedClock() && varp->widthMin()==1) sortbytes = 0; - else if (varp->dtypeSkipRefp()->castNodeArrayDType()) sortbytes=8; + else if (varp->dtypeSkipRefp()->castUnpackArrayDType()) sortbytes=8; else if (varp->basicp() && varp->basicp()->isOpaque()) sortbytes=7; else if (varp->isScBv()) sortbytes=6; else if (sigbytes==8) sortbytes=5; @@ -2234,7 +2235,7 @@ class EmitCTrace : EmitCStmts { if (emitTraceIsScBv(nodep)) puts("VL_SC_BV_DATAP("); varrefp->iterate(*this); // Put var name out // Tracing only supports 1D arrays - if (varp->dtypeSkipRefp()->castNodeArrayDType()) { + if (varp->dtypeSkipRefp()->castUnpackArrayDType()) { if (arrayindex==-2) puts("[i]"); else if (arrayindex==-1) puts("[0]"); else puts("["+cvtToStr(arrayindex)+"]"); diff --git a/src/V3EmitCSyms.cpp b/src/V3EmitCSyms.cpp index 9225d2a92..1d9ca61c2 100644 --- a/src/V3EmitCSyms.cpp +++ b/src/V3EmitCSyms.cpp @@ -454,29 +454,30 @@ void EmitCSyms::emitSymImp() { AstScope* scopep = it->second.m_scopep; AstVar* varp = it->second.m_varp; // - int dim=0; + int pdim=0; + int udim=0; string bounds; if (AstBasicDType* basicp = varp->basicp()) { // Range is always first, it's not in "C" order if (basicp->isRanged()) { bounds += " ,"; bounds += cvtToStr(basicp->msb()); bounds += ","; bounds += cvtToStr(basicp->lsb()); - dim++; + pdim++; } for (AstNodeDType* dtypep=varp->dtypep(); dtypep; ) { dtypep = dtypep->skipRefp(); // Skip AstRefDType/AstTypedef, or return same node if (AstNodeArrayDType* adtypep = dtypep->castNodeArrayDType()) { bounds += " ,"; bounds += cvtToStr(adtypep->msb()); bounds += ","; bounds += cvtToStr(adtypep->lsb()); - dim++; + if (dtypep->castPackArrayDType()) pdim++; else udim++; dtypep = adtypep->subDTypep(); } else break; // AstBasicDType - nothing below, 1 } } // - if (dim>2) { - puts("//UNSUP "); // VerilatedImp can't deal with >2d arrays + if (pdim>1 || udim>1) { + puts("//UNSUP "); // VerilatedImp can't deal with >2d or packed arrays } puts("__Vscope_"+it->second.m_scopeName+".varInsert(__Vfinal,"); putsQuoted(it->second.m_varBasePretty); @@ -496,7 +497,7 @@ void EmitCSyms::emitSymImp() { if (varp->isSigUserRWPublic()) puts("|VLVF_PUB_RW"); else if (varp->isSigUserRdPublic()) puts("|VLVF_PUB_RD"); puts(","); - puts(cvtToStr(dim)); + puts(cvtToStr(pdim+udim)); puts(bounds); puts(");\n"); } diff --git a/src/V3LinkResolve.cpp b/src/V3LinkResolve.cpp index 54e744d75..fffe0b283 100644 --- a/src/V3LinkResolve.cpp +++ b/src/V3LinkResolve.cpp @@ -203,7 +203,7 @@ private: if (AstNodeVarRef* varrefp = basefromp->castNodeVarRef()) { // Maybe varxref - so need to clone nodep->attrp(new AstAttrOf(nodep->fileline(), AstAttrType::VAR_BASE, varrefp->cloneTree(false))); - } else if (AstMemberSel* fromp = nodep->fromp()->castMemberSel()) { + } else if (AstMemberSel* fromp = basefromp->castMemberSel()) { nodep->attrp(new AstAttrOf(nodep->fileline(), AstAttrType::MEMBER_BASE, fromp->cloneTree(false))); } else { diff --git a/src/V3Slice.cpp b/src/V3Slice.cpp index 0351ce75b..7d5c976e0 100644 --- a/src/V3Slice.cpp +++ b/src/V3Slice.cpp @@ -77,7 +77,7 @@ class SliceCloneVisitor : public AstNVisitor { m_selBits.push_back(vector()); AstVar* varp = m_refp->varp(); pair arrDim = varp->dtypep()->dimensions(); - uint32_t dimensions = arrDim.first + arrDim.second; + uint32_t dimensions = arrDim.second; for (uint32_t i = 0; i < dimensions; ++i) { m_selBits[m_vecIdx].push_back(0); } @@ -253,7 +253,7 @@ class SliceVisitor : public AstNVisitor { AstNode* topp = nodep; for (unsigned i = start; i < start + count; ++i) { AstNodeDType* dtypep = varp->dtypep()->dtypeDimensionp(i-1); - AstNodeArrayDType* adtypep = dtypep->castNodeArrayDType(); + AstUnpackArrayDType* adtypep = dtypep->castUnpackArrayDType(); if (!adtypep) nodep->v3fatalSrc("insertImplicit tried to expand an array without an ArrayDType"); vlsint32_t msb = adtypep->msb(); vlsint32_t lsb = adtypep->lsb(); @@ -293,7 +293,7 @@ class SliceVisitor : public AstNVisitor { // case we need to create a slice accross the entire Var if (m_assignp && !nodep->backp()->castArraySel()) { pair arrDim = nodep->varp()->dtypep()->dimensions(); - uint32_t dimensions = arrDim.first + arrDim.second; + uint32_t dimensions = arrDim.second; // unpacked only if (dimensions > 0) { AstVarRef* clonep = nodep->cloneTree(false); clonep->user1p(nodep); @@ -307,7 +307,7 @@ class SliceVisitor : public AstNVisitor { virtual void visit(AstExtend* nodep, AstNUser*) { m_extend = true; if (m_assignp && m_assignp->user2() > 1 && !m_assignError) { - m_assignp->v3error("Unsupported: Assignment between packed arrays of different dimensions"); + m_assignp->v3error("Unsupported: Assignment between unpacked arrays of different dimensions"); m_assignError = true; } nodep->iterateChildren(*this); @@ -327,7 +327,7 @@ class SliceVisitor : public AstNVisitor { unsigned dim = explicitDimensions(nodep); AstVarRef* refp = nodep->user1p()->castNode()->castVarRef(); pair arrDim = refp->varp()->dtypep()->dimensions(); - uint32_t implicit = (arrDim.first + arrDim.second) - dim; + uint32_t implicit = (arrDim.second) - dim; if (implicit > 0) { AstArraySel* newp = insertImplicit(nodep->cloneTree(false), dim+1, implicit); nodep->replaceWith(newp); nodep = newp; @@ -338,7 +338,7 @@ class SliceVisitor : public AstNVisitor { m_assignp->v3error("Slices of arrays in assignments must have the same unpacked dimensions"); } else if (!m_assignp->user2()) { if (m_extend && clones > 1 && !m_assignError) { - m_assignp->v3error("Unsupported: Assignment between packed arrays of different dimensions"); + m_assignp->v3error("Unsupported: Assignment between unpacked arrays of different dimensions"); m_assignError = true; } if (clones > 1 && !refp->lvalue() && refp->varp() == m_lhsVarRefp->varp() @@ -354,7 +354,7 @@ class SliceVisitor : public AstNVisitor { virtual void visit(AstSel* nodep, AstNUser*) { m_extend = true; if (m_assignp && m_assignp->user2() > 1 && !m_assignError) { - m_assignp->v3error("Unsupported: Assignment between packed arrays of different dimensions"); + m_assignp->v3error("Unsupported: Assignment between unpacked arrays of different dimensions"); m_assignError = true; } nodep->iterateChildren(*this); @@ -432,9 +432,9 @@ class SliceVisitor : public AstNVisitor { if ((int)(dim - varDim.second) < 0) { // Unpacked dimensions are referenced first, make sure we have them all nodep->v3error("Unary operator used across unpacked dimensions"); - } else if ((int)(dim - (varDim.first + varDim.second)) < 0) { + } else if ((int)(dim - (varDim.second)) < 0) { // Implicit packed dimensions are allowed, make them explicit - uint32_t newDim = (varDim.first + varDim.second) - dim; + uint32_t newDim = (varDim.second) - dim; AstNode* clonep = nodep->lhsp()->cloneTree(false); clonep->user1p(refp); AstNode* newp = insertImplicit(clonep, dim+1, newDim); diff --git a/src/V3Table.cpp b/src/V3Table.cpp index a8c8c3f0b..c5f8b98a1 100644 --- a/src/V3Table.cpp +++ b/src/V3Table.cpp @@ -349,7 +349,8 @@ private: AstVarScope* vsc2p= *it; AstVar* var2p = vsc2p->varp(); if (var1p->width() == var2p->width() - && var1p->dtypep()->arrayElements() == var2p->dtypep()->arrayElements()) { + && (var1p->dtypep()->arrayUnpackedElements() + == var2p->dtypep()->arrayUnpackedElements())) { AstNode* init1p = var1p->valuep()->castInitArray(); AstNode* init2p = var2p->valuep()->castInitArray(); if (init1p->sameTree(init2p)) { diff --git a/src/V3TraceDecl.cpp b/src/V3TraceDecl.cpp index 13e9a6e48..5c348cf30 100644 --- a/src/V3TraceDecl.cpp +++ b/src/V3TraceDecl.cpp @@ -75,10 +75,10 @@ private: return "Inlined leading underscore"; } if ((int)nodep->width() > v3Global.opt.traceMaxWidth()) return "Wide bus > --trace-max-width bits"; - if ((int)nodep->dtypep()->arrayElements() > v3Global.opt.traceMaxArray()) return "Wide memory > --trace-max-array ents"; + if ((int)nodep->dtypep()->arrayUnpackedElements() > v3Global.opt.traceMaxArray()) return "Wide memory > --trace-max-array ents"; if (!(nodep->dtypeSkipRefp()->castBasicDType() - || (nodep->dtypeSkipRefp()->castNodeArrayDType() - && (nodep->dtypeSkipRefp()->castNodeArrayDType()->subDTypep() + || (nodep->dtypeSkipRefp()->castUnpackArrayDType() + && (nodep->dtypeSkipRefp()->castUnpackArrayDType()->subDTypep() ->skipRefp()->castBasicDType())))) { return "Unsupported: Multi-dimensional array"; } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index 2e2d43bbd..0a14a5065 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -417,7 +417,7 @@ private: nodep->dtypeSetLogicBool(); return; } int width = nodep->widthConst(); - nodep->dtypeSetLogicSized(width,width,AstNumeric::UNSIGNED); + if (!nodep->dtypep()) nodep->v3fatalSrc("dtype wasn't set") // by V3WidthSel if (nodep->lsbp()->castConst() && nodep->msbConst() < nodep->lsbConst()) { nodep->v3error("Unsupported: MSB < LSB of bit extract: " @@ -437,31 +437,29 @@ private: } // Check bit indexes. // What is the MSB? We want the true MSB, not one starting at 0, - // because a 4 bit index is required to look at a one-bit variable[15:15] + // because a 4 bit index is required to look at a one-bit variable[15:15] and 5 bits for [15:-2] int frommsb = nodep->fromp()->width() - 1; int fromlsb = 0; - AstNodeVarRef* varrp = nodep->fromp()->castNodeVarRef(); - if (varrp - && varrp->varp()->basicp() - && varrp->varp()->basicp()->isRanged()) { // Selecting a bit from a multibit register - frommsb = varrp->varp()->basicp()->msbMaxSelect(); // Corrected for negative lsb - fromlsb = varrp->varp()->basicp()->lsb(); + int elw = nodep->declElWidth(); // Must adjust to tell user bit ranges + if (nodep->declRange().ranged()) { + frommsb = nodep->declRange().msbMaxSelect()*elw + (elw-1); // Corrected for negative lsb + fromlsb = nodep->declRange().lsb()*elw; + } else { + //nodep->v3fatalSrc("Should have been declRanged in V3WidthSel"); } int selwidth = V3Number::log2b(frommsb+1-1)+1; // Width to address a bit nodep->fromp()->iterateAndNext(*this,WidthVP(selwidth,selwidth,FINAL).p()); nodep->lsbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,FINAL).p()); if (widthBad(nodep->lsbp(),selwidth,selwidth) && nodep->lsbp()->width()!=32) { - nodep->v3warn(WIDTH,"Bit extraction of var["<lsbp()->width() - <<(nodep->lsbp()->width()!=nodep->lsbp()->widthMin() - ?" or "+cvtToStr(nodep->lsbp()->widthMin()):"") - <<" bits."); if (!nodep->fileline()->warnIsOff(V3ErrorCode::WIDTH)) { + nodep->v3warn(WIDTH,"Bit extraction of var["<<(frommsb/elw)<<":"<<(fromlsb/elw)<<"] requires " + <<(selwidth/elw)<<" bit index, not " + <<(nodep->lsbp()->width()/elw) + <<(nodep->lsbp()->width()!=nodep->lsbp()->widthMin() + ?" or "+cvtToStr(nodep->lsbp()->widthMin()/elw):"") + <<" bits."); UINFO(1," Related node: "<varp()<varp()->dtypep()<lsbp()->castConst() && nodep->msbConst() > frommsb) { @@ -480,8 +478,6 @@ private: <msbConst()<<":"<lsbConst() <<" outside "<varp()<varp()->dtypep()<fromp()->dtypep()->skipRefp(); - if (AstNodeArrayDType* adtypep = fromDtp->castNodeArrayDType()) { + if (AstUnpackArrayDType* adtypep = fromDtp->castUnpackArrayDType()) { frommsb = adtypep->msb(); fromlsb = adtypep->lsb(); if (fromlsb>frommsb) {int t=frommsb; frommsb=fromlsb; fromlsb=t; } @@ -694,7 +690,13 @@ private: // Cleanup array size nodep->rangep()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); nodep->dtypep(nodep); // The array itself, not subDtype - nodep->widthFromSub(nodep->subDTypep()); + if (nodep->castUnpackArrayDType()) { + // Historically array elements have width of the ref type not the full array + nodep->widthFromSub(nodep->subDTypep()); + } else { + int width = nodep->subDTypep()->width() * nodep->rangep()->elementsConst(); + nodep->widthForce(width,width); + } UINFO(4,"dtWidthed "<dumpTree(cout," AssignPre: "); AstNodeDType* oldAssDTypep = m_assDTypep; { + //if (debug()) nodep->dumpTree(cout,"- assin: "); nodep->lhsp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); if (!nodep->lhsp()->dtypep()) nodep->v3fatalSrc("How can LHS be untyped?"); if (!nodep->lhsp()->dtypep()->widthSized()) nodep->v3fatalSrc("How can LHS be unsized?"); m_assDTypep = nodep->lhsp()->dtypep(); nodep->rhsp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,PRELIM).p()); + //if (debug()) nodep->dumpTree(cout,"- assign: "); if (!nodep->lhsp()->isDouble() && nodep->rhsp()->isDouble()) { spliceCvtS(nodep->rhsp(), false); // Round RHS } else if (nodep->lhsp()->isDouble() && !nodep->rhsp()->isDouble()) { @@ -1376,8 +1380,8 @@ private: virtual void visit(AstReadMem* nodep, AstNUser*) { nodep->filenamep()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); nodep->memp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); - if (!nodep->memp()->dtypep()->skipRefp()->castNodeArrayDType()) { - nodep->memp()->v3error("Unsupported: $readmem into non-array"); + if (!nodep->memp()->dtypep()->skipRefp()->castUnpackArrayDType()) { + nodep->memp()->v3error("Unsupported: $readmem into other than unpacked array"); } nodep->lsbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); nodep->msbp()->iterateAndNext(*this,WidthVP(ANYSIZE,0,BOTH).p()); diff --git a/src/V3WidthSel.cpp b/src/V3WidthSel.cpp index 3b85c898d..7e296e56a 100644 --- a/src/V3WidthSel.cpp +++ b/src/V3WidthSel.cpp @@ -196,14 +196,38 @@ private: FromData fromdata = fromDataForArray(nodep, fromp, false); AstNodeDType* ddtypep = fromdata.m_dtypep; VNumRange fromRange = fromdata.m_fromRange; - if (ddtypep->castNodeArrayDType()) { + UINFO(6," ddtypep "<castUnpackArrayDType()) { // SELBIT(array, index) -> ARRAYSEL(array, index) AstNode* subp = rhsp; if (fromRange.lsb()!=0 || fromRange.msb()<0) { subp = newSubNeg (subp, fromRange.lsb()); } - AstArraySel* newp = new AstArraySel (nodep->fileline(), + AstArraySel* newp = new AstArraySel (nodep->fileline(), fromp, subp); + newp->dtypeFrom(adtypep->subDTypep()); // Need to strip off array reference + if (debug()>=9) newp->dumpTree(cout,"--SELBTn: "); + nodep->replaceWith(newp); pushDeletep(nodep); nodep=NULL; + } + else if (AstPackArrayDType* adtypep = ddtypep->castPackArrayDType()) { + // SELBIT(array, index) -> SEL(array, index*width-of-subindex, width-of-subindex) + AstNode* subp = rhsp; + if (fromRange.lsb()!=0 || fromRange.msb()<0) { + subp = newSubNeg (subp, fromRange.lsb()); + } + if (!fromRange.elements() || (adtypep->width() % fromRange.elements())!=0) + adtypep->v3fatalSrc("Array extraction with width miscomputed " + <width()<<"/"<width() / fromRange.elements(); + AstSel* newp = new AstSel (nodep->fileline(), + fromp, + new AstMul(nodep->fileline(), + new AstConst(nodep->fileline(),AstConst::Unsized32(),elwidth), + newSubLsbOf(rhsp, fromRange)), + new AstConst (nodep->fileline(),AstConst::Unsized32(),elwidth)); + newp->declRange(fromRange); + newp->declElWidth(elwidth); + newp->dtypeFrom(adtypep->subDTypep()); // Need to strip off array reference if (debug()>=9) newp->dumpTree(cout,"--SELBTn: "); nodep->replaceWith(newp); pushDeletep(nodep); nodep=NULL; } @@ -214,6 +238,7 @@ private: newSubLsbOf(rhsp, fromRange), // Unsized so width from user new AstConst (nodep->fileline(),AstConst::Unsized32(),1)); + newp->declRange(fromRange); UINFO(6," new "<=9) newp->dumpTree(cout,"--SELBTn: "); nodep->replaceWith(newp); pushDeletep(nodep); nodep=NULL; @@ -225,6 +250,7 @@ private: newSubLsbOf(rhsp, fromRange), // Unsized so width from user new AstConst (nodep->fileline(),AstConst::Unsized32(),1)); + newp->declRange(fromRange); UINFO(6," new "<=9) newp->dumpTree(cout,"--SELBTn: "); nodep->replaceWith(newp); pushDeletep(nodep); nodep=NULL; @@ -257,13 +283,14 @@ private: FromData fromdata = fromDataForArray(nodep, fromp, false); AstNodeDType* ddtypep = fromdata.m_dtypep; VNumRange fromRange = fromdata.m_fromRange; - if (ddtypep->castNodeArrayDType()) { + if (ddtypep->castUnpackArrayDType()) { // Slice extraction AstArraySel* newp = new AstArraySel (nodep->fileline(), fromp, lsbp); newp->start(lsb); newp->length((msb - lsb) + 1); nodep->replaceWith(newp); pushDeletep(nodep); nodep=NULL; - } else if (ddtypep->castBasicDType()) { + } + else if (ddtypep->castBasicDType()) { if (fromRange.littleEndian()) { // Below code assumes big bit endian; just works out if we swap int x = msb; msb = lsb; lsb = x; @@ -278,10 +305,12 @@ private: fromp, newSubLsbOf(lsbp, fromRange), widthp); + newp->declRange(fromRange); UINFO(6," new "<=9) newp->dumpTree(cout,"--SELEXnew: "); nodep->replaceWith(newp); pushDeletep(nodep); nodep=NULL; - } else if (ddtypep->castNodeClassDType()) { + } + else if (ddtypep->castNodeClassDType()) { // Classes aren't little endian if (lsb > msb) { nodep->v3error("["<declRange(fromRange); UINFO(6," new "<=9) newp->dumpTree(cout,"--SELEXnew: "); nodep->replaceWith(newp); pushDeletep(nodep); nodep=NULL; @@ -361,6 +391,7 @@ private: } else { nodep->v3fatalSrc("Bad Case"); } + newp->declRange(fromRange); UINFO(6," new "<replaceWith(newp); pushDeletep(nodep); nodep=NULL; } diff --git a/test_regress/t/t_bitsel_struct.pl b/test_regress/t/t_bitsel_struct.pl new file mode 100755 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_bitsel_struct.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_bitsel_struct.v b/test_regress/t/t_bitsel_struct.v new file mode 100644 index 000000000..f8920d9ca --- /dev/null +++ b/test_regress/t/t_bitsel_struct.v @@ -0,0 +1,32 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// A test case for struct signal bit selection. +// +// This test is to check that bit selection of multi-dimensional signal inside +// of a struct works. +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2012 by Jie Xu. + +module t(/*AUTOARG*/ + // Inputs + clk + ); + + input clk; + typedef struct packed { + logic [1:0][15:0] channel; + logic others; + } buss_t; + + buss_t b; + reg [7:0] a; + + initial begin + b = {16'h8765,16'h4321,1'b1}; + a = b.channel[0][8+:8]; + if (a != 8'h43) $stop; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_bitsel_struct2.pl b/test_regress/t/t_bitsel_struct2.pl new file mode 100755 index 000000000..f91289753 --- /dev/null +++ b/test_regress/t/t_bitsel_struct2.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_bitsel_struct2.v b/test_regress/t/t_bitsel_struct2.v new file mode 100644 index 000000000..1dd6bf179 --- /dev/null +++ b/test_regress/t/t_bitsel_struct2.v @@ -0,0 +1,36 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed into the Public Domain, for any use, +// without warranty, 2013 by Wilson Snyder. + +module t (/*AUTOARG*/); + + typedef struct packed { + logic [3:2] a; + logic [5:4][3:2] b; + } ab_t; + typedef ab_t [7:6] c_t; // array of structs + typedef struct packed { + c_t [17:16] d; + } e_t; + +`define check(got,expec) do if ((got) != (expec)) begin $display("Line%d: Got %b Exp %b\n", `__LINE__, (got), (expec)); $stop; end while(0); + + initial begin + e_t e; + if ($bits(ab_t)!=6) $stop; + if ($bits(c_t)!=12) $stop; + if ($bits(e_t)!=24) $stop; + e = 24'b101101010111010110101010; + `check(e, 24'b101101010111010110101010); + e.d[17] = 12'b111110011011; + `check(e, 24'b111110011011010110101010); + e.d[16][6] = 6'b010101; + `check(e, 24'b111110011011010110010101); + e.d[16][6].b[5] = 2'b10; + `check(e, 24'b111110011011010110011001); + e.d[16][6].b[5][2] = 1'b1; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_mem_packed_assign_bad.pl b/test_regress/t/t_mem_packed_assign.pl similarity index 69% rename from test_regress/t/t_mem_packed_assign_bad.pl rename to test_regress/t/t_mem_packed_assign.pl index 949f3e64f..9ae333078 100755 --- a/test_regress/t/t_mem_packed_assign_bad.pl +++ b/test_regress/t/t_mem_packed_assign.pl @@ -8,12 +8,11 @@ if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); di # Version 2.0. compile ( - v_flags2 => ["--lint-only"], - fails=>1, - expect=> -'%Error: t/t_mem_packed_assign_bad.v:\d+: Unsupported: Assignment between packed arrays of different dimensions -.*', - ); + ); + +execute ( + check_finished=>1, + ); ok(1); 1; diff --git a/test_regress/t/t_mem_packed_assign_bad.v b/test_regress/t/t_mem_packed_assign.v similarity index 96% rename from test_regress/t/t_mem_packed_assign_bad.v rename to test_regress/t/t_mem_packed_assign.v index dfec487d5..a15ba5b37 100644 --- a/test_regress/t/t_mem_packed_assign_bad.v +++ b/test_regress/t/t_mem_packed_assign.v @@ -24,7 +24,9 @@ module t (/*AUTOARG*/ cyc <= cyc + 1; arr_c <= arr_c + 1; arr2 <= arr2 + 1; +`ifdef TEST_VERBOSE $write("cyc%0d c:%0x a0:%0x a1:%0x a2:%0x a3:%0x\n", cyc, arr_c, arr[0], arr[1], arr[2], arr[3]); +`endif if (cyc==99) begin $write("*-* All Finished *-*\n"); $finish; diff --git a/test_regress/t/t_mem_packed_bad.pl b/test_regress/t/t_mem_packed_bad.pl index 8b097f12f..32f91116e 100755 --- a/test_regress/t/t_mem_packed_bad.pl +++ b/test_regress/t/t_mem_packed_bad.pl @@ -13,10 +13,7 @@ compile ( verilator_flags2 => ["--lint-only"], fails=>1, expect=> -'%Error: t/t_mem_packed_bad.v:\d+: Unsupported: Assignment between packed arrays of different dimensions -%Error: t/t_mem_packed_bad.v:\d+: Unsupported: Assignment between packed arrays of different dimensions -%Error: t/t_mem_packed_bad.v:\d+: Unsupported: Assignment between packed arrays of different dimensions -%Error: t/t_mem_packed_bad.v:\d+: Unsupported: Assignment between packed arrays of different dimensions +'%Error: t/t_mem_packed_bad.v:\d+: Unsupported: Assignment between unpacked arrays of different dimensions %Error: Exiting due to.*', ); diff --git a/test_regress/t/t_mem_packed_bad.v b/test_regress/t/t_mem_packed_bad.v index d52bf76eb..28453ca75 100644 --- a/test_regress/t/t_mem_packed_bad.v +++ b/test_regress/t/t_mem_packed_bad.v @@ -18,7 +18,7 @@ module t (/*AUTOARG*/ /* verilator lint_off WIDTH */ always @ (posedge clk) begin - // LHS is a 2D packed array, RHS is 1D packed or Const. Unsupported. + // LHS is a 2D packed array, RHS is 1D packed or Const. Allowed now. ch01 <= {{2{28'd4}}}; ch02 <= {{2{cyc}}}; ch03 <= 56'd0;