From 3c7727a8f45d7a3e6a9ad22f43f76d64095b05b3 Mon Sep 17 00:00:00 2001 From: Yogish Sekhar <160029258+ysekhar@users.noreply.github.com> Date: Fri, 10 Jul 2026 14:00:17 +0100 Subject: [PATCH] Fix spurious FSM COVERIGN on datapath comparisons (#7900) (#7908) Fixes #7900. --- src/V3FsmDetect.cpp | 16 +---- test_regress/t/t_cover_fsm_concat_unsup.out | 6 -- test_regress/t/t_cover_fsm_concat_unsup.v | 18 ------ ...t_unsup.py => t_cover_fsm_datapath_cmp.py} | 4 +- test_regress/t/t_cover_fsm_datapath_cmp.v | 62 +++++++++++++++++++ 5 files changed, 67 insertions(+), 39 deletions(-) delete mode 100644 test_regress/t/t_cover_fsm_concat_unsup.out delete mode 100644 test_regress/t/t_cover_fsm_concat_unsup.v rename test_regress/t/{t_cover_fsm_concat_unsup.py => t_cover_fsm_datapath_cmp.py} (69%) create mode 100644 test_regress/t/t_cover_fsm_datapath_cmp.v diff --git a/src/V3FsmDetect.cpp b/src/V3FsmDetect.cpp index 5ffee9e4c..d0f3049c3 100644 --- a/src/V3FsmDetect.cpp +++ b/src/V3FsmDetect.cpp @@ -988,21 +988,11 @@ class FsmDetectVisitor final : public VNVisitor { return assp; } - static AstVarRef* tryExtractVarRef(AstNodeExpr* const exprp) { - AstVarRef* const varp = VN_CAST(AstArraySel::baseFromp(exprp, true), VarRef); - if (!varp) { - exprp->v3warn(COVERIGN, - "Ignoring unsupported: FSM coverage with " << exprp->prettyTypeName()); - return nullptr; - } - return varp; - } - static AstNodeAssign* nodeStateVarAssign(AstNode* nodep, AstVarScope*& stateVscp, AstVarScope*& fromVscp) { AstNodeAssign* const assp = VN_CAST(nodep, NodeAssign); if (!assp) return nullptr; - AstVarRef* const lhsp = tryExtractVarRef(assp->lhsp()); + AstVarRef* const lhsp = VN_CAST(AstArraySel::baseFromp(assp->lhsp(), true), VarRef); AstVarRef* const rhsp = VN_CAST(assp->rhsp(), VarRef); if (!rhsp || !lhsp) return nullptr; stateVscp = lhsp->varScopep(); @@ -1016,7 +1006,7 @@ class FsmDetectVisitor final : public VNVisitor { FsmStateValue& resetValue) { AstNodeAssign* const assp = VN_CAST(nodep, NodeAssign); if (!assp) return nullptr; - AstVarRef* const lhsp = tryExtractVarRef(assp->lhsp()); + AstVarRef* const lhsp = VN_CAST(AstArraySel::baseFromp(assp->lhsp(), true), VarRef); AstCond* const rhsp = VN_CAST(assp->rhsp(), Cond); if (!rhsp || !lhsp) return nullptr; if (AstVarRef* const elsep = VN_CAST(rhsp->elsep(), VarRef)) { @@ -1228,7 +1218,7 @@ class FsmDetectVisitor final : public VNVisitor { AstVarRef* vrefp = VN_CAST(eqp->lhsp(), VarRef); AstNodeExpr* valuep = eqp->rhsp(); if (!vrefp) { - vrefp = tryExtractVarRef(eqp->rhsp()); + vrefp = VN_CAST(AstArraySel::baseFromp(eqp->rhsp(), true), VarRef); if (!vrefp) { return false; } valuep = eqp->lhsp(); } diff --git a/test_regress/t/t_cover_fsm_concat_unsup.out b/test_regress/t/t_cover_fsm_concat_unsup.out deleted file mode 100644 index 85da4d440..000000000 --- a/test_regress/t/t_cover_fsm_concat_unsup.out +++ /dev/null @@ -1,6 +0,0 @@ -%Warning-COVERIGN: t/t_cover_fsm_concat_unsup.v:12:17: Ignoring unsupported: FSM coverage with CONCAT - 12 | assign c = ({a, b} == 8'h00); - | ^ - ... 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_concat_unsup.v b/test_regress/t/t_cover_fsm_concat_unsup.v deleted file mode 100644 index a7d9a1f84..000000000 --- a/test_regress/t/t_cover_fsm_concat_unsup.v +++ /dev/null @@ -1,18 +0,0 @@ -// DESCRIPTION: Verilator: Verilog Test module -// -// This file ONLY is placed under the Creative Commons Public Domain. -// SPDX-FileCopyrightText: 2026 Antmicro -// SPDX-License-Identifier: CC0-1.0 - -module t ( - input logic [6:0] a, - input logic b, - output logic c -); - assign c = ({a, b} == 8'h00); - - initial begin - $write("*-* All Finished *-*\n"); - $finish; - end -endmodule diff --git a/test_regress/t/t_cover_fsm_concat_unsup.py b/test_regress/t/t_cover_fsm_datapath_cmp.py similarity index 69% rename from test_regress/t/t_cover_fsm_concat_unsup.py rename to test_regress/t/t_cover_fsm_datapath_cmp.py index 4a63bc600..f929ac752 100755 --- a/test_regress/t/t_cover_fsm_concat_unsup.py +++ b/test_regress/t/t_cover_fsm_datapath_cmp.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# DESCRIPTION: Verilator: FSM coverage concat as unsupported operation test +# DESCRIPTION: Verilator: FSM coverage ignores unrelated datapath comparisons # # 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 @@ -11,6 +11,6 @@ import vltest_bootstrap test.scenarios('vlt') -test.lint(fails=True, expect_filename=test.golden_filename, verilator_flags2=['--coverage']) +test.lint(verilator_flags2=['--coverage-fsm']) test.passes() diff --git a/test_regress/t/t_cover_fsm_datapath_cmp.v b/test_regress/t/t_cover_fsm_datapath_cmp.v new file mode 100644 index 000000000..1b6b02ffa --- /dev/null +++ b/test_regress/t/t_cover_fsm_datapath_cmp.v @@ -0,0 +1,62 @@ +// DESCRIPTION: Verilator: FSM coverage ignores unrelated datapath comparisons +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +module datapath_only #( + parameter logic [15:0] MASK = 16'hff00, + parameter logic [15:0] MATCH = 16'h1200 +) ( + input logic [6:0] a, + input logic b, + input logic [15:0] data, + output logic concat_hit, + output logic masked_hit +); + assign concat_hit = ({a, b} == 8'h00); + assign masked_hit = (data & MASK) == MATCH; +endmodule + +module t #( + parameter logic [15:0] MASK = 16'hff00, + parameter logic [15:0] MATCH = 16'h1200 +) ( + input logic clk, + input logic rst, + input logic go, + input logic [6:0] a, + input logic b, + input logic [15:0] data, + output logic busy, + output logic hit, + output logic concat_hit, + output logic masked_hit +); + typedef enum logic [1:0] { IDLE, RUN, DONE } state_t; + state_t state; + + always_ff @(posedge clk) begin + if (rst) begin + state <= IDLE; + end else begin + case (state) + IDLE: if (go) state <= RUN; + RUN: state <= DONE; + DONE: state <= IDLE; + default: state <= IDLE; + endcase + end + end + + assign busy = (state != IDLE); + assign hit = (data & MASK) == MATCH; + + datapath_only datapath_only_u ( + .a(a), + .b(b), + .data(data), + .concat_hit(concat_hit), + .masked_hit(masked_hit) + ); +endmodule