From 0e4da3b0bf9394a61fd8e3eb209d619ae62385cd Mon Sep 17 00:00:00 2001 From: Arkadiusz Kozdra Date: Thu, 20 Oct 2022 12:31:00 +0200 Subject: [PATCH] Support virtual interfaces (#3654) --- src/V3AstNodeDType.h | 9 +++ src/V3AstNodeOther.h | 4 ++ src/V3AstNodes.cpp | 2 + src/V3Common.cpp | 15 ++++ src/V3Const.cpp | 2 +- src/V3Dead.cpp | 15 +++- src/V3EmitCFunc.cpp | 2 + src/V3EmitCFunc.h | 3 + src/V3Life.cpp | 4 +- src/V3LinkDot.cpp | 26 ++++--- src/V3Localize.cpp | 1 + src/V3Scope.cpp | 26 ++++--- src/V3Width.cpp | 65 ++++++++++++++++- src/verilog.y | 30 ++++---- test_regress/t/t_class_unsup_bad.out | 7 -- test_regress/t/t_interface_virtual.out | 7 ++ test_regress/t/t_interface_virtual.pl | 22 ++++++ test_regress/t/t_interface_virtual.v | 72 +++++++++++++++++++ test_regress/t/t_interface_virtual_bad.out | 34 +++++++++ test_regress/t/t_interface_virtual_bad.pl | 19 +++++ test_regress/t/t_interface_virtual_bad.v | 49 +++++++++++++ test_regress/t/t_interface_virtual_inl.pl | 27 +++++++ .../t/t_interface_virtual_unused_bad.out | 5 ++ .../t/t_interface_virtual_unused_bad.pl | 19 +++++ .../t/t_interface_virtual_unused_bad.v | 20 ++++++ 25 files changed, 440 insertions(+), 45 deletions(-) create mode 100644 test_regress/t/t_interface_virtual.out create mode 100755 test_regress/t/t_interface_virtual.pl create mode 100644 test_regress/t/t_interface_virtual.v create mode 100644 test_regress/t/t_interface_virtual_bad.out create mode 100755 test_regress/t/t_interface_virtual_bad.pl create mode 100644 test_regress/t/t_interface_virtual_bad.v create mode 100755 test_regress/t/t_interface_virtual_inl.pl create mode 100644 test_regress/t/t_interface_virtual_unused_bad.out create mode 100755 test_regress/t/t_interface_virtual_unused_bad.pl create mode 100644 test_regress/t/t_interface_virtual_unused_bad.v diff --git a/src/V3AstNodeDType.h b/src/V3AstNodeDType.h index 736eefd8a..c98b10843 100644 --- a/src/V3AstNodeDType.h +++ b/src/V3AstNodeDType.h @@ -768,6 +768,7 @@ public: }; class AstIfaceRefDType final : public AstNodeDType { // Reference to an interface, either for a port, or inside parent cell + // @astgen op1 := paramsp : List[AstPin] private: FileLine* m_modportFileline; // Where modport token was string m_cellName; // "" = no cell, such as when connects to 'input' iface @@ -790,6 +791,14 @@ public: , m_cellName{cellName} , m_ifaceName{ifaceName} , m_modportName{modport} {} + AstIfaceRefDType(FileLine* fl, FileLine* modportFl, const string& cellName, + const string& ifaceName, const string& modport, AstPin* paramsp) + : ASTGEN_SUPER_IfaceRefDType(fl) + , m_modportFileline{modportFl} + , m_cellName{cellName} + , m_ifaceName{ifaceName} { + addParamsp(paramsp); + } ASTGEN_MEMBERS_AstIfaceRefDType; // METHODS const char* broken() const override; diff --git a/src/V3AstNodeOther.h b/src/V3AstNodeOther.h index ec4612587..5d3746ebe 100644 --- a/src/V3AstNodeOther.h +++ b/src/V3AstNodeOther.h @@ -2002,6 +2002,7 @@ class AstVar final : public AstNode { bool m_usedClock : 1; // Signal used as a clock bool m_usedParam : 1; // Parameter is referenced (on link; later signals not setup) bool m_usedLoopIdx : 1; // Variable subject of for unrolling + bool m_usedVirtIface : 1; // Signal used through a virtual interface bool m_funcLocal : 1; // Local variable for a function bool m_funcReturn : 1; // Return variable for a function bool m_attrScBv : 1; // User force bit vector attribute @@ -2040,6 +2041,7 @@ class AstVar final : public AstNode { m_usedClock = false; m_usedParam = false; m_usedLoopIdx = false; + m_usedVirtIface = false; m_sigPublic = false; m_sigModPublic = false; m_sigUserRdPublic = false; @@ -2188,6 +2190,7 @@ public: void usedClock(bool flag) { m_usedClock = flag; } void usedParam(bool flag) { m_usedParam = flag; } void usedLoopIdx(bool flag) { m_usedLoopIdx = flag; } + void usedVirtIface(bool flag) { m_usedVirtIface = flag; } void sigPublic(bool flag) { m_sigPublic = flag; } void sigModPublic(bool flag) { m_sigModPublic = flag; } void sigUserRdPublic(bool flag) { @@ -2271,6 +2274,7 @@ public: bool isUsedClock() const { return m_usedClock; } bool isUsedParam() const { return m_usedParam; } bool isUsedLoopIdx() const { return m_usedLoopIdx; } + bool isUsedVirtIface() const { return m_usedVirtIface; } bool isSc() const VL_MT_SAFE { return m_sc; } bool isScQuad() const; bool isScBv() const; diff --git a/src/V3AstNodes.cpp b/src/V3AstNodes.cpp index 10861568f..2a3a4d975 100644 --- a/src/V3AstNodes.cpp +++ b/src/V3AstNodes.cpp @@ -724,6 +724,8 @@ AstNodeDType::CTypeRecursed AstNodeDType::cTypeRecurse(bool compound) const { info.m_type += ">"; } else if (const auto* const adtypep = VN_CAST(dtypep, ClassRefDType)) { info.m_type = "VlClassRef<" + EmitCBaseVisitor::prefixNameProtect(adtypep) + ">"; + } else if (const auto* const adtypep = VN_CAST(dtypep, IfaceRefDType)) { + info.m_type = EmitCBaseVisitor::prefixNameProtect(adtypep->ifaceViaCellp()) + "*"; } else if (const auto* const adtypep = VN_CAST(dtypep, UnpackArrayDType)) { if (adtypep->isCompound()) compound = true; const CTypeRecursed sub = adtypep->subDTypep()->cTypeRecurse(compound); diff --git a/src/V3Common.cpp b/src/V3Common.cpp index 9961585b9..907c5a33d 100644 --- a/src/V3Common.cpp +++ b/src/V3Common.cpp @@ -47,6 +47,19 @@ static void makeVlToString(AstClass* nodep) { funcp->addStmtsp(new AstCReturn{nodep->fileline(), exprp}); nodep->addStmtsp(funcp); } +static void makeVlToString(AstIface* nodep) { + AstCFunc* const funcp + = new AstCFunc{nodep->fileline(), "VL_TO_STRING", nullptr, "std::string"}; + funcp->argTypes("const " + EmitCBaseVisitor::prefixNameProtect(nodep) + "* obj"); + funcp->isMethod(false); + funcp->isConst(false); + funcp->isStatic(false); + funcp->protect(false); + AstNode* const exprp = new AstCMath{nodep->fileline(), "obj ? obj->name() : \"null\"", 0}; + exprp->dtypeSetString(); + funcp->addStmtsp(new AstCReturn{nodep->fileline(), exprp}); + nodep->addStmtsp(funcp); +} static void makeToString(AstClass* nodep) { AstCFunc* const funcp = new AstCFunc{nodep->fileline(), "to_string", nullptr, "std::string"}; funcp->isConst(true); @@ -117,6 +130,8 @@ void V3Common::commonAll() { makeVlToString(classp); makeToString(classp); makeToStringMiddle(classp); + } else if (AstIface* const ifacep = VN_CAST(nodep, Iface)) { + makeVlToString(ifacep); } } V3Global::dumpCheckGlobalTree("common", 0, dumpTree() >= 3); diff --git a/src/V3Const.cpp b/src/V3Const.cpp index cf90d2156..b499a1ed4 100644 --- a/src/V3Const.cpp +++ b/src/V3Const.cpp @@ -2606,7 +2606,7 @@ private: && m_doNConst && v3Global.opt.fConst() // Default value, not a "known" constant for this usage - && !nodep->varp()->isClassMember() + && !nodep->varp()->isClassMember() && !nodep->varp()->isUsedVirtIface() && !(nodep->varp()->isFuncLocal() && nodep->varp()->isNonOutput()) && !nodep->varp()->noSubst() && !nodep->varp()->isSigPublic()) || nodep->varp()->isParam())) { diff --git a/src/V3Dead.cpp b/src/V3Dead.cpp index 3c396f080..d2ce5734c 100644 --- a/src/V3Dead.cpp +++ b/src/V3Dead.cpp @@ -198,6 +198,19 @@ private: } if (nodep->classp()) nodep->classp()->user1Inc(); } + void visit(AstIfaceRefDType* nodep) override { + iterateChildren(nodep); + checkDType(nodep); + checkAll(nodep); + if (nodep->modportp()) { + if (m_elimCells) { + nodep->modportp(nullptr); + } else { + nodep->modportp()->user1Inc(); + } + } + if (nodep->ifaceViaCellp()) nodep->ifaceViaCellp()->user1Inc(); + } void visit(AstNodeDType* nodep) override { iterateChildren(nodep); checkDType(nodep); @@ -321,7 +334,7 @@ private: } bool mightElimVar(AstVar* nodep) const { if (nodep->isSigPublic()) return false; // Can't elim publics! - if (nodep->isIO() || nodep->isClassMember()) return false; + if (nodep->isIO() || nodep->isClassMember() || nodep->isUsedVirtIface()) return false; if (nodep->isTemp() && !nodep->isTrace()) return true; return m_elimUserVars; // Post-Trace can kill most anything } diff --git a/src/V3EmitCFunc.cpp b/src/V3EmitCFunc.cpp index e1b35f8ae..7ec97f234 100644 --- a/src/V3EmitCFunc.cpp +++ b/src/V3EmitCFunc.cpp @@ -663,6 +663,8 @@ string EmitCFunc::emitVarResetRecurse(const AstVar* varp, const string& varNameP suffix + ".atDefault()" + cvtarray); } else if (VN_IS(dtypep, ClassRefDType)) { return ""; // Constructor does it + } else if (VN_IS(dtypep, IfaceRefDType)) { + return varNameProtected + suffix + " = nullptr;\n"; } else if (const AstDynArrayDType* const adtypep = VN_CAST(dtypep, DynArrayDType)) { // Access std::array as C array const string cvtarray = (adtypep->subDTypep()->isWide() ? ".data()" : ""); diff --git a/src/V3EmitCFunc.h b/src/V3EmitCFunc.h index 166dd60de..c91ef4174 100644 --- a/src/V3EmitCFunc.h +++ b/src/V3EmitCFunc.h @@ -1145,6 +1145,9 @@ public: } else if (VN_IS(varModp, Class) && varModp != m_modp) { // Superclass member reference puts(prefixNameProtect(varModp) + "::"); + } else if (varp->isIfaceRef()) { + puts(nodep->selfPointerProtect(m_useSelfForThis)); + return; } else if (!nodep->selfPointer().empty()) { emitDereference(nodep->selfPointerProtect(m_useSelfForThis)); } diff --git a/src/V3Life.cpp b/src/V3Life.cpp index 41d3ab982..1c4bea7df 100644 --- a/src/V3Life.cpp +++ b/src/V3Life.cpp @@ -141,7 +141,7 @@ public: void checkRemoveAssign(const LifeMap::iterator& it) { const AstVar* const varp = it->first->varp(); LifeVarEntry* const entp = &(it->second); - if (!varp->isSigPublic()) { + if (!varp->isSigPublic() && !varp->isUsedVirtIface()) { // Rather than track what sigs AstUCFunc/AstUCStmt may change, // we just don't optimize any public sigs // Check the var entry, and remove if appropriate @@ -186,7 +186,7 @@ public: const auto it = m_map.find(nodep); if (it != m_map.end()) { if (AstConst* const constp = it->second.constNodep()) { - if (!varrefp->varp()->isSigPublic()) { + if (!varrefp->varp()->isSigPublic() && !varrefp->varp()->isUsedVirtIface()) { // Aha, variable is constant; substitute in. // We'll later constant propagate UINFO(4, " replaceconst: " << varrefp << endl); diff --git a/src/V3LinkDot.cpp b/src/V3LinkDot.cpp index 6c3885ff1..3879c918a 100644 --- a/src/V3LinkDot.cpp +++ b/src/V3LinkDot.cpp @@ -459,7 +459,7 @@ public: } void computeIfaceVarSyms() { for (VSymEnt* varSymp : m_ifaceVarSyms) { - const AstVar* const varp = varSymp ? VN_AS(varSymp->nodep(), Var) : nullptr; + AstVar* const varp = varSymp ? VN_AS(varSymp->nodep(), Var) : nullptr; UINFO(9, " insAllIface se" << cvtToHex(varSymp) << " " << varp << endl); AstIfaceRefDType* const ifacerefp = ifaceRefFromArray(varp->subDTypep()); UASSERT_OBJ(ifacerefp, varp, "Non-ifacerefs on list!"); @@ -475,8 +475,16 @@ public: ifacerefp->v3fatalSrc("Unlinked interface"); } } else if (ifacerefp->ifaceViaCellp()->dead()) { - ifacerefp->v3error("Parent instance's interface is not found: " - << AstNode::prettyNameQ(ifacerefp->ifaceName())); + if (varp->isIfaceRef()) { + ifacerefp->v3error("Parent instance's interface is not found: " + << AstNode::prettyNameQ(ifacerefp->ifaceName())); + } else { + ifacerefp->v3warn( + E_UNSUPPORTED, + "Unsupported: virtual interface never assigned any actual interface"); + varp->dtypep(ifacerefp->findCHandleDType()); + VL_DO_DANGLING(ifacerefp->unlinkFrBack()->deleteTree(), ifacerefp); + } continue; } VSymEnt* const ifaceSymp = getNodeSym(ifacerefp->ifaceViaCellp()); @@ -2420,7 +2428,7 @@ private: m_ds.m_dotSymp = foundp; m_ds.m_dotPos = DP_SCOPE; // Upper AstDot visitor will handle it from here - } else if (VN_IS(foundp->nodep(), Cell) && allowVar && m_cellp) { + } else if (VN_IS(foundp->nodep(), Cell) && allowVar) { AstCell* const cellp = VN_AS(foundp->nodep(), Cell); if (VN_IS(cellp->modp(), Iface)) { // Interfaces can be referenced like a variable for interconnect @@ -2438,7 +2446,7 @@ private: m_ds.m_dotPos = DP_SCOPE; UINFO(9, " cell -> iface varref " << foundp->nodep() << endl); AstNode* const newp - = new AstVarRef(ifaceRefVarp->fileline(), ifaceRefVarp, VAccess::READ); + = new AstVarRef{nodep->fileline(), ifaceRefVarp, VAccess::READ}; nodep->replaceWith(newp); VL_DO_DANGLING(pushDeletep(nodep), nodep); } else if (VN_IS(cellp->modp(), NotFoundModule)) { @@ -2449,7 +2457,7 @@ private: } else if (AstVar* const varp = foundToVarp(foundp, nodep, VAccess::READ)) { AstIfaceRefDType* const ifacerefp = LinkDotState::ifaceRefFromArray(varp->subDTypep()); - if (ifacerefp) { + if (ifacerefp && varp->isIfaceRef()) { UASSERT_OBJ(ifacerefp->ifaceViaCellp(), ifacerefp, "Unlinked interface"); // Really this is a scope reference into an interface UINFO(9, "varref-ifaceref " << m_ds.m_dotText << " " << nodep << endl); @@ -2538,9 +2546,9 @@ private: m_ds.m_dotText = VString::dot(m_ds.m_dotText, ".", nodep->name()); m_ds.m_dotSymp = foundp; m_ds.m_dotPos = DP_SCOPE; - UINFO(9, " cell -> iface varref " << foundp->nodep() << endl); - AstNode* newp - = new AstVarRef(ifaceRefVarp->fileline(), ifaceRefVarp, VAccess::READ); + UINFO(9, " modport -> iface varref " << foundp->nodep() << endl); + // We lose the modport name here, so we cannot detect mismatched modports. + AstNode* newp = new AstVarRef{nodep->fileline(), ifaceRefVarp, VAccess::READ}; auto* const cellarrayrefp = VN_CAST(m_ds.m_unlinkedScopep, CellArrayRef); if (cellarrayrefp) { // iface[vec].modport became CellArrayRef(iface, lsb) diff --git a/src/V3Localize.cpp b/src/V3Localize.cpp index 0d5977dcc..1de5d06e3 100644 --- a/src/V3Localize.cpp +++ b/src/V3Localize.cpp @@ -169,6 +169,7 @@ private: && !nodep->varp()->isFuncLocal() // Not already a function local (e.g.: argument) && !nodep->varp()->isStatic() // Not a static variable && !nodep->varp()->isClassMember() // Statically exists in design hierarchy + && !nodep->varp()->isUsedVirtIface() // Not used through a virtual interface && !nodep->varp()->valuep() // Does not have an initializer ) { UINFO(4, "Consider for localization: " << nodep << endl); diff --git a/src/V3Scope.cpp b/src/V3Scope.cpp index c0694e2ab..ce60b19de 100644 --- a/src/V3Scope.cpp +++ b/src/V3Scope.cpp @@ -43,6 +43,7 @@ class ScopeVisitor final : public VNVisitor { private: // NODE STATE // AstVar::user1p -> AstVarScope replacement for this variable + // AstCell::user2p -> AstScope*. The scope created inside the cell // AstTask::user2p -> AstTask*. Replacement task const VNUser1InUse m_inuser1; const VNUser2InUse m_inuser2; @@ -125,6 +126,10 @@ private: AstNodeModule* const modp = cellp->modp(); UASSERT_OBJ(modp, cellp, "Unlinked mod"); iterate(modp); // Recursive call to visit(AstNodeModule) + if (VN_IS(modp, Iface)) { + // Remember newly created scope + cellp->user2p(m_scopep); + } } } } @@ -261,7 +266,12 @@ private: void visit(AstVar* nodep) override { // Make new scope variable if (!nodep->user1p()) { - AstVarScope* const varscp = new AstVarScope(nodep->fileline(), m_scopep, nodep); + AstScope* scopep = m_scopep; + if (AstIfaceRefDType* const ifacerefp = VN_CAST(nodep->dtypep(), IfaceRefDType)) { + // Attach every non-virtual interface variable its inner scope + if (ifacerefp->cellp()) scopep = VN_AS(ifacerefp->cellp()->user2p(), Scope); + } + AstVarScope* const varscp = new AstVarScope{nodep->fileline(), scopep, nodep}; UINFO(6, " New scope " << varscp << endl); if (m_aboveCellp && !m_aboveCellp->isTrace()) varscp->trace(false); nodep->user1p(varscp); @@ -280,15 +290,11 @@ private: // VarRef needs to point to VarScope // Make sure variable has made user1p. UASSERT_OBJ(nodep->varp(), nodep, "Unlinked"); - if (nodep->varp()->isIfaceRef()) { - nodep->varScopep(nullptr); - } else { - // We may have not made the variable yet, and we can't make it now as - // the var's referenced package etc might not be created yet. - // So push to a list and post-correct. - // No check here for nodep->classOrPackagep(), will check when walk list. - m_varRefScopes.emplace(nodep, m_scopep); - } + // We may have not made the variable yet, and we can't make it now as + // the var's referenced package etc might not be created yet. + // So push to a list and post-correct. + // No check here for nodep->classOrPackagep(), will check when walk list. + m_varRefScopes.emplace(nodep, m_scopep); } void visit(AstScopeName* nodep) override { // If there's a %m in the display text, we add a special node that will contain the name() diff --git a/src/V3Width.cpp b/src/V3Width.cpp index a2ec63a30..23319a0ef 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -2527,6 +2527,20 @@ private: << foundp->warnOther() << "... Location of found object\n" << foundp->warnContextSecondary()); } + } else if (AstIfaceRefDType* const adtypep = VN_CAST(fromDtp, IfaceRefDType)) { + if (AstNode* const foundp = memberSelIface(nodep, adtypep)) { + if (AstVar* const varp = VN_CAST(foundp, Var)) { + nodep->dtypep(foundp->dtypep()); + nodep->varp(varp); + varp->usedVirtIface(true); + return; + } + UINFO(1, "found object " << foundp << endl); + nodep->v3fatalSrc("MemberSel of non-variable\n" + << nodep->warnContextPrimary() << '\n' + << foundp->warnOther() << "... Location of found object\n" + << foundp->warnContextSecondary()); + } } else if (VN_IS(fromDtp, EnumDType) // || VN_IS(fromDtp, AssocArrayDType) // || VN_IS(fromDtp, WildcardArrayDType) // @@ -2576,6 +2590,26 @@ private: << (suggest.empty() ? "" : nodep->fileline()->warnMore() + suggest)); return nullptr; // Caller handles error } + AstNode* memberSelIface(AstMemberSel* nodep, AstIfaceRefDType* adtypep) { + // Returns node if ok + // No need to width-resolve the interface, as it was done when we did the child + AstNodeModule* const ifacep = adtypep->ifacep(); + UASSERT_OBJ(ifacep, nodep, "Unlinked"); + // if (AstNode* const foundp = ifacep->findMember(nodep->name())) return foundp; + VSpellCheck speller; + for (AstNode* itemp = ifacep->stmtsp(); itemp; itemp = itemp->nextp()) { + if (itemp->name() == nodep->name()) return itemp; + if (VN_IS(itemp, Var) || VN_IS(itemp, Modport)) { + speller.pushCandidate(itemp->prettyName()); + } + } + const string suggest = speller.bestCandidateMsg(nodep->prettyName()); + nodep->v3error( + "Member " << nodep->prettyNameQ() << " not found in interface " + << ifacep->prettyNameQ() << "\n" + << (suggest.empty() ? "" : nodep->fileline()->warnMore() + suggest)); + return nullptr; // Caller handles error + } bool memberSelStruct(AstMemberSel* nodep, AstNodeUOrStructDType* adtypep) { // Returns true if ok if (AstMemberDType* const memberp = adtypep->findMember(nodep->name())) { @@ -6026,11 +6060,38 @@ private: // Note the check uses the expected size, not the child's subDTypep as we want the // child node's width to end up correct for the assignment (etc) widthCheckSized(nodep, side, underp, expDTypep, extendRule, warnOn); - } else if (!VN_IS(expDTypep, IfaceRefDType) - && VN_IS(underp->dtypep(), IfaceRefDType)) { + } else if (!VN_IS(expDTypep->skipRefp(), IfaceRefDType) + && VN_IS(underp->dtypep()->skipRefp(), IfaceRefDType)) { underp->v3error(ucfirst(nodep->prettyOperatorName()) << " expected non-interface on " << side << " but '" << underp->name() << "' is an interface."); + } else if (const AstIfaceRefDType* expIfaceRefp + = VN_CAST(expDTypep->skipRefp(), IfaceRefDType)) { + const AstIfaceRefDType* underIfaceRefp + = VN_CAST(underp->dtypep()->skipRefp(), IfaceRefDType); + if (!underIfaceRefp) { + underp->v3error(ucfirst(nodep->prettyOperatorName()) + << " expected " << expIfaceRefp->ifaceViaCellp()->prettyNameQ() + << " interface on " << side << " but " << underp->prettyNameQ() + << " is not an interface."); + } else if (expIfaceRefp->ifaceViaCellp() != underIfaceRefp->ifaceViaCellp()) { + underp->v3error(ucfirst(nodep->prettyOperatorName()) + << " expected " << expIfaceRefp->ifaceViaCellp()->prettyNameQ() + << " interface on " << side << " but '" << underp->name() + << "' is a different interface (" + << underIfaceRefp->ifaceViaCellp()->prettyNameQ() << ")."); + } else if (underIfaceRefp->modportp() + && expIfaceRefp->modportp() != underIfaceRefp->modportp()) { + underp->v3error(ucfirst(nodep->prettyOperatorName()) + << " expected " + << (expIfaceRefp->modportp() + ? expIfaceRefp->modportp()->prettyNameQ() + : "no") + << " interface modport on " << side << " but got " + << underIfaceRefp->modportp()->prettyNameQ() << " modport."); + } else { + underp = userIterateSubtreeReturnEdits(underp, WidthVP{expDTypep, FINAL}.p()); + } } else { // Hope it just works out (perhaps a cast will deal with it) underp = userIterateSubtreeReturnEdits(underp, WidthVP(expDTypep, FINAL).p()); diff --git a/src/verilog.y b/src/verilog.y index f366c5b61..256c530db 100644 --- a/src/verilog.y +++ b/src/verilog.y @@ -1402,9 +1402,9 @@ port: // ==IEEE: port VARDTYPE(new AstIfaceRefDType($2, $4, "", *$2, *$4)); addNextNull($$, VARDONEP($$,$6,$7)); } | portDirNetE yINTERFACE portSig rangeListE sigAttrListE - { $$ = nullptr; BBUNSUP($2, "Unsupported: virtual or generic interfaces"); } + { $$ = nullptr; BBUNSUP($2, "Unsupported: generic interfaces"); } | portDirNetE yINTERFACE '.' idAny/*modport*/ portSig rangeListE sigAttrListE - { $$ = nullptr; BBUNSUP($2, "Unsupported: virtual or generic interfaces"); } + { $$ = nullptr; BBUNSUP($2, "Unsupported: generic interfaces"); } // // // IEEE: ansi_port_declaration, with [port_direction] removed // // IEEE: [ net_port_header | interface_port_header ] @@ -1981,21 +1981,25 @@ data_typeNoRef: // ==IEEE: data_type, excluding class_ty { $$ = new AstBasicDType{$1, VBasicDTypeKwd::CHANDLE}; } | yEVENT { $$ = new AstBasicDType{$1, VBasicDTypeKwd::EVENT}; v3Global.setHasEvents(); } - // // Rules overlap virtual_interface_declaration - // // Parameters here are SV2009 - // // IEEE has ['.' modport] but that will conflict with port - // // declarations which decode '.' modport themselves, so - // // instead see data_typeVar - | yVIRTUAL__INTERFACE yINTERFACE id/*interface*/ - { $$ = new AstBasicDType{$1, VBasicDTypeKwd::CHANDLE}; - BBUNSUP($1, "Unsupported: virtual interface"); } - | yVIRTUAL__anyID id/*interface*/ - { $$ = new AstBasicDType{$1, VBasicDTypeKwd::CHANDLE}; - BBUNSUP($1, "Unsupported: virtual data type"); } | type_reference { $$ = $1; } // // IEEE: class_scope: see data_type above // // IEEE: class_type: see data_type above // // IEEE: ps_covergroup: see data_type above + // // Rules overlap virtual_interface_declaration + | yVIRTUAL__INTERFACE yINTERFACE data_typeVirtual + { $$ = $3; } + | yVIRTUAL__anyID data_typeVirtual + { $$ = $2; } + ; + +data_typeVirtual: // ==IEEE: data_type after yVIRTUAL [ yINTERFACE ] + // // Parameters here are SV2009 + id/*interface*/ { $$ = new AstIfaceRefDType{$1, "", *$1}; } + | id/*interface*/ '.' id/*modport*/ { $$ = new AstIfaceRefDType{$1, $3, "", *$1, *$3}; } + | id/*interface*/ parameter_value_assignmentClass + { $$ = new AstIfaceRefDType{$1, nullptr, "", *$1, "", $2}; } + | id/*interface*/ parameter_value_assignmentClass '.' id/*modport*/ + { $$ = new AstIfaceRefDType{$1, $4, "", *$1, *$4, $2}; } ; data_type_or_void: // ==IEEE: data_type_or_void diff --git a/test_regress/t/t_class_unsup_bad.out b/test_regress/t/t_class_unsup_bad.out index eba10d537..4e3bc999f 100644 --- a/test_regress/t/t_class_unsup_bad.out +++ b/test_regress/t/t_class_unsup_bad.out @@ -1,10 +1,3 @@ -%Error-UNSUPPORTED: t/t_class_unsup_bad.v:7:1: Unsupported: virtual interface - 7 | virtual interface vi_t vi; - | ^~~~~~~ - ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest -%Error-UNSUPPORTED: t/t_class_unsup_bad.v:8:1: Unsupported: virtual data type - 8 | virtual vi_t vi2; - | ^~~~~~~ %Error: t/t_class_unsup_bad.v:29:24: Syntax error: 'const'/'rand'/'randc' not allowed before function/task declaration 29 | const function void func_const; endfunction | ^~~~~~~~~~ diff --git a/test_regress/t/t_interface_virtual.out b/test_regress/t/t_interface_virtual.out new file mode 100644 index 000000000..2e63964b4 --- /dev/null +++ b/test_regress/t/t_interface_virtual.out @@ -0,0 +1,7 @@ +va.addr=aa va.data=11 ia.addr=aa ia.data=11 +vb.addr=bb vb.data=22 ib.addr=bb ib.data=22 +ca.fa.addr=a0 ca.fa.data=11 ca.fa.addr=b0 ca.fb.data=22 +cb.fa.addr=b0 cb.fa.data=22 cb.fa.addr=a0 cb.fb.data=11 +gen.x[0].addr=a0 gen.x[1].addr=b0 +gen='{x:'{top.t.ia, top.t.ib, null, null} } +*-* All Finished *-* diff --git a/test_regress/t/t_interface_virtual.pl b/test_regress/t/t_interface_virtual.pl new file mode 100755 index 000000000..6247bd126 --- /dev/null +++ b/test_regress/t/t_interface_virtual.pl @@ -0,0 +1,22 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_virtual.v b/test_regress/t/t_interface_virtual.v new file mode 100644 index 000000000..878df0995 --- /dev/null +++ b/test_regress/t/t_interface_virtual.v @@ -0,0 +1,72 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Arkadiusz Kozdra. +// SPDX-License-Identifier: CC0-1.0 + +// See also t_interface_virtual_bad.v + +interface PBus; + logic req, grant; + logic [7:0] addr, data; + modport phy(input addr, ref data); +endinterface + +typedef virtual PBus vpbus_t; +typedef vpbus_t vpbus2_t; + +class Cls; + vpbus2_t fa, fb; +endclass + +class Clsgen#(type T = logic); + T x[0:3]; +endclass + +module t (/*AUTOARG*/); + + PBus ia, ib; + virtual PBus va, vb; + virtual PBus.phy pa, pb; + Cls ca, cb; + Clsgen#(virtual PBus) gen; + + initial begin + va = ia; + vb = ib; + + ca = new; + cb = new; + gen = new; + + va.addr = 8'haa; + ia.data = 8'h11; + + vb.addr = 8'hbb; + ib.data = 8'h22; + + $display("va.addr=%x", va.addr, " va.data=%x", va.data, " ia.addr=%x", ia.addr, " ia.data=%x", ia.data); + $display("vb.addr=%x", vb.addr, " vb.data=%x", vb.data, " ib.addr=%x", ib.addr, " ib.data=%x", ib.data); + + ca.fa = ia; + ca.fb = ib; + cb.fa = ib; + cb.fb = ia; + gen.x[0] = va; + gen.x[1] = vb; + + pa = va; + pb = vb; + + pb.addr = 8'hb0; + pa.addr = 8'ha0; + + $display("ca.fa.addr=%x", ca.fa.addr, " ca.fa.data=%x", ca.fa.data, " ca.fa.addr=%x", ca.fb.addr, " ca.fb.data=%x", ca.fb.data); + $display("cb.fa.addr=%x", cb.fa.addr, " cb.fa.data=%x", cb.fa.data, " cb.fa.addr=%x", cb.fb.addr, " cb.fb.data=%x", cb.fb.data); + $display("gen.x[0].addr=%x", gen.x[0].addr, " gen.x[1].addr=%x", gen.x[1].addr); + $display("gen=%p", gen); + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_interface_virtual_bad.out b/test_regress/t/t_interface_virtual_bad.out new file mode 100644 index 000000000..6afbdc125 --- /dev/null +++ b/test_regress/t/t_interface_virtual_bad.out @@ -0,0 +1,34 @@ +%Error: t/t_interface_virtual_bad.v:31:12: Operator ASSIGN expected 'PBus' interface on Assign RHS but 'q8__Viftop' is a different interface ('QBus'). + : ... In instance t + 31 | v8 = q8; + | ^~ +%Error: t/t_interface_virtual_bad.v:35:12: Operator ASSIGN expected no interface modport on Assign RHS but got 'phy' modport. + : ... In instance t + 35 | v8 = v8_phy; + | ^~~~~~ +%Error: t/t_interface_virtual_bad.v:37:17: Operator ASSIGN expected non-interface on Assign RHS but 'p8__Viftop' is an interface. + : ... In instance t + 37 | data = p8.phy; + | ^~~ +%Error: t/t_interface_virtual_bad.v:38:14: Operator ASSIGN expected non-interface on Assign RHS but 'v8_phy' is an interface. + : ... In instance t + 38 | data = v8_phy; + | ^~~~~~ +%Error: t/t_interface_virtual_bad.v:39:14: Operator ASSIGN expected non-interface on Assign RHS but 'v8' is an interface. + : ... In instance t + 39 | data = v8; + | ^~ +%Error: t/t_interface_virtual_bad.v:40:14: Operator ASSIGN expected non-interface on Assign RHS but 'p8__Viftop' is an interface. + : ... In instance t + 40 | data = p8; + | ^~ +%Error: t/t_interface_virtual_bad.v:41:12: Operator ASSIGN expected 'PBus' interface on Assign RHS but 'data' is not an interface. + : ... In instance t + 41 | v8 = data; + | ^~~~ +%Error: t/t_interface_virtual_bad.v:44:79: Member 'gran' not found in interface 'PBus' + : ... In instance t + : ... Suggested alternative: 'grant' + 44 | $display("q8.grant=", p8.grant, " v8.grant=", v8.grant, v8_phy.addr, v8.gran); + | ^~~~ +%Error: Exiting due to diff --git a/test_regress/t/t_interface_virtual_bad.pl b/test_regress/t/t_interface_virtual_bad.pl new file mode 100755 index 000000000..7be596e0f --- /dev/null +++ b/test_regress/t/t_interface_virtual_bad.pl @@ -0,0 +1,19 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_virtual_bad.v b/test_regress/t/t_interface_virtual_bad.v new file mode 100644 index 000000000..3c4f690da --- /dev/null +++ b/test_regress/t/t_interface_virtual_bad.v @@ -0,0 +1,49 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Arkadiusz Kozdra. +// SPDX-License-Identifier: CC0-1.0 + +// See also t_interface_virtual.v + +interface PBus; + logic req, grant; + logic [7:0] addr, data; + modport phy(input addr, ref data); +endinterface + +interface QBus; +endinterface + +typedef virtual PBus vpbus_t; + +module t (/*AUTOARG*/); + + PBus p8; + QBus q8; + vpbus_t v8; + virtual PBus.phy v8_phy; + logic data; + + initial begin + v8 = p8; + p8 = v8; // error + v8 = q8; // error + v8_phy = p8; + v8_phy = v8; + v8_phy = p8.phy; + v8 = v8_phy; // error + v8 = p8.phy; // error + data = p8.phy; // error + data = v8_phy; // error + data = v8; // error + data = p8; // error + v8 = data; // error + v8.grant = 1'b1; + + $display("q8.grant=", p8.grant, " v8.grant=", v8.grant, v8_phy.addr, v8.gran); + + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_interface_virtual_inl.pl b/test_regress/t/t_interface_virtual_inl.pl new file mode 100755 index 000000000..b342b7b3a --- /dev/null +++ b/test_regress/t/t_interface_virtual_inl.pl @@ -0,0 +1,27 @@ +#!/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-2009 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); + +top_filename("t/t_interface_virtual.v"); +golden_filename("t/t_interface_virtual.out"); + +compile( + # Avoid inlining so we find bugs in the non-inliner connection code + verilator_flags2 => ["-fno-inline"], + ); + +execute( + check_finished => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_virtual_unused_bad.out b/test_regress/t/t_interface_virtual_unused_bad.out new file mode 100644 index 000000000..5775cfc79 --- /dev/null +++ b/test_regress/t/t_interface_virtual_unused_bad.out @@ -0,0 +1,5 @@ +%Error-UNSUPPORTED: t/t_interface_virtual_unused_bad.v:14:12: Unsupported: virtual interface never assigned any actual interface + 14 | virtual QBus q8; + | ^~~~ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_interface_virtual_unused_bad.pl b/test_regress/t/t_interface_virtual_unused_bad.pl new file mode 100755 index 000000000..7be596e0f --- /dev/null +++ b/test_regress/t/t_interface_virtual_unused_bad.pl @@ -0,0 +1,19 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2020 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(linter => 1); + +lint( + fails => 1, + expect_filename => $Self->{golden_filename}, + ); + +ok(1); +1; diff --git a/test_regress/t/t_interface_virtual_unused_bad.v b/test_regress/t/t_interface_virtual_unused_bad.v new file mode 100644 index 000000000..c0c7d185f --- /dev/null +++ b/test_regress/t/t_interface_virtual_unused_bad.v @@ -0,0 +1,20 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain, for +// any use, without warranty, 2022 by Arkadiusz Kozdra. +// SPDX-License-Identifier: CC0-1.0 + +// See also t_interface_virtual.v + +interface QBus; +endinterface + +module t (/*AUTOARG*/); + + virtual QBus q8; + + initial begin + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule