From c545552cc138f6717237b4dab09d5e6d4a489823 Mon Sep 17 00:00:00 2001 From: Geza Lore Date: Sun, 20 Sep 2026 17:21:40 +0200 Subject: [PATCH] Fix FSM coverage hierarchy being emitted as an absolute path (#8421) AstNodeCoverDecl::hier() is relative to the scope the declaration is emitted from, as V3EmitCImp builds the reported path as 'vlNamep + hierp', and that is how V3Coverage uses it. V3FsmDetect instead set it to the absolute scope name, so the instance path was counted twice. This was masked whenever the owning module was inlined, as the declaration then ended up in the top scope. With inlining disabled, an FSM in an instance reported 'top.t.forced_wide_u.t.forced_wide_u'. In the inlined case it reported 'top.TOP', leaking the internal top wrapper name into user visible coverage output, rather than plain 'top'. --- src/V3FsmDetect.cpp | 2 -- ...t_cover_fsm_plain_always_zerohit_multi.out | 16 +++++----- .../t/t_cover_fsm_transition_shapes_multi.out | 20 ++++++------ ..._cover_fsm_transition_shapes_no_inline.out | 11 +++++++ ...t_cover_fsm_transition_shapes_no_inline.py | 32 +++++++++++++++++++ 5 files changed, 61 insertions(+), 20 deletions(-) create mode 100644 test_regress/t/t_cover_fsm_transition_shapes_no_inline.out create mode 100755 test_regress/t/t_cover_fsm_transition_shapes_no_inline.py diff --git a/src/V3FsmDetect.cpp b/src/V3FsmDetect.cpp index 90e990e9f..ada820d4e 100644 --- a/src/V3FsmDetect.cpp +++ b/src/V3FsmDetect.cpp @@ -2194,7 +2194,6 @@ class FsmLowerVisitor final { graph.stateVarName(), "", statep->label()}; - declp->hier(scopep->prettyName()); modp->addStmtsp(declp); AstNodeExpr* const guardp = andExpr(flp, @@ -2231,7 +2230,6 @@ class FsmLowerVisitor final { fromVertexp->label(), toStatep->label(), fsmTag}; - declp->hier(scopep->prettyName()); modp->addStmtsp(declp); AstNodeExpr* guardp = nullptr; if (fromVertexp->isResetAny()) { diff --git a/test_regress/t/t_cover_fsm_plain_always_zerohit_multi.out b/test_regress/t/t_cover_fsm_plain_always_zerohit_multi.out index 05101a413..363177270 100644 --- a/test_regress/t/t_cover_fsm_plain_always_zerohit_multi.out +++ b/test_regress/t/t_cover_fsm_plain_always_zerohit_multi.out @@ -1,9 +1,9 @@ # SystemC::Coverage-3 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_arcpagev_fsm_arc/$rootot.near_canonical_state_d_case_u.state_d::S0->S1Fvt.near_canonical_state_d_case_u.state_dFfS0FtS1htop.TOP' 0 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_arcpagev_fsm_arc/$rootot.near_canonical_state_d_case_u.state_d::S0->S2Fvt.near_canonical_state_d_case_u.state_dFfS0FtS2htop.TOP' 0 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_statepagev_fsm_state/$rootot.near_canonical_state_d_case_u.state_d::S0Fvt.near_canonical_state_d_case_u.state_dFtS0htop.TOP' 0 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_statepagev_fsm_state/$rootot.near_canonical_state_d_case_u.state_d::S1Fvt.near_canonical_state_d_case_u.state_dFtS1htop.TOP' 0 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_statepagev_fsm_state/$rootot.near_canonical_state_d_case_u.state_d::S2Fvt.near_canonical_state_d_case_u.state_dFtS2htop.TOP' 0 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl60n5tfsm_statepagev_fsm_state/$rootot.selector_matches_noassign_u.state_q::S0Fvt.selector_matches_noassign_u.state_qFtS0htop.TOP' 0 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl60n5tfsm_statepagev_fsm_state/$rootot.selector_matches_noassign_u.state_q::S1Fvt.selector_matches_noassign_u.state_qFtS1htop.TOP' 0 -C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl60n5tfsm_statepagev_fsm_state/$rootot.selector_matches_noassign_u.state_q::S2Fvt.selector_matches_noassign_u.state_qFtS2htop.TOP' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_arcpagev_fsm_arc/$rootot.near_canonical_state_d_case_u.state_d::S0->S1Fvt.near_canonical_state_d_case_u.state_dFfS0FtS1htop' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_arcpagev_fsm_arc/$rootot.near_canonical_state_d_case_u.state_d::S0->S2Fvt.near_canonical_state_d_case_u.state_dFfS0FtS2htop' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_statepagev_fsm_state/$rootot.near_canonical_state_d_case_u.state_d::S0Fvt.near_canonical_state_d_case_u.state_dFtS0htop' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_statepagev_fsm_state/$rootot.near_canonical_state_d_case_u.state_d::S1Fvt.near_canonical_state_d_case_u.state_dFtS1htop' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl28n5tfsm_statepagev_fsm_state/$rootot.near_canonical_state_d_case_u.state_d::S2Fvt.near_canonical_state_d_case_u.state_dFtS2htop' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl60n5tfsm_statepagev_fsm_state/$rootot.selector_matches_noassign_u.state_q::S0Fvt.selector_matches_noassign_u.state_qFtS0htop' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl60n5tfsm_statepagev_fsm_state/$rootot.selector_matches_noassign_u.state_q::S1Fvt.selector_matches_noassign_u.state_qFtS1htop' 0 +C 'ft/t_cover_fsm_plain_always_zerohit_multi.vl60n5tfsm_statepagev_fsm_state/$rootot.selector_matches_noassign_u.state_q::S2Fvt.selector_matches_noassign_u.state_qFtS2htop' 0 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 1bb0807e2..2fbc10baa 100644 --- a/test_regress/t/t_cover_fsm_transition_shapes_multi.out +++ b/test_regress/t/t_cover_fsm_transition_shapes_multi.out @@ -1,11 +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 +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' 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' 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' 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' 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' 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' 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' 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' 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' 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' 4 diff --git a/test_regress/t/t_cover_fsm_transition_shapes_no_inline.out b/test_regress/t/t_cover_fsm_transition_shapes_no_inline.out new file mode 100644 index 000000000..34fa47983 --- /dev/null +++ b/test_regress/t/t_cover_fsm_transition_shapes_no_inline.out @@ -0,0 +1,11 @@ +# SystemC::Coverage-3 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_arcpagev_fsm_arc/fsm_forced_wideot.forced_wide_u.state::31'h0->31'h1Fvt.forced_wide_u.stateFf31'h0Ft31'h1htop.t.forced_wide_u' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_arcpagev_fsm_arc/fsm_forced_wideot.forced_wide_u.state::31'h1->31'h2Fvt.forced_wide_u.stateFf31'h1Ft31'h2htop.t.forced_wide_u' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_arcpagev_fsm_arc/fsm_forced_wideot.forced_wide_u.state::ANY->31'h0[reset]Fvt.forced_wide_u.stateFfANYFt31'h0Fgresethtop.t.forced_wide_u' 1 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_statepagev_fsm_state/fsm_forced_wideot.forced_wide_u.state::31'h0Fvt.forced_wide_u.stateFt31'h0htop.t.forced_wide_u' 1 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_statepagev_fsm_state/fsm_forced_wideot.forced_wide_u.state::31'h1Fvt.forced_wide_u.stateFt31'h1htop.t.forced_wide_u' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl724n7tfsm_statepagev_fsm_state/fsm_forced_wideot.forced_wide_u.state::31'h2Fvt.forced_wide_u.stateFt31'h2htop.t.forced_wide_u' 2 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_arcpagev_fsm_arc/fsm_forced_if_wideot.forced_if_wide_u.state::31'h0->31'h1Fvt.forced_if_wide_u.stateFf31'h0Ft31'h1htop.t.forced_if_wide_u' 4 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_arcpagev_fsm_arc/fsm_forced_if_wideot.forced_if_wide_u.state::31'h1->31'h0Fvt.forced_if_wide_u.stateFf31'h1Ft31'h0htop.t.forced_if_wide_u' 3 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_statepagev_fsm_state/fsm_forced_if_wideot.forced_if_wide_u.state::31'h0Fvt.forced_if_wide_u.stateFt31'h0htop.t.forced_if_wide_u' 3 +C 'ft/t_cover_fsm_transition_shapes_multi.vl742n5tfsm_statepagev_fsm_state/fsm_forced_if_wideot.forced_if_wide_u.state::31'h1Fvt.forced_if_wide_u.stateFt31'h1htop.t.forced_if_wide_u' 4 diff --git a/test_regress/t/t_cover_fsm_transition_shapes_no_inline.py b/test_regress/t/t_cover_fsm_transition_shapes_no_inline.py new file mode 100755 index 000000000..076f1c67e --- /dev/null +++ b/test_regress/t/t_cover_fsm_transition_shapes_no_inline.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: FSM coverage hierarchy with module inlining disabled +# +# 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 + +# Same as t_cover_fsm_transition_shapes_multi, but without module inlining, so +# the coverage declarations stay in the scope of the instance that owns them. +# The emitted hierarchy is the scope name plus AstNodeCoverDecl::hier(), so hier +# must be relative to that scope. +# +# Note this golden still differs from the inlined one, which reports the top +# scope and '$root', because V3FsmDetect::detect runs after V3Inline and so only +# ever sees the flattened design. Were it to run before, V3Inline would prefix +# the instance names onto hier and both would report the same, at which point +# this test should compare against t_cover_fsm_transition_shapes_multi.out. + +import vltest_bootstrap + +test.scenarios('simulator') +test.top_filename = "t/t_cover_fsm_transition_shapes_multi.v" + +test.compile(verilator_flags2=['--cc --coverage-fsm', '-fno-inline']) + +test.execute() + +test.files_identical(test.obj_dir + "/coverage.dat", "t/" + test.name + ".out") + +test.passes()