From 8312e9d90110c42460882a85182e5d49c4033b7c Mon Sep 17 00:00:00 2001 From: Yogish Sekhar <160029258+ysekhar@users.noreply.github.com> Date: Wed, 13 May 2026 10:59:22 +0000 Subject: [PATCH] Extend FSM Detect to support 'Wide State Encodings' (#7573) --- docs/guide/simulating.rst | 6 + docs/spelling.txt | 1 + src/V3FsmDetect.cpp | 323 +++++++++++------- test_regress/t/t_cover_fsm_beginif.out | 15 +- test_regress/t/t_cover_fsm_enumwide_bad.out | 6 - test_regress/t/t_cover_fsm_enumwide_bad.py | 18 - test_regress/t/t_cover_fsm_enumwide_bad.v | 33 -- .../t_cover_fsm_if_unknown_enum_multi_bad.out | 3 + .../t/t_cover_fsm_if_unknown_enum_multi_bad.v | 27 ++ .../t/t_cover_fsm_nonenum_unsupported_bad.out | 37 +- .../t/t_cover_fsm_nonenum_unsupported_bad.v | 83 ++++- .../t/t_cover_fsm_policy_accept_multi.out | 13 +- .../t/t_cover_fsm_transition_shapes_multi.out | 10 + .../t/t_cover_fsm_transition_shapes_multi.v | 10 +- test_regress/t/t_cover_fsm_wide_sparse.out | 148 ++++++++ test_regress/t/t_cover_fsm_wide_sparse.py | 30 ++ test_regress/t/t_cover_fsm_wide_sparse.v | 114 +++++++ 17 files changed, 673 insertions(+), 204 deletions(-) delete mode 100644 test_regress/t/t_cover_fsm_enumwide_bad.out delete mode 100755 test_regress/t/t_cover_fsm_enumwide_bad.py delete mode 100644 test_regress/t/t_cover_fsm_enumwide_bad.v create mode 100644 test_regress/t/t_cover_fsm_wide_sparse.out create mode 100755 test_regress/t/t_cover_fsm_wide_sparse.py create mode 100644 test_regress/t/t_cover_fsm_wide_sparse.v diff --git a/docs/guide/simulating.rst b/docs/guide/simulating.rst index 86dafb87d..842a21dff 100644 --- a/docs/guide/simulating.rst +++ b/docs/guide/simulating.rst @@ -235,6 +235,12 @@ encodings in these common forms: with a combinational next-state block using the same supported ``case`` or top-level ``if`` / ``else if`` dispatch forms +Scalar state encodings may be wider than 32 bits. This allows sparse +state encodings, such as high-Hamming-distance enum or localparam values, +to be preserved in the detected FSM model. Verilator uses the declared +enum item name, parameter name, or localparam name as the reported state +label where possible. + Simple input guards are supported when they appear inside a recognized state branch, or as a top-level conjunction containing exactly one state comparison, such as ``(state_q == IDLE) && ready``. Directly traceable diff --git a/docs/spelling.txt b/docs/spelling.txt index 2c8c8131e..8b6492d3a 100644 --- a/docs/spelling.txt +++ b/docs/spelling.txt @@ -181,6 +181,7 @@ Gustafsson Güzel Hameed Hammoud +Hamming Hao Haojin Harboe diff --git a/src/V3FsmDetect.cpp b/src/V3FsmDetect.cpp index dc37bec11..c383f55c8 100644 --- a/src/V3FsmDetect.cpp +++ b/src/V3FsmDetect.cpp @@ -40,9 +40,57 @@ VL_DEFINE_DEBUG_FUNCTIONS; namespace { -// FSM graph state ids intentionally stay scalar here; arbitrary-width Verilog -// encodings need a wider graph representation and are rejected before lowering. -using FsmStateValue = uint32_t; +// Width-preserving FSM state identity. FSM detection needs a stable key for +// graph vertices and lookup tables, but lowering still needs the original +// folded Verilog value so emitted comparisons keep the correct width and bits. +class FsmStateValue final { + // Hash/equality key only. It deliberately ignores signedness because + // signed and unsigned constants with the same width and bits denote the + // same encoded FSM state. + string m_key; // Canonical "width:value" identity, independent of signedness + + // Semantic value. This is what diagnostics and lowering use when printing + // values or rebuilding AstConst nodes for instrumentation. + V3Number m_num; // Original folded value, preserving width for lowered comparisons + + static string makeKey(const V3Number& num) { + V3Number keyNum = num; + // Signedness does not change FSM state identity: same width and bits + // should address the same graph vertex. + keyNum.isSigned(false); + return cvtToStr(keyNum.width()) + ":" + keyNum.ascii(true, true); + } + +public: + // Default value is used only for synthetic pseudo-states such as ANY and + // default, which never use m_num as a real Verilog state encoding. + FsmStateValue() + : m_key{"1:1'h0"} + , m_num{static_cast(nullptr), 1, 0} {} + explicit FsmStateValue(const V3Number& num) + : m_key{makeKey(num)} + , m_num{num} {} + + const string& key() const { return m_key; } + const V3Number& num() const { return m_num; } + string ascii() const { return m_num.ascii(true, true); } + string warnText() const { + // Preserve legacy diagnostics for old <=32-bit FSMs, but print wide + // values without truncation. + if (m_num.width() <= 32) return cvtToStr(m_num.toUInt()); + return ascii(); + } + + bool operator==(const FsmStateValue& rhs) const { return m_key == rhs.m_key; } +}; + +// unordered_map needs an explicit hash for this custom key type. Keep the +// hash definition paired with operator== by hashing the same canonical key. +struct FsmStateValueHash final { + size_t operator()(const FsmStateValue& value) const { + return std::hash{}(value.key()); + } +}; // Captures one sensitivity-list entry so the lowering phase can later rebuild // an active block with the same triggering event control. @@ -62,17 +110,17 @@ struct FsmResetCondDesc final { }; class FsmResetArcDesc final { - FsmStateValue m_toValue = 0; // Encoded reset target state. - AstNode* m_nodep = nullptr; // Source node for warnings and emitted metadata. + FsmStateValue m_toValue; // Encoded reset target state. + AstNodeAssign* m_nodep = nullptr; // Source assignment for warnings and emitted metadata public: FsmResetArcDesc() = default; - FsmResetArcDesc(FsmStateValue toValue, AstNode* nodep) + FsmResetArcDesc(FsmStateValue toValue, AstNodeAssign* nodep) : m_toValue{toValue} , m_nodep{nodep} {} FsmStateValue toValue() const { return m_toValue; } - AstNode* nodep() const { return m_nodep; } + AstNodeAssign* nodep() const { return m_nodep; } }; class FsmRegisterCandidate final { @@ -135,7 +183,7 @@ public: private: Kind m_kind; // State vs synthetic ANY/default vertex role. string m_label; // User-facing state or pseudo-state label. - FsmStateValue m_value = 0; // Encoded state value for real state vertices. + FsmStateValue m_value; // Encoded state value for real state vertices. protected: FsmVertex(V3Graph* graphp, Kind kind, string label, FsmStateValue value) VL_MT_DISABLED @@ -153,7 +201,7 @@ public: const string& label() const { return m_label; } FsmStateValue value() const { return m_value; } - string name() const override VL_MT_SAFE { return m_label + "=" + cvtToStr(m_value); } + string name() const override VL_MT_SAFE { return m_label + "=" + m_value.ascii(); } }; class FsmStateVertex final : public FsmVertex { @@ -173,7 +221,7 @@ class FsmPseudoVertex final : public FsmVertex { public: FsmPseudoVertex(V3Graph* graphp, Kind kind, string label) VL_MT_DISABLED - : FsmVertex{graphp, kind, label, 0} {} + : FsmVertex{graphp, kind, label, FsmStateValue{}} {} ~FsmPseudoVertex() override = default; string name() const override VL_MT_SAFE { return label(); } @@ -230,7 +278,8 @@ class FsmGraph final : public V3Graph { bool m_resetInclude = false; // Whether reset arcs count toward coverage totals. bool m_inclCond = false; // Whether conditional arcs should be kept explicitly. FileLine* m_flp = nullptr; // Representative source location for declarations/arcs. - std::unordered_map m_stateVertices; // Value to state map. + std::unordered_map + m_stateVertices; // Value to state map. FsmPseudoVertex* m_resetVertexp = nullptr; // Synthetic ANY source for reset arcs. FsmPseudoVertex* m_defaultVertexp = nullptr; // Synthetic default source for case defaults. @@ -312,7 +361,7 @@ struct FsmCaseCandidate final { struct FsmStateComparison final { AstVarScope* stateVscp = nullptr; // Compared state variable AstNodeExpr* valuep = nullptr; // Compared constant value expression - FsmStateValue value = 0; // Encoded compared state value + FsmStateValue value; // Encoded compared state value }; // A branch is usable only after its predicate has exactly one state comparison; @@ -321,7 +370,7 @@ struct FsmIfBranch final { AstIf* ifp = nullptr; // Source if/else-if node AstNode* stmtsp = nullptr; // Branch body AstNodeExpr* valuep = nullptr; // Source state value expression - FsmStateValue fromValue = 0; // Encoded source state value + FsmStateValue fromValue; // Encoded source state value bool hasTopGuard = false; // Branch condition had extra guard terms }; @@ -346,7 +395,8 @@ struct StateConstLabel final { struct FsmStateSpace final { std::vector> states; // User label and encoded value - std::unordered_map labels; // Encoded value to label + std::unordered_map + labels; // Encoded value to label AstVar* stateVarp = nullptr; // Tracked FSM state variable bool enumBacked = false; // Whether states came from an enum declaration }; @@ -471,15 +521,15 @@ class FsmDetectVisitor final : public VNVisitor { firstIfp->thensp(), resetStateVscp, reg.resetArcs()); if (resetStatus == ResetAssignStatus::NONE || resetStateVscp != vscp) { reg.resetArcs().clear(); - FsmStateValue resetValue = 0; + FsmStateValue resetValue; AstNode* const thenNodep = FsmDetectVisitor::singleMeaningfulBranch(firstIfp->thensp()); UASSERT_OBJ(thenNodep, firstIfp, "one-block reset fallback requires a non-empty reset branch"); - if (FsmDetectVisitor::directConstStateAssignNode(thenNodep, resetStateVscp, - resetValue) - && resetStateVscp == vscp) { - reg.resetArcs().emplace_back(resetValue, firstIfp->thensp()); + AstNodeAssign* const resetAssp = FsmDetectVisitor::directConstStateAssignNode( + thenNodep, resetStateVscp, resetValue); + if (resetAssp && resetStateVscp == vscp) { + reg.resetArcs().emplace_back(resetValue, resetAssp); } } else if (resetStatus == ResetAssignStatus::MULTI_SAME_STATE) { reg.resetArcs().clear(); @@ -743,7 +793,7 @@ class FsmDetectVisitor final : public VNVisitor { UASSERT_OBJ(nodep, stmtp, "Empty reset branch unexpectedly survived to FSM detection"); for (;; nodep = nodep->nextp()) { AstVarScope* assignStateVscp = nullptr; - FsmStateValue value = 0; + FsmStateValue value; AstNodeAssign* const assp = directConstStateAssignNode(nodep, assignStateVscp, value); if (!assp) return ResetAssignStatus::NONE; if (!stateVscp) stateVscp = assignStateVscp; @@ -829,11 +879,11 @@ class FsmDetectVisitor final : public VNVisitor { } AstNodeAssign* const assp = directStateAssign(itemp->stmtsp(), stateVscp); if (assp) { - FsmStateValue toValue = 0; + FsmStateValue toValue; if (constValueStatus(assp->rhsp(), toValue) == ConstValueStatus::OK) return assp; } - FsmStateValue thenValue = 0; - FsmStateValue elseValue = 0; + FsmStateValue thenValue; + FsmStateValue elseValue; if (directStateCondConstAssign(itemp->stmtsp(), stateVscp, thenValue, elseValue)) { return assp; } @@ -857,10 +907,10 @@ class FsmDetectVisitor final : public VNVisitor { static AstNode* caseItemSupportedArcNodeLike(AstNode* stmtsp, AstVarScope* stateVscp) { if (AstNodeAssign* const assp = directStateAssign(stmtsp, stateVscp)) { - FsmStateValue toValue = 0; + FsmStateValue toValue; if (constValueStatus(assp->rhsp(), toValue) == ConstValueStatus::OK) return assp; - FsmStateValue thenValue = 0; - FsmStateValue elseValue = 0; + FsmStateValue thenValue; + FsmStateValue elseValue; if (directStateCondConstAssign(stmtsp, stateVscp, thenValue, elseValue)) return assp; } return nullptr; @@ -878,24 +928,22 @@ class FsmDetectVisitor final : public VNVisitor { // Prefer user labels in reports. Forced non-enum FSMs prepopulate synthetic // labels, so all emitted arcs should already have a known label here. - static string labelForValue(const std::unordered_map& labels, - FsmStateValue value) { + static string labelForValue( + const std::unordered_map& labels, + const FsmStateValue& value) { return labels.at(value).text; } // The extractor only models constant-valued state transitions, and by the // time detect runs those values have already been constant-folded. - enum class ConstValueStatus : uint8_t { OK, NOT_CONST, XZ, WIDE }; + enum class ConstValueStatus : uint8_t { OK, NOT_CONST, XZ }; static ConstValueStatus constValueStatus(AstNodeExpr* exprp, FsmStateValue& value) { const AstConst* const constp = VN_CAST(exprp, Const); if (!constp) return ConstValueStatus::NOT_CONST; const V3Number& num = constp->num(); if (num.isAnyXZ()) return ConstValueStatus::XZ; - // Some callers are still only probing candidate shapes, so wide constants - // should reject the candidate instead of reporting a V3Number error. - if (constp->width() > 32) return ConstValueStatus::WIDE; - value = constp->toUInt(); + value = FsmStateValue{num}; return ConstValueStatus::OK; } @@ -912,7 +960,7 @@ class FsmDetectVisitor final : public VNVisitor { valuep = eqp->lhsp(); } - FsmStateValue value = 0; + FsmStateValue value; if (constValueStatus(valuep, value) != ConstValueStatus::OK) return false; cmp.stateVscp = vrefp->varScopep(); cmp.valuep = valuep; @@ -1023,7 +1071,7 @@ class FsmDetectVisitor final : public VNVisitor { static bool collectIfChain(AstIf* ifp, const FsmAliasMap& aliases, FsmIfChainCandidate& chain) { chain.ifp = ifp; - std::unordered_set seenValues; + std::unordered_set seenValues; AstIf* curp = ifp; // Only the top-level spine represents dispatch; treating nested branch // logic as additional source states would invent transitions. @@ -1032,7 +1080,7 @@ class FsmDetectVisitor final : public VNVisitor { bool hasGuard = false; if (!resolveIfPredicate(curp->condp(), aliases, cmp, hasGuard)) return false; if (chain.compareVscp && chain.compareVscp != cmp.stateVscp) return false; - if (!seenValues.insert(cmp.value).second) return false; + if (!seenValues.insert(cmp.value.key()).second) return false; chain.compareVscp = cmp.stateVscp; chain.branches.push_back( FsmIfBranch{curp, curp->thensp(), cmp.valuep, cmp.value, hasGuard}); @@ -1055,19 +1103,19 @@ class FsmDetectVisitor final : public VNVisitor { // instrumentation for that edge rather than silently dropping it or turning // optional coverage into a hard compile failure. static bool validateKnownStateValue(AstNode* nodep, const FsmStateSpace& stateSpace, - FsmStateValue value, const string& role) { + const FsmStateValue& value, const string& role) { if (stateSpace.labels.find(value) != stateSpace.labels.end()) return true; if (stateSpace.enumBacked) { const string enumRole = role == "source" ? "case item value" : "assigned value"; nodep->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on enum state variable " + stateSpace.stateVarp->prettyNameQ() + ": " + enumRole - + " " + cvtToStr(value) + + " " + value.warnText() + " is not present in the declared enum"); return false; } nodep->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on non-enum state variable " + stateSpace.stateVarp->prettyNameQ() + ": " + role + " value " - + cvtToStr(value) + + value.warnText() + " is not present in the inferred state space"); return false; } @@ -1078,7 +1126,7 @@ class FsmDetectVisitor final : public VNVisitor { return StateConstLabel{constp->name(), false, 0}; } - static void updateStateLabel(FsmStateSpace& stateSpace, FsmStateValue value, + static void updateStateLabel(FsmStateSpace& stateSpace, const FsmStateValue& value, const StateConstLabel& label) { stateSpace.states.at(stateSpace.labels.at(value).stateIndex).first = label.text; } @@ -1102,12 +1150,13 @@ class FsmDetectVisitor final : public VNVisitor { = collectConstStateAssigns(ifp->thensp(), resetStateVscp, cand.resetArcs()); if (resetStatus == ResetAssignStatus::NONE) { cand.resetArcs().clear(); - FsmStateValue resetValue = 0; + FsmStateValue resetValue; AstNode* const thenNodep = singleMeaningfulBranch(ifp->thensp()); UASSERT_OBJ(thenNodep, ifp, "reset fallback requires a non-empty reset branch"); - if (!directConstStateAssignNode(thenNodep, resetStateVscp, resetValue)) - return false; - cand.resetArcs().emplace_back(resetValue, ifp->thensp()); + AstNodeAssign* const resetAssp + = directConstStateAssignNode(thenNodep, resetStateVscp, resetValue); + if (!resetAssp) return false; + cand.resetArcs().emplace_back(resetValue, resetAssp); } else if (resetStatus == ResetAssignStatus::MULTI_SAME_STATE) { cand.resetArcs().clear(); } @@ -1119,7 +1168,7 @@ class FsmDetectVisitor final : public VNVisitor { cand.hasResetCond(cand.resetCond().varScopep != nullptr); } else { AstNodeExpr* resetCondp = nullptr; - FsmStateValue resetValue = 0; + FsmStateValue resetValue; if (AstNodeAssign* const assp = directCondStateVarAssign(nodep, stateVscp, nextVscp, resetCondp, resetValue)) { cand.resetArcs().emplace_back(resetValue, assp); @@ -1139,32 +1188,18 @@ class FsmDetectVisitor final : public VNVisitor { return true; } - // Helper: process a single condition expression and add it to the state space. - // Returns true on success, false if the state space is invalid. - static bool addCondToStateSpace(AstNodeExpr* condp, FsmStateSpace& stateSpace) { - FsmStateValue value = 0; - const ConstValueStatus status = constValueStatus(condp, value); - if (status != ConstValueStatus::OK) { - if (status == ConstValueStatus::XZ) { - condp->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on non-enum " - "state variable " - + stateSpace.stateVarp->prettyNameQ() - + " with X/Z state encoding values"); - } - return false; - } - AstConst* const constp = VN_AS(condp, Const); - const StateConstLabel label = stateLabelForConst(constp); + static bool addValueToStateSpace(AstNode* nodep, FsmStateSpace& stateSpace, + const FsmStateValue& value, StateConstLabel label) { const auto labelIt = stateSpace.labels.find(value); if (labelIt != stateSpace.labels.end()) { StateConstLabel& existingLabel = labelIt->second; if (existingLabel.text != label.text && existingLabel.fromParam && label.fromParam) { - condp->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on non-enum " + nodep->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on non-enum " "state variable " + stateSpace.stateVarp->prettyNameQ() + " with multiple labels for the same value " - + cvtToStr(value) + ": " + existingLabel.text + " and " - + label.text); + + value.warnText() + ": " + existingLabel.text + + " and " + label.text); return false; } if (!existingLabel.fromParam && label.fromParam) { @@ -1181,6 +1216,51 @@ class FsmDetectVisitor final : public VNVisitor { return true; } + // Helper: process a single observed state expression and add it to the state space + // Returns true on success, false if the state space is invalid + static bool addExprToStateSpace(AstNodeExpr* valuep, FsmStateSpace& stateSpace) { + FsmStateValue value; + const ConstValueStatus status = constValueStatus(valuep, value); + if (status != ConstValueStatus::OK) { + if (status == ConstValueStatus::XZ) { + valuep->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on non-enum " + "state variable " + + stateSpace.stateVarp->prettyNameQ() + + " with X/Z state encoding values"); + } + return false; + } + AstConst* const constp = VN_AS(valuep, Const); + return addValueToStateSpace(valuep, stateSpace, value, stateLabelForConst(constp)); + } + + static bool addOptionalTargetExprToStateSpace(AstNodeExpr* valuep, FsmStateSpace& stateSpace) { + FsmStateValue value; + const ConstValueStatus status = constValueStatus(valuep, value); + if (status != ConstValueStatus::OK) { + valuep->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on non-enum " + "state variable " + + stateSpace.stateVarp->prettyNameQ() + + " with non-constant target state values"); + return false; + } + AstConst* const constp = VN_AS(valuep, Const); + return addValueToStateSpace(valuep, stateSpace, value, stateLabelForConst(constp)); + } + + static void addResetTargetsToStateSpace(const std::vector& resetArcs, + FsmStateSpace& stateSpace) { + for (const FsmResetArcDesc& resetArc : resetArcs) { + StateConstLabel label{resetArc.toValue().ascii(), false, 0}; + if (AstConst* const constp = VN_CAST(resetArc.nodep()->rhsp(), Const)) { + label = stateLabelForConst(constp); + } + UASSERT_OBJ( + addValueToStateSpace(resetArc.nodep(), stateSpace, resetArc.toValue(), label), + resetArc.nodep(), "reset target labels should be unambiguous"); + } + } + // Build the Phase 1 state space from the tracked registered state // variable, not from whichever signal the transition statement happened to use. static bool collectDeclaredStateSpace(AstNode* warnNodep, AstVarScope* stateVscp, @@ -1192,19 +1272,11 @@ class FsmDetectVisitor final : public VNVisitor { stateSpace.stateVarp = stateVarp; if (enump) { - if (stateVscp->width() > 32) { - warnNodep->v3warn(COVERIGN, - "Ignoring unsupported: FSM coverage on enum-typed state " - "variable " - + stateSpace.stateVarp->prettyNameQ() + " with width " - + cvtToStr(stateVscp->width()) + " wider than 32 bits"); - return false; - } stateSpace.enumBacked = true; for (AstEnumItem* itemp = enump->itemsp(); itemp; itemp = VN_AS(itemp->nextp(), EnumItem)) { const AstConst* const constp = VN_AS(itemp->valuep(), Const); - const FsmStateValue value = constp->toUInt(); + const FsmStateValue value{constp->num()}; const size_t stateIndex = stateSpace.states.size(); stateSpace.states.emplace_back(itemp->name(), value); stateSpace.labels.emplace(value, @@ -1214,34 +1286,17 @@ class FsmDetectVisitor final : public VNVisitor { } if (forced) { - const int width = stateVarp->width(); - // Forced non-enum FSMs have no declared state list, so enumeration - // must stay small enough for this scalar graph representation. - if (width >= 31) return false; - const unsigned stateCount = 1U << width; - for (FsmStateValue value = 0; value < stateCount; ++value) { - const string label = "S" + cvtToStr(value); - const size_t stateIndex = stateSpace.states.size(); - stateSpace.states.emplace_back(label, value); - stateSpace.labels.emplace(value, StateConstLabel{label, false, stateIndex}); - } + needsSourceValues = true; return true; } - if (stateVscp->width() > 32) { - warnNodep->v3warn(COVERIGN, "Ignoring unsupported: FSM coverage on non-enum state " - "variable " - + stateSpace.stateVarp->prettyNameQ() + " with width " - + cvtToStr(stateVscp->width()) - + " wider than 32 bits"); - return false; - } needsSourceValues = true; return true; } template static bool collectStateSpaceFromValues(AstNode* warnNodep, AstVarScope* stateVscp, + const std::vector& resetArcs, FsmStateSpace& stateSpace, const T_ValuepVisitor& visitValueps) { bool needsSourceValues = false; @@ -1251,23 +1306,41 @@ class FsmDetectVisitor final : public VNVisitor { return false; } if (!needsSourceValues) return true; + addResetTargetsToStateSpace(resetArcs, stateSpace); if (!visitValueps( - [&](AstNodeExpr* valuep) { return addCondToStateSpace(valuep, stateSpace); })) { + [&](AstNodeExpr* valuep) { return addExprToStateSpace(valuep, stateSpace); })) { return false; } return stateSpace.states.size() >= 2; } - static bool collectStateSpace(AstCase* casep, AstVarScope* stateVscp, + static bool collectStateSpace(AstCase* casep, AstVarScope* stateVscp, AstVarScope* assignVscp, + const std::vector& resetArcs, FsmStateSpace& stateSpace) { return collectStateSpaceFromValues( - casep, stateVscp, stateSpace, [casep](const auto& visitValuep) { + casep, stateVscp, resetArcs, stateSpace, + [casep, assignVscp, &stateSpace](const auto& visitValuep) { for (AstCaseItem* itemp = casep->itemsp(); itemp; itemp = VN_AS(itemp->nextp(), CaseItem)) { - if (itemp->isDefault()) continue; - for (AstNodeExpr* condp = itemp->condsp(); condp; - condp = VN_AS(condp->nextp(), NodeExpr)) { - if (!visitValuep(condp)) return false; + if (!itemp->isDefault()) { + for (AstNodeExpr* condp = itemp->condsp(); condp; + condp = VN_AS(condp->nextp(), NodeExpr)) { + if (!visitValuep(condp)) return false; + } + } + if (AstNodeAssign* const assp + = directStateAssign(itemp->stmtsp(), assignVscp)) { + FsmStateValue thenValue; + FsmStateValue elseValue; + AstCond* const condp = VN_CAST(assp->rhsp(), Cond); + if (condp + && directStateCondConstAssign(itemp->stmtsp(), assignVscp, thenValue, + elseValue)) { + if (!visitValuep(condp->thenp())) return false; + if (!visitValuep(condp->elsep())) return false; + } else if (!addOptionalTargetExprToStateSpace(assp->rhsp(), stateSpace)) { + return false; + } } } return true; @@ -1275,14 +1348,32 @@ class FsmDetectVisitor final : public VNVisitor { } static bool collectStateSpace(const FsmIfChainCandidate& chain, AstVarScope* stateVscp, + AstVarScope* assignVscp, + const std::vector& resetArcs, FsmStateSpace& stateSpace) { return collectStateSpaceFromValues( - chain.ifp, stateVscp, stateSpace, [&chain](const auto& visitValuep) { + chain.ifp, stateVscp, resetArcs, stateSpace, + [&chain, assignVscp, &stateSpace](const auto& visitValuep) { for (const FsmIfBranch& branch : chain.branches) { // Reaching this point with an unresolvable source value // would mean the if-chain classifier and emitter disagree. UASSERT_OBJ(visitValuep(branch.valuep), branch.valuep, "FSM if-chain source values should be prevalidated"); + AstNodeAssign* const assp = directStateAssign(branch.stmtsp, assignVscp); + UASSERT_OBJ(assp, branch.stmtsp, + "FSM if-chain target values should be prevalidated"); + FsmStateValue thenValue; + FsmStateValue elseValue; + AstCond* const condp = VN_CAST(assp->rhsp(), Cond); + if (condp) { + UASSERT_OBJ(directStateCondConstAssign(branch.stmtsp, assignVscp, + thenValue, elseValue), + condp, "FSM if-chain ternary targets should be prevalidated"); + if (!visitValuep(condp->thenp())) return false; + if (!visitValuep(condp->elsep())) return false; + } else if (!addOptionalTargetExprToStateSpace(assp->rhsp(), stateSpace)) { + return false; + } } return true; }); @@ -1297,11 +1388,11 @@ class FsmDetectVisitor final : public VNVisitor { std::vector> froms; if (itemp->isDefault()) { if (!inclCond) return false; - froms.emplace_back("default", 0); + froms.emplace_back("default", FsmStateValue{}); } else { for (AstNodeExpr* condp = itemp->condsp(); condp; condp = VN_AS(condp->nextp(), NodeExpr)) { - FsmStateValue value = 0; + FsmStateValue value; if (constValueStatus(condp, value) != ConstValueStatus::OK) continue; if (!validateKnownStateValue(condp, stateSpace, value, "source")) return true; froms.emplace_back(labelForValue(stateSpace.labels, value), value); @@ -1310,7 +1401,7 @@ class FsmDetectVisitor final : public VNVisitor { } if (AstNodeAssign* const assp = directStateAssign(itemp->stmtsp(), stateVscp)) { - FsmStateValue toValue = 0; + FsmStateValue toValue; const ConstValueStatus status = constValueStatus(assp->rhsp(), toValue); if (status == ConstValueStatus::OK) { if (!validateKnownStateValue(assp, stateSpace, toValue, "target")) return true; @@ -1322,15 +1413,15 @@ class FsmDetectVisitor final : public VNVisitor { } } - FsmStateValue thenValue = 0; - FsmStateValue elseValue = 0; + FsmStateValue thenValue; + FsmStateValue elseValue; if (directStateCondConstAssign(itemp->stmtsp(), stateVscp, thenValue, elseValue) || ifStateConstAssign(itemp->stmtsp(), stateVscp, thenValue, elseValue)) { if (!validateKnownStateValue(itemp->stmtsp(), stateSpace, thenValue, "target")) return true; if (!validateKnownStateValue(itemp->stmtsp(), stateSpace, elseValue, "target")) return true; - for (const FsmStateValue branchValue : {thenValue, elseValue}) { + for (const FsmStateValue& branchValue : {thenValue, elseValue}) { for (const std::pair& from : froms) { graph.addArc(from.second, branchValue, false, true, itemp->isDefault(), itemp->stmtsp()->fileline()); @@ -1347,7 +1438,7 @@ class FsmDetectVisitor final : public VNVisitor { bool isDefault, bool forceCond) { AstNodeAssign* const assp = directStateAssign(stmtsp, stateVscp); UASSERT_OBJ(assp, stmtsp, "FSM if-chain branch should have been prevalidated"); - FsmStateValue toValue = 0; + FsmStateValue toValue; const ConstValueStatus status = constValueStatus(assp->rhsp(), toValue); if (status == ConstValueStatus::OK) { if (!validateKnownStateValue(assp, stateSpace, toValue, "target")) return; @@ -1357,15 +1448,15 @@ class FsmDetectVisitor final : public VNVisitor { return; } - FsmStateValue thenValue = 0; - FsmStateValue elseValue = 0; + FsmStateValue thenValue; + FsmStateValue elseValue; const bool condAssign = directStateCondConstAssign(stmtsp, stateVscp, thenValue, elseValue); UASSERT_OBJ(condAssign, stmtsp, "FSM if-chain branch should be a direct constant transition"); if (!validateKnownStateValue(stmtsp, stateSpace, thenValue, "target")) return; if (!validateKnownStateValue(stmtsp, stateSpace, elseValue, "target")) return; - for (const FsmStateValue branchValue : {thenValue, elseValue}) { + for (const FsmStateValue& branchValue : {thenValue, elseValue}) { graph.addArc(fromValue, branchValue, false, true, isDefault, stmtsp->fileline()); } } @@ -1391,7 +1482,8 @@ class FsmDetectVisitor final : public VNVisitor { if (!validateKnownStateValue(resetArc.nodep(), stateSpace, resetArc.toValue(), "target")) continue; - graph.addArc(0, resetArc.toValue(), true, false, false, resetArc.nodep()->fileline()); + graph.addArc(FsmStateValue{}, resetArc.toValue(), true, false, false, + resetArc.nodep()->fileline()); } } @@ -1402,7 +1494,7 @@ class FsmDetectVisitor final : public VNVisitor { UASSERT_OBJ(assignVscp, casep, "FSM case processing requires a non-null assignment var"); AstVarScope* const stateVscp = reg.stateVscp(); FsmStateSpace stateSpace; - if (!collectStateSpace(casep, stateVscp, stateSpace)) return; + if (!collectStateSpace(casep, stateVscp, assignVscp, reg.resetArcs(), stateSpace)) return; DetectedFsm& entry = m_state.fsms()[stateVscp]; if (!entry.graphp) { entry.graphp.reset(new FsmGraph{}); @@ -1435,7 +1527,7 @@ class FsmDetectVisitor final : public VNVisitor { "FSM if-chain processing requires a non-null assignment var"); AstVarScope* const stateVscp = reg.stateVscp(); FsmStateSpace stateSpace; - if (!collectStateSpace(chain, stateVscp, stateSpace)) return; + if (!collectStateSpace(chain, stateVscp, assignVscp, reg.resetArcs(), stateSpace)) return; DetectedFsm& entry = m_state.fsms()[stateVscp]; // Case candidates keep ownership of existing graphs; reaching this path // means the if-chain is the only supported dispatch for this FSM. @@ -1698,8 +1790,9 @@ class FsmLowerVisitor final { // METHODS // Rebuild a state-typed constant using the tracked state variable // width/sign so emitted comparisons match the original representation. - static AstConst* makeStateConst(FileLine* flp, AstVarScope* vscp, FsmStateValue value) { - V3Number num{flp, vscp->width(), value}; + static AstConst* makeStateConst(FileLine* flp, AstVarScope* vscp, const FsmStateValue& value) { + V3Number num{static_cast(nullptr), vscp->width()}; + num.opAssign(value.num()); num.isSigned(vscp->dtypep()->isSigned()); return new AstConst{flp, num}; } diff --git a/test_regress/t/t_cover_fsm_beginif.out b/test_regress/t/t_cover_fsm_beginif.out index 9f49edc43..e7192d335 100644 --- a/test_regress/t/t_cover_fsm_beginif.out +++ b/test_regress/t/t_cover_fsm_beginif.out @@ -151,14 +151,13 @@ if (rst) state <= 2'd0; %000003 else if (state == 2'd0) state <= 2'd1; // [FSM coverage] -%000001 // [fsm_arc t.literal_forced_u.state::ANY->S0[reset]] [reset arc, excluded from %] -%000003 // [fsm_arc t.literal_forced_u.state::S0->S1] -%000003 // [fsm_arc t.literal_forced_u.state::S1->S2] -%000003 // [fsm_arc t.literal_forced_u.state::S2->S0] -%000003 // [fsm_state t.literal_forced_u.state::S0] -%000003 // [fsm_state t.literal_forced_u.state::S1] -%000003 // [fsm_state t.literal_forced_u.state::S2] -%000000 // [fsm_state t.literal_forced_u.state::S3] *** UNCOVERED *** +%000003 // [fsm_arc t.literal_forced_u.state::2'h0->2'h1] +%000003 // [fsm_arc t.literal_forced_u.state::2'h1->2'h2] +%000003 // [fsm_arc t.literal_forced_u.state::2'h2->2'h0] +%000001 // [fsm_arc t.literal_forced_u.state::ANY->2'h0[reset]] [reset arc, excluded from %] +%000003 // [fsm_state t.literal_forced_u.state::2'h0] +%000003 // [fsm_state t.literal_forced_u.state::2'h1] +%000003 // [fsm_state t.literal_forced_u.state::2'h2] else if (state == 2'd1) state <= 2'd2; else if (state == 2'd2) state <= 2'd0; end diff --git a/test_regress/t/t_cover_fsm_enumwide_bad.out b/test_regress/t/t_cover_fsm_enumwide_bad.out deleted file mode 100644 index 42994dce4..000000000 --- a/test_regress/t/t_cover_fsm_enumwide_bad.out +++ /dev/null @@ -1,6 +0,0 @@ -%Warning-COVERIGN: t/t_cover_fsm_enumwide_bad.v:26:7: Ignoring unsupported: FSM coverage on enum-typed state variable 't.state' with width 33 wider than 32 bits - 26 | case (state) - | ^~~~ - ... For warning description see https://verilator.org/warn/COVERIGN?v=latest - ... Use "/* verilator lint_off COVERIGN */" and lint_on around source to disable this message. -%Error: Exiting due to diff --git a/test_regress/t/t_cover_fsm_enumwide_bad.py b/test_regress/t/t_cover_fsm_enumwide_bad.py deleted file mode 100755 index 0acd5c320..000000000 --- a/test_regress/t/t_cover_fsm_enumwide_bad.py +++ /dev/null @@ -1,18 +0,0 @@ -#!/usr/bin/env python3 -# DESCRIPTION: Verilator: FSM enum width limit test -# -# 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') - -# FSM coverage currently stores recovered enum state values in the detector's -# 32-bit internal representation, so wider enum-backed FSMs are rejected. -test.lint(verilator_flags2=["--coverage-fsm"], fails=True, expect_filename=test.golden_filename) - -test.passes() diff --git a/test_regress/t/t_cover_fsm_enumwide_bad.v b/test_regress/t/t_cover_fsm_enumwide_bad.v deleted file mode 100644 index 4bb845dd1..000000000 --- a/test_regress/t/t_cover_fsm_enumwide_bad.v +++ /dev/null @@ -1,33 +0,0 @@ -// DESCRIPTION: Verilator: FSM enum width limit rejects >32-bit enums -// -// This file ONLY is placed under the Creative Commons Public Domain. -// SPDX-FileCopyrightText: 2026 Wilson Snyder -// SPDX-License-Identifier: CC0-1.0 - -module t ( - input logic clk, - input logic rst -); - - typedef enum logic [32:0] { - S0 = 33'd0, - S1 = 33'd1 - } state_t; - - state_t state; - - // FSM coverage currently supports enum-backed state variables only up to - // 32 bits wide, so this wider enum should be rejected at FSM detection time. - always_ff @(posedge clk) begin - if (rst) begin - state <= S0; - end - else begin - case (state) - S0: state <= S1; - default: state <= S0; - endcase - end - end - -endmodule diff --git a/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.out b/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.out index e433db988..7416b7aa4 100644 --- a/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.out +++ b/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.out @@ -27,4 +27,7 @@ %Warning-COVERIGN: t/t_cover_fsm_if_unknown_enum_multi_bad.v:249:32: Ignoring unsupported: FSM coverage on enum state variable 't.unknown_if_else_target_u.state_q': assigned value 3 is not present in the declared enum 249 | if (state_q == S0) state_d = sel ? S1 : 2'd3; | ^ +%Warning-COVERIGN: t/t_cover_fsm_if_unknown_enum_multi_bad.v:274:19: Ignoring unsupported: FSM coverage on enum state variable 't.unknown_wide_direct_u.state_q': assigned value 40'hffffffffff is not present in the declared enum + 274 | S0: state_d = 40'hffff_ffff_ff; + | ^ %Error: Exiting due to diff --git a/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.v b/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.v index ddd3c8231..dabebd9e7 100644 --- a/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.v +++ b/test_regress/t/t_cover_fsm_if_unknown_enum_multi_bad.v @@ -256,6 +256,32 @@ module unknown_if_else_target ( end endmodule +module unknown_wide_direct ( + input logic clk +); + typedef enum logic [39:0] { + S0 = 40'h0000_0000_01, + S1 = 40'h8000_0000_02 + } state_t; + + state_t state_q /*verilator fsm_state*/; + state_t state_d; + + always_comb begin + state_d = state_q; + case (state_q) + /* verilator lint_off ENUMVALUE */ + S0: state_d = 40'hffff_ffff_ff; + /* verilator lint_on ENUMVALUE */ + default: state_d = S0; + endcase + end + + always_ff @(posedge clk) begin + state_q <= state_d; + end +endmodule + module t; logic clk; unknown_then unknown_then_u (.clk(clk)); @@ -267,4 +293,5 @@ module t; unknown_if_direct_target unknown_if_direct_target_u (.clk(clk)); unknown_if_then_target unknown_if_then_target_u (.clk(clk)); unknown_if_else_target unknown_if_else_target_u (.clk(clk)); + unknown_wide_direct unknown_wide_direct_u (.clk(clk)); endmodule diff --git a/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.out b/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.out index e467e12f5..b0abea925 100644 --- a/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.out +++ b/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.out @@ -1,15 +1,30 @@ -%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:33:7: Ignoring unsupported: FSM coverage on non-enum state variable 't.wide_state' with width 33 wider than 32 bits - 33 | case (wide_state) - | ^~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:68:9: Ignoring unsupported: FSM coverage on non-enum state variable 't.duplicate_state' with multiple labels for the same value 0: IDLE and RESET + 68 | RESET: duplicate_state <= IDLE; + | ^~~~~ ... For warning description see https://verilator.org/warn/COVERIGN?v=latest ... Use "/* verilator lint_off COVERIGN */" and lint_on around source to disable this message. -%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:48:28: Ignoring unsupported: FSM coverage on non-enum state variable 't.target_state': target value 2 is not present in the inferred state space - 48 | 2'h1: target_state <= 2'h2; - | ^~ -%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:62:9: Ignoring unsupported: FSM coverage on non-enum state variable 't.duplicate_state' with multiple labels for the same value 0: IDLE and RESET - 62 | RESET: duplicate_state <= IDLE; - | ^~~~~ -%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:78:9: Ignoring unsupported: FSM coverage on non-enum state variable 't.xz_case_state' with X/Z state encoding values - 78 | 2'b1x: xz_case_state <= 2'h0; +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:84:9: Ignoring unsupported: FSM coverage on non-enum state variable 't.xz_case_state' with X/Z state encoding values + 84 | 2'b1x: xz_case_state <= 2'h0; | ^~~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:97:64: Ignoring unsupported: FSM coverage on non-enum state variable 't.duplicate_ternary_target_state' with multiple labels for the same value 0: IDLE and RESET + 97 | IDLE: duplicate_ternary_target_state <= start ? BUSY : RESET; + | ^~~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:110:62: Ignoring unsupported: FSM coverage on non-enum state variable 't.duplicate_ternary_then_target_state' with multiple labels for the same value 0: IDLE and RESET + 110 | IDLE: duplicate_ternary_then_target_state <= start ? RESET : BUSY; + | ^~~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:125:36: Ignoring unsupported: FSM coverage on non-enum state variable 't.duplicate_if_target_state' with multiple labels for the same value 0: IDLE and RESET + 125 | duplicate_if_target_state <= RESET; + | ^~~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:134:57: Ignoring unsupported: FSM coverage on non-enum state variable 't.duplicate_if_ternary_then_target_state' with multiple labels for the same value 0: IDLE and RESET + 134 | duplicate_if_ternary_then_target_state <= start ? RESET : BUSY; + | ^~~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:146:64: Ignoring unsupported: FSM coverage on non-enum state variable 't.duplicate_if_ternary_else_target_state' with multiple labels for the same value 0: IDLE and RESET + 146 | duplicate_if_ternary_else_target_state <= start ? BUSY : RESET; + | ^~~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:201:37: Ignoring unsupported: FSM coverage on non-enum state variable 't.xz_rhs_probe_state' with non-constant target state values + 201 | 2'h0: xz_rhs_probe_state <= 2'b0x; + | ^~~~~ +%Warning-COVERIGN: t/t_cover_fsm_nonenum_unsupported_bad.v:214:54: Ignoring unsupported: FSM coverage on non-enum state variable 't.nonconst_ternary_target_state' with non-constant target state values + 214 | 2'h0: nonconst_ternary_target_state <= start ? 2'h1 : {1'b0, start}; + | ^ %Error: Exiting due to diff --git a/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.v b/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.v index 139b1ee2f..d1fc56b2a 100644 --- a/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.v +++ b/test_regress/t/t_cover_fsm_nonenum_unsupported_bad.v @@ -18,11 +18,17 @@ module t ( logic [32:0] wide_state; logic [1:0] target_state; logic [1:0] duplicate_state; + logic [1:0] duplicate_ternary_target_state; + logic [1:0] duplicate_ternary_then_target_state; + logic [1:0] duplicate_if_target_state; + logic [1:0] duplicate_if_ternary_then_target_state; + logic [1:0] duplicate_if_ternary_else_target_state; logic [1:0] xz_case_state; logic [1:0] duplicate_literal_state; logic [1:0] underscore_state; logic [1:0] xz_reset_probe_state; - logic [1:0] xz_rhs_probe_state; + logic [1:0] xz_rhs_probe_state /*verilator fsm_state*/; + logic [1:0] nonconst_ternary_target_state /*verilator fsm_state*/; logic [32:0] wide_if_state /*verilator fsm_state*/; always_ff @(posedge clk) begin @@ -82,6 +88,68 @@ module t ( end end + always_ff @(posedge clk) begin + if (rst) begin + duplicate_ternary_target_state <= IDLE; + end + else begin + case (duplicate_ternary_target_state) + IDLE: duplicate_ternary_target_state <= start ? BUSY : RESET; + BUSY: duplicate_ternary_target_state <= IDLE; + default: duplicate_ternary_target_state <= IDLE; + endcase + end + end + + always_ff @(posedge clk) begin + if (rst) begin + duplicate_ternary_then_target_state <= IDLE; + end + else begin + case (duplicate_ternary_then_target_state) + IDLE: duplicate_ternary_then_target_state <= start ? RESET : BUSY; + BUSY: duplicate_ternary_then_target_state <= IDLE; + default: duplicate_ternary_then_target_state <= IDLE; + endcase + end + end + + always_ff @(posedge clk) begin + if (rst) begin + duplicate_if_target_state <= IDLE; + end + else if (duplicate_if_target_state == IDLE) begin + duplicate_if_target_state <= BUSY; + end + else if (duplicate_if_target_state == BUSY) begin + duplicate_if_target_state <= RESET; + end + end + + always_ff @(posedge clk) begin + if (rst) begin + duplicate_if_ternary_then_target_state <= IDLE; + end + else if (duplicate_if_ternary_then_target_state == IDLE) begin + duplicate_if_ternary_then_target_state <= start ? RESET : BUSY; + end + else if (duplicate_if_ternary_then_target_state == BUSY) begin + duplicate_if_ternary_then_target_state <= IDLE; + end + end + + always_ff @(posedge clk) begin + if (rst) begin + duplicate_if_ternary_else_target_state <= IDLE; + end + else if (duplicate_if_ternary_else_target_state == IDLE) begin + duplicate_if_ternary_else_target_state <= start ? BUSY : RESET; + end + else if (duplicate_if_ternary_else_target_state == BUSY) begin + duplicate_if_ternary_else_target_state <= IDLE; + end + end + always_ff @(posedge clk) begin if (rst) begin duplicate_literal_state <= 2'h0; @@ -137,6 +205,19 @@ module t ( end end + always_ff @(posedge clk) begin + if (rst) begin + nonconst_ternary_target_state <= 2'h0; + end + else begin + case (nonconst_ternary_target_state) + 2'h0: nonconst_ternary_target_state <= start ? 2'h1 : {1'b0, start}; + 2'h1: nonconst_ternary_target_state <= 2'h0; + default: nonconst_ternary_target_state <= 2'h0; + endcase + end + end + always_ff @(posedge clk) begin if (rst) begin wide_if_state <= 33'h0; diff --git a/test_regress/t/t_cover_fsm_policy_accept_multi.out b/test_regress/t/t_cover_fsm_policy_accept_multi.out index 1875352a1..ca501145c 100644 --- a/test_regress/t/t_cover_fsm_policy_accept_multi.out +++ b/test_regress/t/t_cover_fsm_policy_accept_multi.out @@ -97,13 +97,12 @@ else begin %000002 case (state) // [FSM coverage] -%000001 // [fsm_arc t.forced_u.state::ANY->S0[reset]] [reset arc, excluded from %] -%000002 // [fsm_arc t.forced_u.state::S0->S1] -%000002 // [fsm_arc t.forced_u.state::S1->S2] -%000001 // [fsm_state t.forced_u.state::S0] -%000002 // [fsm_state t.forced_u.state::S1] -%000002 // [fsm_state t.forced_u.state::S2] -%000000 // [fsm_state t.forced_u.state::S3] *** UNCOVERED *** +%000002 // [fsm_arc t.forced_u.state::2'h0->2'h1] +%000002 // [fsm_arc t.forced_u.state::2'h1->2'h2] +%000001 // [fsm_arc t.forced_u.state::ANY->2'h0[reset]] [reset arc, excluded from %] +%000001 // [fsm_state t.forced_u.state::2'h0] +%000002 // [fsm_state t.forced_u.state::2'h1] +%000002 // [fsm_state t.forced_u.state::2'h2] 2'd0: state <= 2'd1; 2'd1: state <= 2'd2; default: state <= 2'd0; diff --git a/test_regress/t/t_cover_fsm_transition_shapes_multi.out b/test_regress/t/t_cover_fsm_transition_shapes_multi.out index 8b8f255e5..1bb0807e2 100644 --- a/test_regress/t/t_cover_fsm_transition_shapes_multi.out +++ b/test_regress/t/t_cover_fsm_transition_shapes_multi.out @@ -1 +1,11 @@ # SystemC::Coverage-3 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_arcpagev_fsm_arc/$rootot.forced_wide_u.state::31'h0->31'h1Fvt.forced_wide_u.stateFf31'h0Ft31'h1htop.TOP' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_arcpagev_fsm_arc/$rootot.forced_wide_u.state::31'h1->31'h2Fvt.forced_wide_u.stateFf31'h1Ft31'h2htop.TOP' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_arcpagev_fsm_arc/$rootot.forced_wide_u.state::ANY->31'h0[reset]Fvt.forced_wide_u.stateFfANYFt31'h0Fgresethtop.TOP' 1 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_statepagev_fsm_state/$rootot.forced_wide_u.state::31'h0Fvt.forced_wide_u.stateFt31'h0htop.TOP' 1 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_statepagev_fsm_state/$rootot.forced_wide_u.state::31'h1Fvt.forced_wide_u.stateFt31'h1htop.TOP' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_statepagev_fsm_state/$rootot.forced_wide_u.state::31'h2Fvt.forced_wide_u.stateFt31'h2htop.TOP' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_arcpagev_fsm_arc/$rootot.forced_if_wide_u.state::31'h0->31'h1Fvt.forced_if_wide_u.stateFf31'h0Ft31'h1htop.TOP' 4 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_arcpagev_fsm_arc/$rootot.forced_if_wide_u.state::31'h1->31'h0Fvt.forced_if_wide_u.stateFf31'h1Ft31'h0htop.TOP' 3 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_statepagev_fsm_state/$rootot.forced_if_wide_u.state::31'h0Fvt.forced_if_wide_u.stateFt31'h0htop.TOP' 3 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_statepagev_fsm_state/$rootot.forced_if_wide_u.state::31'h1Fvt.forced_if_wide_u.stateFt31'h1htop.TOP' 4 diff --git a/test_regress/t/t_cover_fsm_transition_shapes_multi.v b/test_regress/t/t_cover_fsm_transition_shapes_multi.v index db15fdc7e..30315dfcb 100644 --- a/test_regress/t/t_cover_fsm_transition_shapes_multi.v +++ b/test_regress/t/t_cover_fsm_transition_shapes_multi.v @@ -694,7 +694,7 @@ module fsm_case_next_wrongrhs_bad ( end endmodule -module fsm_forced_wide_bad ( +module fsm_forced_wide ( input logic clk ); @@ -731,11 +731,11 @@ module fsm_forced_wide_bad ( endmodule -module fsm_forced_if_wide_bad ( +module fsm_forced_if_wide ( input logic clk ); - // Forced non-enum FSMs enumerate every possible state; 31 bits is too many. + // Forced non-enum FSMs infer only values observed in the transition logic logic [30:0] state /*verilator fsm_state*/; always_ff @(posedge clk) begin @@ -865,8 +865,8 @@ module t ( fsm_normalized_if_follow_wrongfrom_bad normalized_if_follow_wrongfrom_bad_u (.clk(clk)); fsm_normalized_if_follow_wronglhs_bad normalized_if_follow_wronglhs_bad_u (.clk(clk)); fsm_case_next_wrongrhs_bad case_next_wrongrhs_bad_u (.clk(clk)); - fsm_forced_wide_bad forced_wide_bad_u (.clk(clk)); - fsm_forced_if_wide_bad forced_if_wide_bad_u (.clk(clk)); + fsm_forced_wide forced_wide_u (.clk(clk)); + fsm_forced_if_wide forced_if_wide_u (.clk(clk)); fsm_reset_commit_mismatch_bad reset_commit_mismatch_bad_u (.clk(clk)); fsm_reset_then_bad reset_then_bad_u (.clk(clk)); diff --git a/test_regress/t/t_cover_fsm_wide_sparse.out b/test_regress/t/t_cover_fsm_wide_sparse.out new file mode 100644 index 000000000..62e8eb068 --- /dev/null +++ b/test_regress/t/t_cover_fsm_wide_sparse.out @@ -0,0 +1,148 @@ +// // verilator_coverage annotation + // DESCRIPTION: Verilator: FSM coverage supports wide sparse state values + // + // This file ONLY is placed under the Creative Commons Public Domain. + // SPDX-FileCopyrightText: 2026 Wilson Snyder + // SPDX-License-Identifier: CC0-1.0 + + module t ( + input logic clk + ); + + typedef enum logic [39:0] { + E_S0_IDLE = 40'h0000_0000_01, + E_S1_BUSY = 40'h8000_0000_02, + E_S2_DONE = 40'hffff_0000_03 + } enum_state_t; + + localparam logic [47:0] L_S0_IDLE = 48'h0000_0000_0001; + localparam logic [47:0] L_S1_BUSY = 48'h8000_0000_0002; + localparam logic [47:0] L_S2_DONE = 48'hffff_0000_0003; + + enum_state_t enum_state /*verilator fsm_reset_arc*/; + logic rst; + logic start; + integer cyc; + logic [39:0] forced_state /*verilator fsm_state*/; + logic [39:0] forced_case_if_state /*verilator fsm_state*/; + logic [47:0] param_state_q; + logic [47:0] param_state_d; + + initial begin + rst = 1'b1; + start = 1'b0; + cyc = 0; + end + + always @(posedge clk) begin + cyc <= cyc + 1; + if (cyc == 1) rst <= 1'b0; + if (cyc == 2) start <= 1'b1; + if (cyc == 3) start <= 1'b0; + if (cyc == 8) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + + always_ff @(posedge clk) begin + if (rst) begin + enum_state <= E_S0_IDLE; + end + else begin +%000005 case (enum_state) + // [FSM coverage] +%000001 // [fsm_arc t.enum_state::ANY->E_S0_IDLE[reset_include]] [reset arc, excluded from %] +%000005 // [fsm_arc t.enum_state::E_S0_IDLE->E_S0_IDLE] +%000001 // [fsm_arc t.enum_state::E_S0_IDLE->E_S1_BUSY] +%000001 // [fsm_arc t.enum_state::E_S1_BUSY->E_S2_DONE] +%000002 // [fsm_state t.enum_state::E_S0_IDLE] +%000001 // [fsm_state t.enum_state::E_S1_BUSY] +%000001 // [fsm_state t.enum_state::E_S2_DONE] + E_S0_IDLE: enum_state <= start ? E_S1_BUSY : E_S0_IDLE; + E_S1_BUSY: enum_state <= E_S2_DONE; + default: enum_state <= E_S0_IDLE; + endcase + end + end + + always_ff @(posedge clk) begin + if (rst) begin + forced_state <= 40'h0000_0000_01; + end + else begin +%000005 case (forced_state) + // [FSM coverage] +%000005 // [fsm_arc t.forced_state::40'h1->40'h1] +%000001 // [fsm_arc t.forced_state::40'h1->40'h8000000002] +%000001 // [fsm_arc t.forced_state::40'h8000000002->40'hffff000003] +%000001 // [fsm_arc t.forced_state::ANY->40'h1[reset]] [reset arc, excluded from %] +%000002 // [fsm_state t.forced_state::40'h1] +%000001 // [fsm_state t.forced_state::40'h8000000002] +%000001 // [fsm_state t.forced_state::40'hffff000003] + 40'h0000_0000_01: forced_state <= start ? 40'h8000_0000_02 : 40'h0000_0000_01; + 40'h8000_0000_02: forced_state <= 40'hffff_0000_03; + default: forced_state <= 40'h0000_0000_01; + endcase + end + end + + always_ff @(posedge clk) begin + if (rst) begin + forced_case_if_state <= 40'h0000_0000_01; + end + else begin +%000006 case (forced_case_if_state) + // [FSM coverage] +%000006 // [fsm_arc t.forced_case_if_state::40'h1->40'h1] +%000001 // [fsm_arc t.forced_case_if_state::40'h1->40'h8000000002] +%000001 // [fsm_arc t.forced_case_if_state::40'h8000000002->40'h1] +%000001 // [fsm_arc t.forced_case_if_state::ANY->40'h1[reset]] [reset arc, excluded from %] +%000002 // [fsm_state t.forced_case_if_state::40'h1] +%000001 // [fsm_state t.forced_case_if_state::40'h8000000002] + 40'h0000_0000_01: begin + if (start) begin + forced_case_if_state <= 40'h8000_0000_02; + end + else begin + forced_case_if_state <= 40'h0000_0000_01; + end + end + 40'h8000_0000_02: forced_case_if_state <= 40'h0000_0000_01; + default: forced_case_if_state <= 40'h0000_0000_01; + endcase + end + end + always_comb begin + param_state_d = param_state_q; +%000005 if (param_state_q == L_S0_IDLE) begin + // [FSM coverage] +%000001 // [fsm_arc t.param_state_q::ANY->L_S0_IDLE[reset]] [reset arc, excluded from %] +%000005 // [fsm_arc t.param_state_q::L_S0_IDLE->L_S0_IDLE] +%000001 // [fsm_arc t.param_state_q::L_S0_IDLE->L_S1_BUSY] +%000001 // [fsm_arc t.param_state_q::L_S1_BUSY->L_S2_DONE] +%000001 // [fsm_arc t.param_state_q::L_S2_DONE->L_S0_IDLE] +%000002 // [fsm_state t.param_state_q::L_S0_IDLE] +%000001 // [fsm_state t.param_state_q::L_S1_BUSY] +%000001 // [fsm_state t.param_state_q::L_S2_DONE] + param_state_d = start ? L_S1_BUSY : L_S0_IDLE; + end + else if (param_state_q == L_S1_BUSY) begin + param_state_d = L_S2_DONE; + end + else if (param_state_q == L_S2_DONE) begin + param_state_d = L_S0_IDLE; + end + end + + always_ff @(posedge clk) begin + if (rst) begin + param_state_q <= L_S0_IDLE; + end + else begin + param_state_q <= param_state_d; + end + end + + endmodule + diff --git a/test_regress/t/t_cover_fsm_wide_sparse.py b/test_regress/t/t_cover_fsm_wide_sparse.py new file mode 100755 index 000000000..8df28da15 --- /dev/null +++ b/test_regress/t/t_cover_fsm_wide_sparse.py @@ -0,0 +1,30 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: FSM wide sparse state encoding test +# +# 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 os + +import vltest_bootstrap + +test.scenarios('simulator') + +test.compile(verilator_flags2=['--cc --coverage-fsm']) + +test.execute() + +test.run(cmd=[ + os.environ["VERILATOR_ROOT"] + "/bin/verilator_coverage", + "--annotate", + test.obj_dir + "/annotated", + test.obj_dir + "/coverage.dat", +], + verilator_run=True) + +test.files_identical(test.obj_dir + "/annotated/" + test.name + ".v", test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_cover_fsm_wide_sparse.v b/test_regress/t/t_cover_fsm_wide_sparse.v new file mode 100644 index 000000000..8da44a43e --- /dev/null +++ b/test_regress/t/t_cover_fsm_wide_sparse.v @@ -0,0 +1,114 @@ +// DESCRIPTION: Verilator: FSM coverage supports wide sparse state values +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +module t ( + input logic clk +); + + typedef enum logic [39:0] { + E_S0_IDLE = 40'h0000_0000_01, + E_S1_BUSY = 40'h8000_0000_02, + E_S2_DONE = 40'hffff_0000_03 + } enum_state_t; + + localparam logic [47:0] L_S0_IDLE = 48'h0000_0000_0001; + localparam logic [47:0] L_S1_BUSY = 48'h8000_0000_0002; + localparam logic [47:0] L_S2_DONE = 48'hffff_0000_0003; + + enum_state_t enum_state /*verilator fsm_reset_arc*/; + logic rst; + logic start; + integer cyc; + logic [39:0] forced_state /*verilator fsm_state*/; + logic [39:0] forced_case_if_state /*verilator fsm_state*/; + logic [47:0] param_state_q; + logic [47:0] param_state_d; + + initial begin + rst = 1'b1; + start = 1'b0; + cyc = 0; + end + + always @(posedge clk) begin + cyc <= cyc + 1; + if (cyc == 1) rst <= 1'b0; + if (cyc == 2) start <= 1'b1; + if (cyc == 3) start <= 1'b0; + if (cyc == 8) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end + + always_ff @(posedge clk) begin + if (rst) begin + enum_state <= E_S0_IDLE; + end + else begin + case (enum_state) + E_S0_IDLE: enum_state <= start ? E_S1_BUSY : E_S0_IDLE; + E_S1_BUSY: enum_state <= E_S2_DONE; + default: enum_state <= E_S0_IDLE; + endcase + end + end + + always_ff @(posedge clk) begin + if (rst) begin + forced_state <= 40'h0000_0000_01; + end + else begin + case (forced_state) + 40'h0000_0000_01: forced_state <= start ? 40'h8000_0000_02 : 40'h0000_0000_01; + 40'h8000_0000_02: forced_state <= 40'hffff_0000_03; + default: forced_state <= 40'h0000_0000_01; + endcase + end + end + + always_ff @(posedge clk) begin + if (rst) begin + forced_case_if_state <= 40'h0000_0000_01; + end + else begin + case (forced_case_if_state) + 40'h0000_0000_01: begin + if (start) begin + forced_case_if_state <= 40'h8000_0000_02; + end + else begin + forced_case_if_state <= 40'h0000_0000_01; + end + end + 40'h8000_0000_02: forced_case_if_state <= 40'h0000_0000_01; + default: forced_case_if_state <= 40'h0000_0000_01; + endcase + end + end + always_comb begin + param_state_d = param_state_q; + if (param_state_q == L_S0_IDLE) begin + param_state_d = start ? L_S1_BUSY : L_S0_IDLE; + end + else if (param_state_q == L_S1_BUSY) begin + param_state_d = L_S2_DONE; + end + else if (param_state_q == L_S2_DONE) begin + param_state_d = L_S0_IDLE; + end + end + + always_ff @(posedge clk) begin + if (rst) begin + param_state_q <= L_S0_IDLE; + end + else begin + param_state_q <= param_state_d; + end + end + +endmodule