diff --git a/Changes b/Changes index 97ea9ae11..9e4611092 100644 --- a/Changes +++ b/Changes @@ -12,6 +12,8 @@ The contributors that suggested a given feature are shown in []. Thanks! **** Fix WIDTH warnings on comparisons with nullptr (#2602). [Rupert Swarbrick] +**** Fix queue poping wrong value when otherwise unused (#2512). [nanduraj1] + * Verilator 4.102 2020-10-15 diff --git a/src/V3Ast.h b/src/V3Ast.h index 45f6e9ae6..3f94d53b3 100644 --- a/src/V3Ast.h +++ b/src/V3Ast.h @@ -125,14 +125,22 @@ inline std::ostream& operator<<(std::ostream& os, const VLifetime& rhs) { class VAccess { public: - enum en : uint8_t { READ, WRITE }; + enum en : uint8_t { + READ, // Read/Consumed, variable not changed + WRITE, // Written/Updated, variable might be updated, but not consumed + // // so variable might be removable if not consumed elsewhere + READWRITE, // Read/Consumed and written/updated, variable both set and + // // also consumed, cannot remove usage of variable. + // // For non-simple data types only e.g. no tristates/delayed vars. + NOCHANGE // No change to previous state, used only in V3LinkLValue + }; enum en m_e; const char* ascii() const { - static const char* const names[] = {"RD", "WR"}; + static const char* const names[] = {"RD", "WR", "RW", "--"}; return names[m_e]; } const char* arrow() const { - static const char* const names[] = {"[RV] <-", "[LV] =>"}; + static const char* const names[] = {"[RV] <-", "[LV] =>", "[LV] <=>", "--"}; return names[m_e]; } inline VAccess() @@ -143,10 +151,13 @@ public: explicit inline VAccess(int _e) : m_e(static_cast(_e)) {} // Need () or GCC 4.8 false warning operator en() const { return m_e; } - VAccess invert() const { return (m_e == WRITE) ? VAccess(READ) : VAccess(WRITE); } - bool isReadOnly() const { return m_e == READ; } // False if/when support READWRITE - bool isWrite() const { return m_e == WRITE; } // Need audit if/when support READWRITE - bool isWriteOnly() const { return m_e == WRITE; } // False if/when support READWRITE + VAccess invert() const { + return (m_e == READWRITE) ? VAccess(m_e) : (m_e == WRITE ? VAccess(READ) : VAccess(WRITE)); + } + bool isReadOnly() const { return m_e == READ; } // False with READWRITE + bool isReadOrRW() const { return m_e == READ || m_e == READWRITE; } + bool isWriteOrRW() const { return m_e == WRITE || m_e == READWRITE; } + bool isRW() const { return m_e == READWRITE; } // False with READWRITE }; inline bool operator==(const VAccess& lhs, const VAccess& rhs) { return lhs.m_e == rhs.m_e; } inline bool operator==(const VAccess& lhs, VAccess::en rhs) { return lhs.m_e == rhs; } diff --git a/src/V3AstNodes.h b/src/V3AstNodes.h index facf04f6c..b4f564412 100644 --- a/src/V3AstNodes.h +++ b/src/V3AstNodes.h @@ -2366,7 +2366,7 @@ public: } } virtual int instrCount() const override { - return widthInstrs() * (access().isWriteOnly() ? 1 : instrCountLd()); + return widthInstrs() * (access().isReadOrRW() ? instrCountLd() : 1); } virtual string emitVerilog() override { V3ERROR_NA_RETURN(""); } virtual string emitC() override { V3ERROR_NA_RETURN(""); } diff --git a/src/V3Broken.cpp b/src/V3Broken.cpp index 9c1082689..f017c7fe5 100644 --- a/src/V3Broken.cpp +++ b/src/V3Broken.cpp @@ -275,7 +275,7 @@ private: processAndIterate(nodep); UASSERT_OBJ(!(v3Global.assertDTypesResolved() && nodep->brokeLhsMustBeLvalue() && VN_IS(nodep->lhsp(), NodeVarRef) - && !VN_CAST(nodep->lhsp(), NodeVarRef)->access().isWrite()), + && !VN_CAST(nodep->lhsp(), NodeVarRef)->access().isWriteOrRW()), nodep, "Assignment LHS is not an lvalue"); } virtual void visit(AstNode* nodep) override { diff --git a/src/V3Cdc.cpp b/src/V3Cdc.cpp index 72d4bc243..993f016fd 100644 --- a/src/V3Cdc.cpp +++ b/src/V3Cdc.cpp @@ -661,7 +661,7 @@ private: // We use weight of one for normal edges, // Weight of CDC_WEIGHT_ASYNC to indicate feeds async (for reporting) // When simplify we'll take the MAX weight - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { new V3GraphEdge(&m_graph, m_logicVertexp, varvertexp, 1); if (m_inDly) { varvertexp->fromFlop(true); diff --git a/src/V3Delayed.cpp b/src/V3Delayed.cpp index 89fb81dc7..c2f80ca6d 100644 --- a/src/V3Delayed.cpp +++ b/src/V3Delayed.cpp @@ -410,10 +410,11 @@ private: virtual void visit(AstVarRef* nodep) override { if (!nodep->user2Inc()) { // Not done yet - if (m_inDly && nodep->access().isWrite()) { + if (m_inDly && nodep->access().isWriteOrRW()) { UINFO(4, "AssignDlyVar: " << nodep << endl); markVarUsage(nodep->varScopep(), VU_DLY); UASSERT_OBJ(m_activep, nodep, "<= not under sensitivity block"); + UASSERT_OBJ(!nodep->access().isRW(), nodep, "<= on read+write method"); if (!m_activep->hasClocked()) { nodep->v3error("Internal: Blocking <= assignment in non-clocked block, should " "have converted in V3Active"); @@ -458,7 +459,7 @@ private: newrefp->user2(true); // No reason to do it again nodep->replaceWith(newrefp); VL_DO_DANGLING(nodep->deleteTree(), nodep); - } else if (!m_inDly && nodep->access().isWrite()) { + } else if (!m_inDly && nodep->access().isWriteOrRW()) { // UINFO(9, "NBA " << nodep << endl); if (!m_inInitial) { UINFO(4, "AssignNDlyVar: " << nodep << endl); diff --git a/src/V3Gate.cpp b/src/V3Gate.cpp index e5a1ae68a..962baa03d 100644 --- a/src/V3Gate.cpp +++ b/src/V3Gate.cpp @@ -220,7 +220,9 @@ private: if (nodep->varScopep()->varp()->isSc()) { clearSimple("SystemC sig"); // Don't want to eliminate the VL_ASSIGN_SI's } - if (nodep->access().isWrite()) { + if (nodep->access().isRW()) { + clearSimple("R/W"); + } else if (nodep->access().isWriteOrRW()) { if (m_lhsVarRef) clearSimple(">1 lhs varRefs"); m_lhsVarRef = nodep; } else { @@ -457,9 +459,10 @@ private: } // We use weight of one; if we ref the var more than once, when we simplify, // the weight will increase - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { new V3GraphEdge(&m_graph, m_logicVertexp, vvertexp, 1); - } else { + } + if (nodep->access().isReadOrRW()) { new V3GraphEdge(&m_graph, vvertexp, m_logicVertexp, 1); } } diff --git a/src/V3GenClk.cpp b/src/V3GenClk.cpp index 163ab4bc3..0aa972080 100644 --- a/src/V3GenClk.cpp +++ b/src/V3GenClk.cpp @@ -185,7 +185,7 @@ private: UINFO(8, " VarAct " << nodep << endl); vscp->user1(true); } - if (m_assignp && nodep->access().isWrite() && vscp->user1()) { + if (m_assignp && nodep->access().isWriteOrRW() && vscp->user1()) { // Variable was previously used as a clock, and is now being set // Thus a unordered generated clock... UINFO(8, " VarSetAct " << nodep << endl); diff --git a/src/V3Life.cpp b/src/V3Life.cpp index aa8ca95ec..8b9907339 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -300,7 +300,7 @@ private: // AstVarScope* vscp = nodep->varScopep(); UASSERT_OBJ(vscp, nodep, "Scope not assigned"); - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { m_sideEffect = true; // $sscanf etc may have RHS vars that are lvalues m_lifep->complexAssign(vscp); } else { diff --git a/src/V3LifePost.cpp b/src/V3LifePost.cpp index f7fd88a83..d67f20b96 100644 --- a/src/V3LifePost.cpp +++ b/src/V3LifePost.cpp @@ -283,11 +283,8 @@ private: UASSERT_OBJ(vscp, nodep, "Scope not assigned"); LifeLocation loc(m_execMTaskp, ++m_sequence); - if (nodep->access().isWrite()) { - m_writes[vscp].insert(loc); - } else { - m_reads[vscp].insert(loc); - } + if (nodep->access().isWriteOrRW()) m_writes[vscp].insert(loc); + if (nodep->access().isReadOrRW()) m_reads[vscp].insert(loc); } virtual void visit(AstAssignPre* nodep) override { // Do not record varrefs within assign pre. diff --git a/src/V3LinkLValue.cpp b/src/V3LinkLValue.cpp index 453747555..d5a26d0f5 100644 --- a/src/V3LinkLValue.cpp +++ b/src/V3LinkLValue.cpp @@ -35,7 +35,7 @@ private: // NODE STATE // STATE - bool m_setRefLvalue; // Set VarRefs to lvalues for pin assignments + VAccess m_setRefLvalue; // Set VarRefs to lvalues for pin assignments AstNodeFTask* m_ftaskp = nullptr; // Function or task we're inside // METHODS @@ -45,9 +45,9 @@ private: // Result handing virtual void visit(AstNodeVarRef* nodep) override { // VarRef: LValue its reference - if (m_setRefLvalue) nodep->access(VAccess::WRITE); + if (m_setRefLvalue != VAccess::NOCHANGE) nodep->access(m_setRefLvalue); if (nodep->varp()) { - if (nodep->access().isWrite() && !m_ftaskp && nodep->varp()->isReadOnly()) { + if (nodep->access().isWriteOrRW() && !m_ftaskp && nodep->varp()->isReadOnly()) { nodep->v3warn(ASSIGNIN, "Assigning to input/const variable: " << nodep->prettyNameQ()); } @@ -62,7 +62,7 @@ private: // Now that we do, and it's from a output, we know it's a lvalue m_setRefLvalue = VAccess::WRITE; iterateChildren(nodep); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; } else { iterateChildren(nodep); } @@ -72,7 +72,7 @@ private: { m_setRefLvalue = VAccess::WRITE; iterateAndNextNull(nodep->lhsp()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->rhsp()); } } @@ -81,7 +81,7 @@ private: { m_setRefLvalue = VAccess::WRITE; iterateAndNextNull(nodep->lhsp()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->rhsp()); } } @@ -90,7 +90,7 @@ private: { m_setRefLvalue = VAccess::WRITE; iterateAndNextNull(nodep->filep()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->filenamep()); iterateAndNextNull(nodep->modep()); } @@ -100,7 +100,7 @@ private: { m_setRefLvalue = VAccess::WRITE; iterateAndNextNull(nodep->filep()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->filenamep()); } } @@ -181,7 +181,7 @@ private: { m_setRefLvalue = VAccess::WRITE; iterateAndNextNull(nodep->memp()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->filenamep()); iterateAndNextNull(nodep->lsbp()); iterateAndNextNull(nodep->msbp()); @@ -190,7 +190,7 @@ private: virtual void visit(AstValuePlusArgs* nodep) override { VL_RESTORER(m_setRefLvalue); { - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->searchp()); m_setRefLvalue = VAccess::WRITE; iterateAndNextNull(nodep->outp()); @@ -201,14 +201,14 @@ private: { m_setRefLvalue = VAccess::WRITE; iterateAndNextNull(nodep->lhsp()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->fmtp()); } } void prepost_visit(AstNodeTriop* nodep) { VL_RESTORER(m_setRefLvalue); { - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->lhsp()); iterateAndNextNull(nodep->rhsp()); m_setRefLvalue = VAccess::WRITE; @@ -226,7 +226,7 @@ private: { iterateAndNextNull(nodep->lhsp()); // Only set lvalues on the from - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->rhsp()); iterateAndNextNull(nodep->thsp()); } @@ -235,14 +235,14 @@ private: VL_RESTORER(m_setRefLvalue); { // Only set lvalues on the from iterateAndNextNull(nodep->lhsp()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->rhsp()); } } virtual void visit(AstCellArrayRef* nodep) override { VL_RESTORER(m_setRefLvalue); { // selp is not an lvalue - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->selp()); } } @@ -250,7 +250,7 @@ private: VL_RESTORER(m_setRefLvalue); { // Only set lvalues on the from iterateAndNextNull(nodep->lhsp()); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; iterateAndNextNull(nodep->rhsp()); iterateAndNextNull(nodep->thsp()); } @@ -271,7 +271,7 @@ private: if (portp->isWritable()) { m_setRefLvalue = VAccess::WRITE; iterate(pinp); - m_setRefLvalue = false; + m_setRefLvalue = VAccess::NOCHANGE; } else { iterate(pinp); } @@ -286,7 +286,7 @@ private: public: // CONSTRUCTORS - LinkLValueVisitor(AstNode* nodep, bool start) + LinkLValueVisitor(AstNode* nodep, VAccess start) : m_setRefLvalue{start} { iterate(nodep); } @@ -298,12 +298,12 @@ public: void V3LinkLValue::linkLValue(AstNetlist* nodep) { UINFO(4, __FUNCTION__ << ": " << endl); - { LinkLValueVisitor visitor(nodep, false); } // Destruct before checking + { LinkLValueVisitor visitor(nodep, VAccess::NOCHANGE); } // Destruct before checking V3Global::dumpCheckGlobalTree("linklvalue", 0, v3Global.opt.dumpTreeLevel(__FILE__) >= 6); } void V3LinkLValue::linkLValueSet(AstNode* nodep) { // Called by later link functions when it is known a node needs // to be converted to a lvalue. UINFO(9, __FUNCTION__ << ": " << endl); - LinkLValueVisitor visitor(nodep, true); + LinkLValueVisitor visitor(nodep, VAccess::WRITE); } diff --git a/src/V3Localize.cpp b/src/V3Localize.cpp index efa34527c..f412649cf 100644 --- a/src/V3Localize.cpp +++ b/src/V3Localize.cpp @@ -168,7 +168,8 @@ private: for (; nodep; nodep = nodep->nextp()) { if (VN_IS(nodep, NodeAssign)) { if (AstVarRef* varrefp = VN_CAST(VN_CAST(nodep, NodeAssign)->lhsp(), VarRef)) { - UASSERT_OBJ(varrefp->access().isWrite(), varrefp, "LHS assignment not lvalue"); + UASSERT_OBJ(varrefp->access().isWriteOrRW(), varrefp, + "LHS assignment not lvalue"); if (!varrefp->varp()->user4p()) { UINFO(4, " FuncAsn " << varrefp << endl); varrefp->varp()->user4p(varrefp); diff --git a/src/V3MergeCond.cpp b/src/V3MergeCond.cpp index acdfe95b7..e7dcb3955 100644 --- a/src/V3MergeCond.cpp +++ b/src/V3MergeCond.cpp @@ -80,7 +80,7 @@ private: virtual void visit(AstVarRef* nodep) override { if (!m_mergeable) return; // Clear if it's an LValue referencing a marked variable - if (nodep->access().isWrite() && nodep->varp()->user1()) { + if (nodep->access().isWriteOrRW() && nodep->varp()->user1()) { clearMergeable(nodep, "might modify condition"); } } diff --git a/src/V3Order.cpp b/src/V3Order.cpp index 6be9a3fa6..5bf687bf5 100644 --- a/src/V3Order.cpp +++ b/src/V3Order.cpp @@ -1030,7 +1030,7 @@ private: UASSERT_OBJ(varscp, nodep, "Var didn't get varscoped in V3Scope.cpp"); if (m_inSenTree) { // Add CLOCK dependency... This is a root of the tree we'll trace - UASSERT_OBJ(!nodep->access().isWrite(), nodep, + UASSERT_OBJ(!nodep->access().isWriteOrRW(), nodep, "How can a sensitivity be setting a var?"); OrderVarVertex* varVxp = newVarUserVertex(varscp, WV_STD); varVxp->isClock(true); @@ -1041,9 +1041,8 @@ private: // We don't want to add extra edges if the logic block has many usages of same var bool gen = false; bool con = false; - if (nodep->access().isWrite()) { - gen = !(varscp->user4() & VU_GEN); - } else { + if (nodep->access().isWriteOrRW()) gen = !(varscp->user4() & VU_GEN); + if (nodep->access().isReadOrRW()) { con = !(varscp->user4() & VU_CON); if ((varscp->user4() & VU_GEN) && !m_inClocked) { // Dangerous assumption: diff --git a/src/V3Premit.cpp b/src/V3Premit.cpp index 3195e30d0..7e432830a 100644 --- a/src/V3Premit.cpp +++ b/src/V3Premit.cpp @@ -58,7 +58,7 @@ private: } virtual void visit(AstVarRef* nodep) override { // it's LHS var is used so need a deep temporary - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { nodep->varp()->user4(true); } else { if (nodep->varp()->user4()) { diff --git a/src/V3Simulate.h b/src/V3Simulate.h index 84bf595ef..846d303f5 100644 --- a/src/V3Simulate.h +++ b/src/V3Simulate.h @@ -401,7 +401,7 @@ private: && !VN_IS(nodep->varp()->dtypeSkipRefp(), UnpackArrayDType) && !VN_IS(nodep->varp()->dtypeSkipRefp(), StructDType)) clearOptimizable(nodep, "Array references/not basic"); - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { if (m_inDlyAssign) { if (!(vscp->user1() & VU_LVDLY)) { vscp->user1(vscp->user1() | VU_LVDLY); @@ -416,7 +416,8 @@ private: if (m_checkOnly) varRefCb(nodep); } } - } else { + } + if (nodep->access().isReadOrRW()) { if (!(vscp->user1() & VU_RV)) { if (!m_params && (vscp->user1() & VU_LV)) { clearOptimizable(nodep, "Var write & read"); diff --git a/src/V3Split.cpp b/src/V3Split.cpp index 5fc44131a..31ddbd268 100644 --- a/src/V3Split.cpp +++ b/src/V3Split.cpp @@ -383,7 +383,7 @@ protected: SplitVarStdVertex* vstdp = reinterpret_cast(vscp->user1p()); // SPEEDUP: We add duplicate edges, that should be fixed - if (m_inDly && nodep->access().isWrite()) { + if (m_inDly && nodep->access().isWriteOrRW()) { UINFO(4, " VARREFDLY: " << nodep << endl); // Delayed variable is different from non-delayed variable if (!vscp->user2p()) { @@ -398,7 +398,7 @@ protected: new SplitLVEdge(&m_graph, vpostp, vxp); } } else { // Nondelayed assignment - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { // Non-delay; need to maintain existing ordering // with all consumers of the signal UINFO(4, " VARREFLV: " << nodep << endl); diff --git a/src/V3SplitAs.cpp b/src/V3SplitAs.cpp index eb1f1d2d1..48a1c2960 100644 --- a/src/V3SplitAs.cpp +++ b/src/V3SplitAs.cpp @@ -49,7 +49,7 @@ private: // METHODS virtual void visit(AstVarRef* nodep) override { - if (nodep->access().isWrite() && !m_splitVscp && nodep->varp()->attrIsolateAssign()) { + if (nodep->access().isWriteOrRW() && !m_splitVscp && nodep->varp()->attrIsolateAssign()) { m_splitVscp = nodep->varScopep(); } } @@ -76,7 +76,7 @@ private: // METHODS virtual void visit(AstVarRef* nodep) override { - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { if (nodep->varScopep() == m_splitVscp) { UINFO(6, " CL VAR " << nodep << endl); m_matches = true; diff --git a/src/V3SplitVar.cpp b/src/V3SplitVar.cpp index b5bd06e1a..bc635a42a 100644 --- a/src/V3SplitVar.cpp +++ b/src/V3SplitVar.cpp @@ -898,10 +898,8 @@ public: : m_basicp{varp->dtypep()->basicp()} {} void append(const PackedVarRefEntry& e, const VAccess& access) { UASSERT(!m_dedupDone, "cannot add after dedup()"); - if (access.isWrite()) - m_lhs.push_back(e); - else - m_rhs.push_back(e); + if (access.isWriteOrRW()) m_lhs.push_back(e); + if (access.isReadOrRW()) m_rhs.push_back(e); } void dedup() { UASSERT(!m_dedupDone, "dedup() called twice"); diff --git a/src/V3Subst.cpp b/src/V3Subst.cpp index 33f554067..d8a01fbb3 100644 --- a/src/V3Subst.cpp +++ b/src/V3Subst.cpp @@ -329,14 +329,14 @@ private: } virtual void visit(AstVarRef* nodep) override { // Any variable - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { m_assignStep++; nodep->varp()->user2(m_assignStep); UINFO(9, " ASSIGNstep u2=" << nodep->varp()->user2() << " " << nodep << endl); } if (isSubstVar(nodep->varp())) { SubstVarEntry* entryp = getEntryp(nodep); - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { UINFO(8, " ASSIGNcpx " << nodep << endl); entryp->assignComplex(); } else if (AstNode* substp = entryp->substWhole(nodep)) { diff --git a/src/V3Table.cpp b/src/V3Table.cpp index b83c44a68..76428dc32 100644 --- a/src/V3Table.cpp +++ b/src/V3Table.cpp @@ -154,10 +154,11 @@ public: // Called by TableSimulateVisitor on each unique varref encountered UINFO(9, " SimVARREF " << nodep << endl); AstVarScope* vscp = nodep->varScopep(); - if (nodep->access().isWrite()) { + if (nodep->access().isWriteOrRW()) { m_outWidth += nodep->varp()->dtypeSkipRefp()->widthTotalBytes(); m_outVarps.push_back(vscp); - } else { + } + if (nodep->access().isReadOrRW()) { // We'll make the table with a separate natural alignment for each // output var, so always have char, 16 or 32 bit widths, so use widthTotalBytes m_inWidth += nodep->varp()->width(); // Space for var diff --git a/src/V3Trace.cpp b/src/V3Trace.cpp index 01067cb8e..28712534e 100644 --- a/src/V3Trace.cpp +++ b/src/V3Trace.cpp @@ -859,7 +859,7 @@ private: || nodep->varp()->isSigPublic()) { // Or ones user can change new V3GraphEdge(&m_graph, m_alwaysVtxp, traceVtxp, 1); } - } else if (m_cfuncp && m_finding && nodep->access().isWrite()) { + } else if (m_cfuncp && m_finding && nodep->access().isWriteOrRW()) { UASSERT_OBJ(nodep->varScopep(), nodep, "No var scope?"); V3GraphVertex* const funcVtxp = getCFuncVertexp(m_cfuncp); V3GraphVertex* const varVtxp = nodep->varScopep()->user1u().toGraphVertex(); diff --git a/src/V3Tristate.cpp b/src/V3Tristate.cpp index dad10cffc..92131f735 100644 --- a/src/V3Tristate.cpp +++ b/src/V3Tristate.cpp @@ -168,7 +168,7 @@ private: for (V3GraphEdge* edgep = vtxp->inBeginp(); edgep; edgep = edgep->inNextp()) { TristateVertex* vvertexp = dynamic_cast(edgep->fromp()); if (const AstVarRef* refp = VN_CAST(vvertexp->nodep(), VarRef)) { - if (refp->access().isWrite() + if (refp->access().isWriteOrRW() // Doesn't hurt to not check if already set, but by doing so when we // print out the debug messages, we'll see this node at level 0 instead. && !vvertexp->isTristate()) { @@ -276,10 +276,11 @@ class TristatePinVisitor : public TristateBaseVisitor { bool m_lvalue; // Flip to be an LVALUE // VISITORS virtual void visit(AstVarRef* nodep) override { - if (m_lvalue && !nodep->access().isWrite()) { + UASSERT_OBJ(!nodep->access().isRW(), nodep, "Tristate unexpected on R/W access flip"); + if (m_lvalue && !nodep->access().isWriteOrRW()) { UINFO(9, " Flip-to-LValue " << nodep << endl); nodep->access(VAccess::WRITE); - } else if (!m_lvalue && nodep->access().isWrite()) { + } else if (!m_lvalue && !nodep->access().isReadOnly()) { UINFO(9, " Flip-to-RValue " << nodep << endl); nodep->access(VAccess::READ); // Mark the ex-output as tristated @@ -1222,19 +1223,17 @@ class TristateVisitor : public TristateBaseVisitor { virtual void visit(AstVarRef* nodep) override { UINFO(9, dbgState() << nodep << endl); if (m_graphing) { - if (nodep->access().isWrite()) { - associateLogic(nodep, nodep->varp()); - } else { - associateLogic(nodep->varp(), nodep); - } + if (nodep->access().isWriteOrRW()) associateLogic(nodep, nodep->varp()); + if (nodep->access().isReadOrRW()) associateLogic(nodep->varp(), nodep); } else { if (nodep->user2() & U2_NONGRAPH) return; // Processed nodep->user2(U2_NONGRAPH); // Detect all var lhs drivers and adds them to the // VarMap so that after the walk through the module we can expand // any tristate logic on the driver. - if (nodep->access().isWrite() && m_tgraph.isTristate(nodep->varp())) { + if (nodep->access().isWriteOrRW() && m_tgraph.isTristate(nodep->varp())) { UINFO(9, " Ref-to-lvalue " << nodep << endl); + UASSERT_OBJ(!nodep->access().isRW(), nodep, "Tristate unexpected on R/W access"); m_tgraph.didProcess(nodep); mapInsertLhsVarRef(nodep); } else if (nodep->access().isReadOnly() diff --git a/src/V3Undriven.cpp b/src/V3Undriven.cpp index c09336dcd..2e5dc239d 100644 --- a/src/V3Undriven.cpp +++ b/src/V3Undriven.cpp @@ -318,7 +318,7 @@ private: for (int usr = 1; usr < (m_alwaysCombp ? 3 : 2); ++usr) { UndrivenVarEntry* entryp = getEntryp(varrefp->varp(), usr); int lsb = constp->toUInt(); - if (m_inBBox || varrefp->access().isWrite()) { + if (m_inBBox || varrefp->access().isWriteOrRW()) { // Don't warn if already driven earlier as "a=0; if(a) a=1;" is fine. if (usr == 2 && m_alwaysCombp && entryp->isUsedNotDrivenBit(lsb, nodep->width())) { @@ -327,7 +327,8 @@ private: } entryp->drivenBit(lsb, nodep->width()); } - if (m_inBBox || !varrefp->access().isWrite()) entryp->usedBit(lsb, nodep->width()); + if (m_inBBox || !varrefp->access().isWriteOrRW()) + entryp->usedBit(lsb, nodep->width()); } } else { // else other varrefs handled as unknown mess in AstVarRef @@ -336,7 +337,7 @@ private: } virtual void visit(AstNodeVarRef* nodep) override { // Any variable - if (nodep->access().isWrite() + if (nodep->access().isWriteOrRW() && !VN_IS(nodep, VarXRef)) { // Ignore interface variables and similar ugly items if (m_inProcAssign && !nodep->varp()->varType().isProcAssignable() && !nodep->varp()->isDeclTyped() // @@ -355,16 +356,16 @@ private: } for (int usr = 1; usr < (m_alwaysCombp ? 3 : 2); ++usr) { UndrivenVarEntry* entryp = getEntryp(nodep->varp(), usr); - bool fdrv = nodep->access().isWrite() + bool fdrv = nodep->access().isWriteOrRW() && nodep->varp()->attrFileDescr(); // FD's are also being read from - if (m_inBBox || nodep->access().isWrite()) { + if (m_inBBox || nodep->access().isWriteOrRW()) { if (usr == 2 && m_alwaysCombp && entryp->isUsedNotDrivenAny()) { UINFO(9, " Full bus. Entryp=" << cvtToHex(entryp) << endl); warnAlwCombOrder(nodep); } entryp->drivenWhole(); } - if (m_inBBox || !nodep->access().isWrite() || fdrv) entryp->usedWhole(); + if (m_inBBox || nodep->access().isReadOrRW() || fdrv) entryp->usedWhole(); } } diff --git a/src/V3Unknown.cpp b/src/V3Unknown.cpp index 160c161c3..21e61b019 100644 --- a/src/V3Unknown.cpp +++ b/src/V3Unknown.cpp @@ -313,7 +313,7 @@ private: AstNode* basefromp = AstArraySel::baseFromp(nodep); bool lvalue = false; if (const AstNodeVarRef* varrefp = VN_CAST(basefromp, NodeVarRef)) { - lvalue = varrefp->access().isWrite(); + lvalue = varrefp->access().isWriteOrRW(); } // Find range of dtype we are selecting from // Similar code in V3Const::warnSelect @@ -361,7 +361,7 @@ private: AstNode* basefromp = AstArraySel::baseFromp(nodep->fromp()); bool lvalue = false; if (const AstNodeVarRef* varrefp = VN_CAST(basefromp, NodeVarRef)) { - lvalue = varrefp->access().isWrite(); + lvalue = varrefp->access().isWriteOrRW(); } else if (VN_IS(basefromp, Const)) { // If it's a PARAMETER[bit], then basefromp may be a constant instead of a varrefp } else { diff --git a/src/V3Unroll.cpp b/src/V3Unroll.cpp index 38fb4e602..7a9c15e04 100644 --- a/src/V3Unroll.cpp +++ b/src/V3Unroll.cpp @@ -445,7 +445,7 @@ private: virtual void visit(AstVarRef* nodep) override { if (m_varModeCheck && nodep->varp() == m_forVarp && nodep->varScopep() == m_forVscp - && nodep->access().isWrite()) { + && nodep->access().isWriteOrRW()) { UINFO(8, " Itervar assigned to: " << nodep << endl); m_varAssignHit = true; } diff --git a/src/V3Width.cpp b/src/V3Width.cpp index f4a49629d..bed690e32 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -1870,13 +1870,13 @@ private: // nodep->varp()->dumpTree(cout, " forvar "); } // Note genvar's are also entered as integers nodep->dtypeFrom(nodep->varp()); - if (VN_IS(nodep->backp(), NodeAssign) && nodep->access().isWrite()) { // On LHS + if (VN_IS(nodep->backp(), NodeAssign) && nodep->access().isWriteOrRW()) { // On LHS UASSERT_OBJ(nodep->dtypep(), nodep, "LHS var should be dtype completed"); } // if (debug() >= 9) nodep->dumpTree(cout, " VRout "); - if (nodep->access().isWrite() && nodep->varp()->direction() == VDirection::CONSTREF) { + if (nodep->access().isWriteOrRW() && nodep->varp()->direction() == VDirection::CONSTREF) { nodep->v3error("Assigning to const ref variable: " << nodep->prettyNameQ()); - } else if (nodep->access().isWrite() && nodep->varp()->isConst() && !m_paramsOnly + } else if (nodep->access().isWriteOrRW() && nodep->varp()->isConst() && !m_paramsOnly && (!m_ftaskp || !m_ftaskp->isConstructor()) && !VN_IS(m_procedurep, Initial)) { // Too loose, but need to allow our generated first assignment // Move this to a property of the AstInitial block @@ -2754,7 +2754,8 @@ private: } } else if (nodep->name() == "pop_front" || nodep->name() == "pop_back") { methodOkArguments(nodep, 0, 0); - methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::WRITE); + // Returns element, so method both consumes (reads) and modifies the queue + methodCallLValueRecurse(nodep, nodep->fromp(), VAccess::READWRITE); newp = new AstCMethodHard(nodep->fileline(), nodep->fromp()->unlinkFrBack(), nodep->name(), nullptr); newp->dtypeFrom(adtypep->subDTypep()); diff --git a/test_regress/t/t_queue_pushpop.pl b/test_regress/t/t_queue_pushpop.pl new file mode 100755 index 000000000..b46d46042 --- /dev/null +++ b/test_regress/t/t_queue_pushpop.pl @@ -0,0 +1,21 @@ +#!/usr/bin/env 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. +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + ); + +execute( + check_finished => 1, + ); + +ok(1); +1; diff --git a/test_regress/t/t_queue_pushpop.v b/test_regress/t/t_queue_pushpop.v new file mode 100644 index 000000000..acad0a3a2 --- /dev/null +++ b/test_regress/t/t_queue_pushpop.v @@ -0,0 +1,42 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2020 by Wilson Snyder. +// SPDX-License-Identifier: CC0-1.0 + +`define stop $stop +`define checkh(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='h%x exp='h%x\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); +`define checks(gotv,expv) do if ((gotv) !== (expv)) begin $write("%%Error: %s:%0d: got='%s' exp='%s'\n", `__FILE__,`__LINE__, (gotv), (expv)); `stop; end while(0); + +module t (/*AUTOARG*/ + // Inputs + clk + ); + input clk; + + logic q [$]; + int cycle = 0; + + always @(posedge clk) begin + cycle <= cycle + 1'b1; + end + + always @(posedge clk) begin + q.push_front(1'b1); + end + + // Important this is a separate always to expose bug where "q" thought unused + always @(posedge clk) begin + if (cycle == 1) begin + if (q.pop_back() != 1) $stop; + end + end + + always @(posedge clk) begin + if (cycle == 2) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + +endmodule