From 894f6c4c5830a6335cc632d30f64cd7a8d7979b9 Mon Sep 17 00:00:00 2001 From: Yilou Wang Date: Wed, 1 Apr 2026 22:42:42 +0200 Subject: [PATCH] Fix virtual interface member trigger convergence (#5116) (#7323) --- src/V3Sched.cpp | 137 ++++++++--------- src/V3Sched.h | 45 +++--- src/V3SchedTrigger.cpp | 85 +++++++++++ src/V3SchedVirtIface.cpp | 141 +++++++----------- src/V3Width.cpp | 5 + .../t/t_virtual_interface_member_trigger.py | 2 - ...t_virtual_interface_member_trigger_real.py | 2 - .../t/t_virtual_interface_trigger_unsup.out | 6 + .../t/t_virtual_interface_trigger_unsup.py | 16 ++ .../t/t_virtual_interface_trigger_unsup.v | 19 +++ 10 files changed, 267 insertions(+), 191 deletions(-) create mode 100644 test_regress/t/t_virtual_interface_trigger_unsup.out create mode 100755 test_regress/t/t_virtual_interface_trigger_unsup.py create mode 100644 test_regress/t/t_virtual_interface_trigger_unsup.v diff --git a/src/V3Sched.cpp b/src/V3Sched.cpp index c2f7e24e2..8adc0a52e 100644 --- a/src/V3Sched.cpp +++ b/src/V3Sched.cpp @@ -85,37 +85,28 @@ void invertAndMergeSenTreeMap( for (const auto& pair : senTreeMap) result.emplace(pair.second, pair.first); } -std::vector -findTriggeredIface(const AstVarScope* vscp, - const VirtIfaceTriggers::IfaceMemberSensMap& vifMemberTriggered) { - const AstIface* ifacep; - if (vscp->varp()->isVirtIface()) { - // If `vscp->varp()->isVirtIface()` is true then the interface type that viface is pointing - // to is under `VN_AS(vscp->varp()->dtypep(), IfaceRefDType)->ifacep()` - - ifacep = VN_AS(vscp->varp()->dtypep(), IfaceRefDType)->ifacep(); - - // Virtual interface is sensitive to a different interface type than it is a virtual type - // of - this may be a valid behaviour but this function does not expects that - UASSERT_OBJ(vscp->varp()->sensIfacep() == nullptr, vscp, - "Virtual interface has an ambiguous type - " - << vscp->varp()->sensIfacep()->prettyTypeName() - << " != " << ifacep->prettyTypeName()); - } else { - // If `vscp->varp()` is of a non-virtual interface type it has `sensIfacep()` set to - // interface it is sensitive to - ifacep = vscp->varp()->sensIfacep(); - } - UASSERT_OBJ(ifacep, vscp, "Variable is not sensitive for any interface"); +// Find VIF triggers that a given VarScope should be sensitive to. +// Case 2 (non-virtual interface read): sensitive to that specific VarScope's trigger only +// Case 3 (virtual interface read): sensitive to all member triggers of the same interface type +std::vector findTriggeredIface(const AstVarScope* vscp, + const VirtIfaceTriggers::VscpSensMap& vscpToSens, + const VirtIfaceTriggers& virtIfaceTriggers) { std::vector result; - for (const auto& memberIt : vifMemberTriggered) { - // Interface member variables already identify the exact member that can - // change externally. Sensitizing them to every triggered member of the interface causes - // false feedback paths, e.g. a block reading one signal becoming spuriously sensitive to - // another signal just because both belong to the same interface. - if (memberIt.first.m_memberp != vscp->varp()) continue; - if (memberIt.first.m_ifacep == ifacep) result.push_back(memberIt.second); + if (vscp->varp()->isVirtIface()) { + // Virtual interface variable -- sensitive to all member triggers of this interface type + const AstIface* const ifacep = VN_AS(vscp->varp()->dtypep(), IfaceRefDType)->ifacep(); + for (const auto& entry : virtIfaceTriggers.m_triggers) { + if (entry.m_ifacep == ifacep) { + const auto it = vscpToSens.find(entry.m_vscp); + if (it != vscpToSens.end()) result.push_back(it->second); + } + } + } else { + // Non-virtual interface member -- sensitive to this VarScope's trigger only + const auto it = vscpToSens.find(vscp); + if (it != vscpToSens.end()) result.push_back(it->second); } + // May be empty if sensIfacep() is set but no VIF write targets this member return result; } @@ -426,14 +417,15 @@ void createFinal(AstNetlist* netlistp, const LogicClasses& logicClasses) { } //============================================================================ -// Helper that creates virtual interface trigger resets +// Helper that creates virtual interface value-change triggers -void addVirtIfaceTriggerAssignments(const VirtIfaceTriggers& virtIfaceTriggers, - uint32_t vifTriggerIndex, uint32_t vifMemberTriggerIndex, - const TriggerKit& trigKit) { - for (const auto& p : virtIfaceTriggers.m_memberTriggers) { - trigKit.addExtraTriggerAssignment(p.second, vifMemberTriggerIndex); - ++vifMemberTriggerIndex; +void addVirtIfaceTriggerAssignments(AstNetlist* netlistp, AstCFunc* initFuncp, + const VirtIfaceTriggers& virtIfaceTriggers, + uint32_t firstIndex, const TriggerKit& trigKit) { + uint32_t index = firstIndex; + for (const auto& entry : virtIfaceTriggers.m_triggers) { + trigKit.addValueChangeTriggerAssignment(netlistp, initFuncp, entry.m_vscp, index); + ++index; } } @@ -531,12 +523,10 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, const uint32_t dpiExportTriggerIndex = dpiExportTriggerVscp ? extraTriggers.allocate("DPI export trigger") : std::numeric_limits::max(); - const size_t firstVifTriggerIndex = extraTriggers.size(); - const size_t firstVifMemberTriggerIndex = extraTriggers.size(); - for (const auto& p : virtIfaceTriggers.m_memberTriggers) { - const auto& item = p.first; - extraTriggers.allocate("virtual interface member: " + item.m_ifacep->name() + "." - + item.m_memberp->name()); + const uint32_t firstVifTriggerIndex = extraTriggers.size(); + for (const auto& entry : virtIfaceTriggers.m_triggers) { + extraTriggers.allocate("virtual interface member: " + entry.m_ifacep->name() + "." + + entry.m_memberp->name()); } // Gather the relevant sensitivity expressions and create the trigger kit @@ -548,8 +538,8 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, if (dpiExportTriggerVscp) { trigKit.addExtraTriggerAssignment(dpiExportTriggerVscp, dpiExportTriggerIndex); } - addVirtIfaceTriggerAssignments(virtIfaceTriggers, firstVifTriggerIndex, - firstVifMemberTriggerIndex, trigKit); + addVirtIfaceTriggerAssignments(netlistp, initFuncp, virtIfaceTriggers, firstVifTriggerIndex, + trigKit); // Remap sensitivities remapSensitivities(logic, trigKit.mapVec()); @@ -567,21 +557,21 @@ AstNode* createInputCombLoop(AstNetlist* netlistp, AstCFunc* const initFuncp, = dpiExportTriggerVscp ? trigKit.newExtraTriggerSenTree(trigKit.vscp(), dpiExportTriggerIndex) : nullptr; - const auto& vifMemberTriggeredIco = virtIfaceTriggers.makeMemberToSensMap( - trigKit, firstVifMemberTriggerIndex, trigKit.vscp()); + const auto& vifVscpToSensIco + = virtIfaceTriggers.makeVscpToSensMap(trigKit, firstVifTriggerIndex, trigKit.vscp()); // Create and Order the body function AstCFunc* const icoFuncp = V3Order::order( netlistp, {&logic}, trigToSen, "ico", false, false, - [=](const AstVarScope* vscp, std::vector& out) { + [&](const AstVarScope* vscp, std::vector& out) { AstVar* const varp = vscp->varp(); if (varp->isPrimaryInish() || varp->isSigUserRWPublic()) { out.push_back(inputChanged); } if (varp->isWrittenByDpi()) out.push_back(dpiExportTriggered); - if (vscp->varp()->isVirtIface()) { - std::vector ifaceTriggered - = findTriggeredIface(vscp, vifMemberTriggeredIco); + if (vscp->varp()->sensIfacep() || vscp->varp()->isVirtIface()) { + const auto& ifaceTriggered + = findTriggeredIface(vscp, vifVscpToSensIco, virtIfaceTriggers); out.insert(out.end(), ifaceTriggered.begin(), ifaceTriggered.end()); } }); @@ -844,13 +834,14 @@ void createEval(AstNetlist* netlistp, // //============================================================================ // Helper that builds virtual interface trigger sentrees -VirtIfaceTriggers::IfaceMemberSensMap -VirtIfaceTriggers::makeMemberToSensMap(const TriggerKit& trigKit, uint32_t vifTriggerIndex, - AstVarScope* trigVscp) const { - IfaceMemberSensMap map; - for (const auto& p : m_memberTriggers) { - map.emplace(p.first, trigKit.newExtraTriggerSenTree(trigVscp, vifTriggerIndex)); - ++vifTriggerIndex; +VirtIfaceTriggers::VscpSensMap VirtIfaceTriggers::makeVscpToSensMap(const TriggerKit& trigKit, + uint32_t firstIndex, + AstVarScope* trigVscp) const { + VscpSensMap map; + uint32_t index = firstIndex; + for (const auto& entry : m_triggers) { + map.emplace(entry.m_vscp, trigKit.newExtraTriggerSenTree(trigVscp, index)); + ++index; } return map; } @@ -981,11 +972,9 @@ void schedule(AstNetlist* netlistp) { ? extraTriggers.allocate("DPI export trigger") : std::numeric_limits::max(); const uint32_t firstVifTriggerIndex = extraTriggers.size(); - const uint32_t firstVifMemberTriggerIndex = extraTriggers.size(); - for (const auto& p : virtIfaceTriggers.m_memberTriggers) { - const auto& item = p.first; - extraTriggers.allocate("virtual interface member: " + item.m_ifacep->name() + "." - + item.m_memberp->name()); + for (const auto& entry : virtIfaceTriggers.m_triggers) { + extraTriggers.allocate("virtual interface member: " + entry.m_ifacep->name() + "." + + entry.m_memberp->name()); } const auto& preTreeps = getSenTreesUsedBy({&logicRegions.m_pre}); @@ -1004,8 +993,8 @@ void schedule(AstNetlist* netlistp) { if (dpiExportTriggerVscp) { trigKit.addExtraTriggerAssignment(dpiExportTriggerVscp, dpiExportTriggerIndex); } - addVirtIfaceTriggerAssignments(virtIfaceTriggers, firstVifTriggerIndex, - firstVifMemberTriggerIndex, trigKit); + addVirtIfaceTriggerAssignments(netlistp, staticp, virtIfaceTriggers, firstVifTriggerIndex, + trigKit); if (v3Global.opt.stats()) V3Stats::statsStage("sched-create-triggers"); // Note: Experiments so far show that running the Act (or Ico) regions on @@ -1034,8 +1023,8 @@ void schedule(AstNetlist* netlistp) { ? trigKit.newExtraTriggerSenTree(trigKit.vscp(), dpiExportTriggerIndex) : nullptr; - const auto& vifMemberTriggeredAct = virtIfaceTriggers.makeMemberToSensMap( - trigKit, firstVifMemberTriggerIndex, trigKit.vscp()); + const auto& vifVscpToSensAct + = virtIfaceTriggers.makeVscpToSensMap(trigKit, firstVifTriggerIndex, trigKit.vscp()); AstCFunc* const actFuncp = V3Order::order( netlistp, {&logicRegions.m_pre, &logicRegions.m_act, &logicReplicas.m_act}, trigToSenAct, @@ -1043,9 +1032,9 @@ void schedule(AstNetlist* netlistp) { auto it = actTimingDomains.find(vscp); if (it != actTimingDomains.end()) out = it->second; if (vscp->varp()->isWrittenByDpi()) out.push_back(dpiExportTriggeredAct); - if (vscp->varp()->isVirtIface()) { - std::vector ifaceTriggered - = findTriggeredIface(vscp, vifMemberTriggeredAct); + if (vscp->varp()->sensIfacep() || vscp->varp()->isVirtIface()) { + const auto& ifaceTriggered + = findTriggeredIface(vscp, vifVscpToSensAct, virtIfaceTriggers); out.insert(out.end(), ifaceTriggered.begin(), ifaceTriggered.end()); } }); @@ -1071,8 +1060,8 @@ void schedule(AstNetlist* netlistp) { = dpiExportTriggerVscp ? trigKit.newExtraTriggerSenTree(trigVscp, dpiExportTriggerIndex) : nullptr; - const auto& vifMemberTriggered - = virtIfaceTriggers.makeMemberToSensMap(trigKit, firstVifMemberTriggerIndex, trigVscp); + const auto& vifVscpToSens + = virtIfaceTriggers.makeVscpToSensMap(trigKit, firstVifTriggerIndex, trigVscp); const auto& timingDomains = timingKit.remapDomains(trigMap); AstCFunc* const funcp = V3Order::order( @@ -1081,11 +1070,9 @@ void schedule(AstNetlist* netlistp) { auto it = timingDomains.find(vscp); if (it != timingDomains.end()) out = it->second; if (vscp->varp()->isWrittenByDpi()) out.push_back(dpiExportTriggered); - // Sometimes virtual interfaces mix with non-virtual one so, here both have to be - // detected - look `t_virtual_interface_nba_assign` if (vscp->varp()->sensIfacep() || vscp->varp()->isVirtIface()) { - std::vector ifaceTriggered - = findTriggeredIface(vscp, vifMemberTriggered); + const auto& ifaceTriggered + = findTriggeredIface(vscp, vifVscpToSens, virtIfaceTriggers); out.insert(out.end(), ifaceTriggered.begin(), ifaceTriggered.end()); } }); diff --git a/src/V3Sched.h b/src/V3Sched.h index 68945f7a7..0c5907886 100644 --- a/src/V3Sched.h +++ b/src/V3Sched.h @@ -312,6 +312,11 @@ public: // Set then extra trigger bit at 'index' to the value of 'vscp', then set 'vscp' to 0 void addExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, bool clear = true) const; + + // Set trigger bit at 'index' when vscp's value changes from previous evaluation. + // Creates a prev variable: trigger[bit] = (vscp != prev); prev = vscp + void addValueChangeTriggerAssignment(AstNetlist* netlistp, AstCFunc* initFuncp, + AstVarScope* vscp, uint32_t index) const; }; // Everything needed for combining timing with static scheduling. @@ -348,37 +353,27 @@ public: }; class VirtIfaceTriggers final { - // Represents a specific member in a virtual interface - struct IfaceMember final { +public: + // One entry per VarScope of an interface member that may be written through a virtual + // interface. Each gets its own extra trigger bit with value-change detection. + struct TriggerEntry final { const AstIface* m_ifacep; // Interface type - const AstVar* m_memberp; // Member variable - - // TODO: sorting by pointer is non-deterministic - bool operator<(const IfaceMember& other) const { - if (m_ifacep != other.m_ifacep) return m_ifacep < other.m_ifacep; - return m_memberp < other.m_memberp; - } + const AstVar* m_memberp; // Member variable in the interface definition + AstVarScope* m_vscp; // VarScope to monitor }; -public: - using IfaceMemberSensMap = std::map; + std::vector m_triggers; - std::vector> m_memberTriggers; - - void addMemberTrigger(const AstIface* ifacep, const AstVar* memberp, AstVarScope* vscp) { - m_memberTriggers.emplace_back(IfaceMember{ifacep, memberp}, vscp); + void addTrigger(const AstIface* ifacep, const AstVar* memberp, AstVarScope* vscp) { + m_triggers.push_back(TriggerEntry{ifacep, memberp, vscp}); } - AstVarScope* findMemberTrigger(const AstIface* ifacep, const AstVar* memberp) const { - for (const auto& pair : m_memberTriggers) { - const IfaceMember& item = pair.first; - if (item.m_ifacep == ifacep && item.m_memberp == memberp) return pair.second; - } - return nullptr; - } - - IfaceMemberSensMap makeMemberToSensMap(const TriggerKit& trigKit, uint32_t vifTriggerIndex, - AstVarScope* trigVscp) const; + // Maps each triggered VarScope to its AstSenTree. findTriggeredIface() uses this + // to look up triggers -- by specific VarScope for non-virtual reads, or by iterating + // all entries of the same interface type for virtual interface reads. + using VscpSensMap = std::map; + VscpSensMap makeVscpToSensMap(const TriggerKit& trigKit, uint32_t firstIndex, + AstVarScope* trigVscp) const; VL_UNCOPYABLE(VirtIfaceTriggers); VirtIfaceTriggers() = default; diff --git a/src/V3SchedTrigger.cpp b/src/V3SchedTrigger.cpp index 6bd10a407..12afaa051 100644 --- a/src/V3SchedTrigger.cpp +++ b/src/V3SchedTrigger.cpp @@ -426,6 +426,91 @@ void TriggerKit::addExtraTriggerAssignment(AstVarScope* vscp, uint32_t index, bo m_compVecp->addStmtsp(setp); } +// Value-change detection for a single interface member VarScope written through a VIF. +// Creates: trigger[bit] = (vscp != prev); prev = vscp; +void TriggerKit::addValueChangeTriggerAssignment(AstNetlist* netlistp, AstCFunc* initFuncp, + AstVarScope* instVscp, uint32_t index) const { + index += m_nSenseWords * WORD_SIZE; + const uint32_t wordIndex = index / WORD_SIZE; + const uint32_t bitIndex = index % WORD_SIZE; + + AstScope* const scopeTopp = netlistp->topScopep()->scopep(); + FileLine* const flp = netlistp->fileline(); + AstNodeDType* const dtypep = instVscp->dtypep()->skipRefp(); + + // Reject types that do not support value-change comparison + if (const AstBasicDType* const bdtypep = VN_CAST(dtypep, BasicDType)) { + if (bdtypep->isEvent() || bdtypep->isString() + || bdtypep->keyword() == VBasicDTypeKwd::CHANDLE) { + instVscp->v3warn(E_UNSUPPORTED, "Unsupported: virtual interface trigger on " + << dtypep->prettyDTypeNameQ() << " type"); + return; + } + } else if (!VN_IS(dtypep, PackArrayDType) && !VN_IS(dtypep, UnpackArrayDType) + && !VN_IS(dtypep, NodeUOrStructDType) && !VN_IS(dtypep, ClassRefDType)) { + instVscp->v3warn(E_UNSUPPORTED, "Unsupported: virtual interface trigger on " + << dtypep->prettyDTypeNameQ() << " type"); + return; + } + + const auto rdInst = [flp](AstVarScope* vp) { return new AstVarRef{flp, vp, VAccess::READ}; }; + const auto wrPrev = [flp](AstVarScope* vp) { return new AstVarRef{flp, vp, VAccess::WRITE}; }; + const auto rdPrev = [flp](AstVarScope* vp) { return new AstVarRef{flp, vp, VAccess::READ}; }; + + // Create prev variable + const std::string prevName = "__Vtrigprevvif_" + m_name + "_" + + instVscp->scopep()->nameDotless() + "__" + + instVscp->varp()->name(); + AstVarScope* const prevVscp = scopeTopp->createTemp(prevName, instVscp->dtypep()); + + // Initialize prev = inst + if (VN_IS(dtypep, UnpackArrayDType)) { + AstCMethodHard* const cmhp = new AstCMethodHard{ + flp, wrPrev(prevVscp), VCMethod::UNPACKED_ASSIGN, rdInst(instVscp)}; + cmhp->dtypeSetVoid(); + initFuncp->addStmtsp(cmhp->makeStmt()); + } else { + initFuncp->addStmtsp(new AstAssign{flp, wrPrev(prevVscp), rdInst(instVscp)}); + } + + // Build comparison: inst != prev + AstNodeExpr* neqp; + if (VN_IS(dtypep, UnpackArrayDType)) { + AstCMethodHard* const cmhp + = new AstCMethodHard{flp, rdPrev(prevVscp), VCMethod::UNPACKED_NEQ, rdInst(instVscp)}; + cmhp->dtypeSetBit(); + neqp = cmhp; + } else { + neqp = new AstNeq{flp, rdInst(instVscp), rdPrev(prevVscp)}; + } + + // Build post-update: prev = inst + AstNode* updp; + if (VN_IS(dtypep, UnpackArrayDType)) { + AstCMethodHard* const cmhp = new AstCMethodHard{ + flp, wrPrev(prevVscp), VCMethod::UNPACKED_ASSIGN, rdInst(instVscp)}; + cmhp->dtypeSetVoid(); + updp = cmhp->makeStmt(); + } else { + updp = new AstAssign{flp, wrPrev(prevVscp), rdInst(instVscp)}; + } + + // Set trigger bit + AstVarRef* const refp = new AstVarRef{flp, m_vscp, VAccess::WRITE}; + AstNodeExpr* const wordp = new AstArraySel{flp, refp, static_cast(wordIndex)}; + AstNodeExpr* const trigLhsp = new AstSel{flp, wordp, static_cast(bitIndex), 1}; + AstNode* const setp = new AstAssign{flp, trigLhsp, neqp}; + + // Chain: set trigger bit -> update prev + setp->addNext(updp); + + // Prepend before existing statements + if (AstNode* const nodep = m_compVecp->stmtsp()) { + setp->addNext(setp, nodep->unlinkFrBackWithNext()); + } + m_compVecp->addStmtsp(setp); +} + TriggerKit::TriggerKit(const std::string& name, bool slow, uint32_t nSenseWords, uint32_t nExtraWords, uint32_t nPreWords, std::unordered_map, size_t> senItem2TrigIdx, diff --git a/src/V3SchedVirtIface.cpp b/src/V3SchedVirtIface.cpp index 7d19d7c7a..4dd95662d 100644 --- a/src/V3SchedVirtIface.cpp +++ b/src/V3SchedVirtIface.cpp @@ -16,13 +16,12 @@ //************************************************************************* // V3SchedVirtIface's Transformations: // -// Each interface type written to via virtual interface, or written to normally but read via -// virtual interface: -// Create a trigger var for it -// Each statement: -// If it writes to a virtual interface, or to a variable read via virtual interface: -// Set the corresponding trigger to 1 -// If the write is done by an AssignDly, the trigger is also set by AssignDly +// Identify interface members written through virtual interface variables (VIF writes). +// For each such member, collect all VarScopes across all instantiations of that +// interface type. Each VarScope gets its own value-change trigger in the scheduler. +// +// Also disables lifetime optimization for variables in AlwaysPost blocks that +// write through virtual interfaces. // //************************************************************************* @@ -30,7 +29,6 @@ #include "V3AstNodeExpr.h" #include "V3Sched.h" -#include "V3UniqueNames.h" VL_DEFINE_DEBUG_FUNCTIONS; @@ -40,103 +38,72 @@ namespace { class VirtIfaceVisitor final : public VNVisitor { private: - // NODE STATE - // AstVarRef::user1() -> bool. Whether it has been visited - // AstMemberSel::user1() -> bool. Whether it has been visited - const VNUser1InUse m_user1InUse; - - // TYPES - using OnWriteToVirtIface = std::function; - using OnWriteToVirtIfaceMember - = std::function; - // STATE - AstNetlist* const m_netlistp; // Root node - V3UniqueNames m_vifTriggerNames{"__VvifTrigger"}; // Unique names for virt iface - // triggers - VirtIfaceTriggers m_triggers; // Interfaces and corresponding trigger vars + // Set of (iface, member) pairs written through VIF -- defines which members need triggers + std::set> m_vifWrittenMembers; + // All candidate VarScopes of interface members (keyed by interface type + member name) + std::map, std::vector> + m_candidateVscps; + VirtIfaceTriggers m_triggers; // METHODS - // For each write across a virtual interface boundary - // Returns true if there is a write across a virtual interface boundary + // Returns true if statement writes across a virtual interface boundary static bool writesToVirtIface(const AstNode* const nodep) { - return nodep->exists([](const AstVarRef* const refp) { - if (!refp->access().isWriteOrRW()) return false; - AstIfaceRefDType* const dtypep = VN_CAST(refp->varp()->dtypep(), IfaceRefDType); - const bool writesToVirtIfaceMember - = (dtypep && dtypep->isVirtual() && VN_IS(refp->firstAbovep(), MemberSel)); - const bool writesToIfaceSensVar = refp->varp()->isVirtIface(); - return writesToVirtIfaceMember || writesToIfaceSensVar; + return nodep->exists([](const AstMemberSel* const memberSelp) { + if (!memberSelp->access().isWriteOrRW()) return false; + AstIfaceRefDType* const dtypep + = VN_CAST(memberSelp->fromp()->dtypep()->skipRefp(), IfaceRefDType); + return dtypep && dtypep->isVirtual(); }); } - // Create trigger reference for a specific interface member - AstVarRef* createVirtIfaceMemberTriggerRefp(FileLine* const flp, AstIface* ifacep, - const AstVar* memberVarp) { - // Check if we already have a trigger for this specific member - AstVarScope* existingTrigger = m_triggers.findMemberTrigger(ifacep, memberVarp); - if (!existingTrigger) { - AstScope* const scopeTopp = m_netlistp->topScopep()->scopep(); - // Create a unique name for this member trigger - const std::string triggerName - = m_vifTriggerNames.get(ifacep) + "_Vtrigm_" + memberVarp->name(); - AstVarScope* const vscp = scopeTopp->createTemp(triggerName, 1); - m_triggers.addMemberTrigger(ifacep, memberVarp, vscp); - existingTrigger = vscp; - } - return new AstVarRef{flp, existingTrigger, VAccess::WRITE}; - } - - template - void handleIface(T nodep) { - static_assert(std::is_same::type, - typename std::add_pointer::type>::value - || std::is_same::type, - typename std::add_pointer::type>::value, - "Node has to be of AstVarRef* or AstMemberSel* type"); - if (nodep->access().isReadOnly()) return; - if (nodep->user1SetOnce()) return; - AstIface* ifacep = nullptr; - AstVar* memberVarp = nullptr; - if (nodep->varp()->isVirtIface()) { - if (AstMemberSel* const memberSelp = VN_CAST(nodep->firstAbovep(), MemberSel)) { - ifacep = VN_AS(nodep->varp()->dtypep(), IfaceRefDType)->ifacep(); - memberVarp = memberSelp->varp(); - } - } else if ((ifacep = nodep->varp()->sensIfacep())) { - memberVarp = nodep->varp(); - } - - if (ifacep && memberVarp) { - FileLine* const flp = nodep->fileline(); - VNRelinker relinker; - nodep->unlinkFrBack(&relinker); - relinker.relink(new AstExprStmt{ - flp, - new AstAssign{flp, createVirtIfaceMemberTriggerRefp(flp, ifacep, memberVarp), - new AstConst{flp, AstConst::BitTrue{}}}, - nodep}); - } - } - // VISITORS + void visit(AstMemberSel* nodep) override { + // Detect writes through VIF: the MemberSel's fromp resolves to a virtual interface type. + // Handles both direct VIF access (vif.member) and class chain (obj.vif.member). + if (nodep->access().isWriteOrRW()) { + AstIfaceRefDType* const dtypep + = VN_CAST(nodep->fromp()->dtypep()->skipRefp(), IfaceRefDType); + if (dtypep && dtypep->isVirtual()) { + m_vifWrittenMembers.emplace(dtypep->ifacep(), nodep->varp()->name()); + } + } + iterateChildren(nodep); + } + void visit(AstVarScope* nodep) override { + // Collect candidate VarScopes. sensIfacep() is set on interface members + // accessed via any MemberSel (virtual or non-virtual). + if (const AstIface* const ifacep = nodep->varp()->sensIfacep()) { + m_candidateVscps[{ifacep, nodep->varp()->name()}].push_back(nodep); + } + } void visit(AstNodeProcedure* nodep) override { - // Not sure if needed, but be paranoid to match previous behavior as didn't optimize - // before .. + // Disable lifetime optimization for variables in AlwaysPost blocks + // that write to virtual interface members, as VIF reads may observe them if (VN_IS(nodep, AlwaysPost) && writesToVirtIface(nodep)) { nodep->foreach([](AstVarRef* refp) { refp->varScopep()->optimizeLifePost(false); }); } iterateChildren(nodep); } - void visit(AstMemberSel* const nodep) override { handleIface(nodep); } - void visit(AstVarRef* const nodep) override { handleIface(nodep); } void visit(AstNode* nodep) override { iterateChildren(nodep); } + // Build final trigger list by intersecting VIF writes with candidate VarScopes + void buildTriggers() { + for (const auto& written : m_vifWrittenMembers) { + const auto it = m_candidateVscps.find(written); + if (it == m_candidateVscps.end()) continue; + for (AstVarScope* const vscp : it->second) { + const AstIface* const ifacep = written.first; + m_triggers.addTrigger(ifacep, vscp->varp(), vscp); + } + } + } + public: // CONSTRUCTORS - explicit VirtIfaceVisitor(AstNetlist* nodep) - : m_netlistp{nodep} { + explicit VirtIfaceVisitor(AstNetlist* nodep) { iterate(nodep); + buildTriggers(); } ~VirtIfaceVisitor() override = default; @@ -151,7 +118,7 @@ VirtIfaceTriggers makeVirtIfaceTriggers(AstNetlist* nodep) { VirtIfaceTriggers triggers{}; if (v3Global.hasVirtIfaces()) { triggers = VirtIfaceVisitor{nodep}.take_triggers(); - // Dump afer destructor so VNDeleter runs + // Dump after destructor so VNDeleter runs V3Global::dumpCheckGlobalTree("sched_vif", 0, dumpTreeEitherLevel() >= 6); } return triggers; diff --git a/src/V3Width.cpp b/src/V3Width.cpp index c0b25f1c8..1b823038d 100644 --- a/src/V3Width.cpp +++ b/src/V3Width.cpp @@ -4499,6 +4499,11 @@ class WidthVisitor final : public VNVisitor { if (AstNodeFTask* const ftaskp = VN_CAST(m_memberMap.findMember(ifacep, nodep->name()), NodeFTask)) { UINFO(5, __FUNCTION__ << "AstNodeFTask" << nodep); + if (adtypep->isVirtual()) { + for (AstNode* itemp = ifacep->stmtsp(); itemp; itemp = itemp->nextp()) { + if (AstVar* const mvarp = VN_CAST(itemp, Var)) { mvarp->sensIfacep(ifacep); } + } + } userIterate(ftaskp, nullptr); if (ftaskp->isStatic()) { AstArg* const argsp = nodep->argsp(); diff --git a/test_regress/t/t_virtual_interface_member_trigger.py b/test_regress/t/t_virtual_interface_member_trigger.py index 567f18b29..4ee7f9e14 100755 --- a/test_regress/t/t_virtual_interface_member_trigger.py +++ b/test_regress/t/t_virtual_interface_member_trigger.py @@ -9,8 +9,6 @@ import vltest_bootstrap -test.skip("Test is broken, see #6613") - test.scenarios('simulator') test.compile(verilator_flags2=['--binary']) diff --git a/test_regress/t/t_virtual_interface_member_trigger_real.py b/test_regress/t/t_virtual_interface_member_trigger_real.py index 567f18b29..4ee7f9e14 100755 --- a/test_regress/t/t_virtual_interface_member_trigger_real.py +++ b/test_regress/t/t_virtual_interface_member_trigger_real.py @@ -9,8 +9,6 @@ import vltest_bootstrap -test.skip("Test is broken, see #6613") - test.scenarios('simulator') test.compile(verilator_flags2=['--binary']) diff --git a/test_regress/t/t_virtual_interface_trigger_unsup.out b/test_regress/t/t_virtual_interface_trigger_unsup.out new file mode 100644 index 000000000..fdd056f7c --- /dev/null +++ b/test_regress/t/t_virtual_interface_trigger_unsup.out @@ -0,0 +1,6 @@ +%Error-UNSUPPORTED: t/t_virtual_interface_trigger_unsup.v:8:10: Unsupported: virtual interface trigger on 'string' type + : ... note: In instance 't.sif' + 8 | string s; + | ^ + ... For error description see https://verilator.org/warn/UNSUPPORTED?v=latest +%Error: Exiting due to diff --git a/test_regress/t/t_virtual_interface_trigger_unsup.py b/test_regress/t/t_virtual_interface_trigger_unsup.py new file mode 100755 index 000000000..f2bbe4479 --- /dev/null +++ b/test_regress/t/t_virtual_interface_trigger_unsup.py @@ -0,0 +1,16 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# 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-FileCopyrightText: 2026 Wilson Snyder +# SPDX-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +import vltest_bootstrap + +test.scenarios('vlt') + +test.compile(verilator_flags2=['--binary'], fails=True, expect_filename=test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_virtual_interface_trigger_unsup.v b/test_regress/t/t_virtual_interface_trigger_unsup.v new file mode 100644 index 000000000..6fb2315e2 --- /dev/null +++ b/test_regress/t/t_virtual_interface_trigger_unsup.v @@ -0,0 +1,19 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 PlanV GmbH +// SPDX-License-Identifier: CC0-1.0 + +interface str_if; + string s; +endinterface + +module t; + str_if sif(); + virtual str_if vif = sif; + + initial begin + vif.s = "hello"; + $finish; + end +endmodule