From dde6aa34cebb628986478c5a19cfec72c7f12b65 Mon Sep 17 00:00:00 2001 From: Yogish Sekhar <160029258+ysekhar@users.noreply.github.com> Date: Sat, 8 Aug 2026 18:44:46 +0000 Subject: [PATCH] Fix FSM coverage on empty reset branches (#8005) (#8064) --- src/V3FsmDetect.cpp | 14 +- test_regress/t/t_cover_fsm_empty_reset.out | 207 +++++++++++++++++++++ test_regress/t/t_cover_fsm_empty_reset.py | 32 ++++ test_regress/t/t_cover_fsm_empty_reset.sv | 192 +++++++++++++++++++ 4 files changed, 441 insertions(+), 4 deletions(-) create mode 100644 test_regress/t/t_cover_fsm_empty_reset.out create mode 100755 test_regress/t/t_cover_fsm_empty_reset.py create mode 100644 test_regress/t/t_cover_fsm_empty_reset.sv diff --git a/src/V3FsmDetect.cpp b/src/V3FsmDetect.cpp index 40a984f7d..526867913 100644 --- a/src/V3FsmDetect.cpp +++ b/src/V3FsmDetect.cpp @@ -701,7 +701,9 @@ class FsmDetectVisitor final : public VNVisitor { AstVarScope* resetStateVscp = nullptr; const ResetAssignStatus resetStatus = FsmDetectVisitor::collectConstStateAssigns( firstIfp->thensp(), resetStateVscp, reg.resetArcs()); - if (resetStatus == ResetAssignStatus::NONE || resetStateVscp != vscp) { + if (resetStatus == ResetAssignStatus::EMPTY) { + reg.resetArcs().clear(); + } else if (resetStatus == ResetAssignStatus::NONE || resetStateVscp != vscp) { reg.resetArcs().clear(); FsmStateValue resetValue; AstNode* const thenNodep @@ -1040,6 +1042,7 @@ class FsmDetectVisitor final : public VNVisitor { } enum class ResetAssignStatus : uint8_t { + EMPTY, // Reset branch had no non-coverage statements. NONE, // Reset branch was not the supported direct-constant shape. SINGLE, // Exactly one supported reset assignment was collected. MULTI_SAME_STATE // Multiple assignments to the same FSM state var; warn and ignore. @@ -1052,7 +1055,7 @@ class FsmDetectVisitor final : public VNVisitor { static ResetAssignStatus collectConstStateAssigns(AstNode* stmtp, AstVarScope*& stateVscp, std::vector& resetArcs) { AstNode* nodep = skipLeadingIgnorableStmt(stmtp); - UASSERT_OBJ(nodep, stmtp, "Empty reset branch unexpectedly survived to FSM detection"); + if (!nodep) return ResetAssignStatus::EMPTY; for (;; nodep = nodep->nextp()) { AstVarScope* assignStateVscp = nullptr; FsmStateValue value; @@ -1410,7 +1413,10 @@ class FsmDetectVisitor final : public VNVisitor { AstVarScope* resetStateVscp = nullptr; const ResetAssignStatus resetStatus = collectConstStateAssigns(ifp->thensp(), resetStateVscp, cand.resetArcs()); - if (resetStatus == ResetAssignStatus::NONE) { + const bool emptyResetBranch = resetStatus == ResetAssignStatus::EMPTY; + if (emptyResetBranch) { + cand.resetArcs().clear(); + } else if (resetStatus == ResetAssignStatus::NONE) { cand.resetArcs().clear(); FsmStateValue resetValue; AstNode* const thenNodep = singleMeaningfulBranch(ifp->thensp()); @@ -1425,7 +1431,7 @@ class FsmDetectVisitor final : public VNVisitor { AstNode* const elseNodep = singleMeaningfulBranch(ifp->elsesp()); UASSERT_OBJ(elseNodep, ifp, "register reset match requires a non-empty commit branch"); if (!nodeStateVarAssign(elseNodep, stateVscp, nextVscp)) return false; - if (resetStateVscp != stateVscp) return false; + if (!emptyResetBranch && resetStateVscp != stateVscp) return false; cand.resetCond() = describeResetCond(ifp->condp()); cand.hasResetCond(cand.resetCond().varScopep != nullptr); } else { diff --git a/test_regress/t/t_cover_fsm_empty_reset.out b/test_regress/t/t_cover_fsm_empty_reset.out new file mode 100644 index 000000000..327a25bde --- /dev/null +++ b/test_regress/t/t_cover_fsm_empty_reset.out @@ -0,0 +1,207 @@ +// // verilator_coverage annotation + // DESCRIPTION: Verilator: FSM coverage ignores empty reset branches + // + // This file ONLY is placed under the Creative Commons Public Domain. + // SPDX-FileCopyrightText: 2026 Wilson Snyder + // SPDX-License-Identifier: CC0-1.0 + + module two_proc_empty_reset ( + input logic clk, + input logic rst, + input logic go + ); + typedef enum logic { + IDLE, + BUSY + } state_t; + +%000001 state_t state = IDLE; + state_t state_next; + +%000008 always_ff @(posedge clk) begin +%000006 if (rst) begin + end +%000006 else state <= state_next; + end + +%000009 always_comb begin +%000009 state_next = state; +%000009 case (state) + // [FSM coverage] +%000001 // [fsm_arc t.two_proc_u.state::IDLE->BUSY] +%000006 // [fsm_arc t.two_proc_u.state::IDLE->IDLE] +%000001 // [fsm_state t.two_proc_u.state::BUSY] +%000001 // [fsm_state t.two_proc_u.state::IDLE] +%000008 IDLE: state_next = go ? BUSY : IDLE; +%000001 default: state_next = IDLE; + endcase + end + endmodule + + module oneblock_empty_reset ( + input logic clk, + input logic rst, + input logic take_alt + ); + typedef enum logic [1:0] { + WAITING, + COLLECTING, + SENDING + } state_t; + +%000001 state_t state = WAITING; + +%000008 always_ff @(posedge clk) begin +%000006 if (rst) begin + end + else +%000006 case (state) + // [FSM coverage] +%000001 // [fsm_arc t.oneblock_u.state::COLLECTING->SENDING] +%000002 // [fsm_arc t.oneblock_u.state::SENDING->WAITING] +%000001 // [fsm_arc t.oneblock_u.state::WAITING->COLLECTING] +%000002 // [fsm_arc t.oneblock_u.state::WAITING->SENDING] +%000001 // [fsm_state t.oneblock_u.state::COLLECTING] +%000003 // [fsm_state t.oneblock_u.state::SENDING] +%000002 // [fsm_state t.oneblock_u.state::WAITING] +%000003 WAITING: state <= take_alt ? SENDING : COLLECTING; +%000001 COLLECTING: state <= SENDING; +%000002 SENDING: state <= WAITING; +%000000 default: state <= WAITING; + endcase + end + endmodule + + module inline_empty_reset ( + input logic clk, + input logic rst, + input logic input_valid, + input logic input_last, + input logic output_ready, + input logic [6:0] input_data, + output logic [6:0] output_data, + output logic output_valid + ); + typedef enum logic [1:0] { + WAITING, + COLLECTING, + SENDING + } state_t; + +%000001 state_t state = WAITING; +%000001 logic [6:0] saved_data = '0; + +%000008 always_ff @(posedge clk) begin +%000002 if (rst) begin + end +%000004 else if (state == WAITING) begin +%000003 if (input_valid) begin +%000001 saved_data <= input_data; +%000001 state <= input_last ? SENDING : COLLECTING; + end +%000003 else begin +%000003 state <= WAITING; + end + end +%000001 else if (state == COLLECTING) begin +%000001 if (input_valid && input_last) begin +%000001 state <= SENDING; + end +%000000 else begin +%000000 state <= COLLECTING; + end + end +%000001 else if (state == SENDING) begin +%000001 if (output_ready) begin +%000001 state <= WAITING; + end +%000000 else begin +%000000 state <= SENDING; + end + end + end + +%000001 always_comb begin +%000001 output_valid = state == SENDING; +%000001 output_data = saved_data; + end + endmodule + + module t ( + input logic clk + ); + logic rst; + logic go; + logic take_alt; + logic input_valid; + logic input_last; + logic output_ready; + logic [6:0] input_data; + logic [6:0] output_data; + logic output_valid; + int cyc; + + two_proc_empty_reset two_proc_u ( + .clk(clk), + .rst(rst), + .go(go) + ); + + oneblock_empty_reset oneblock_u ( + .clk(clk), + .rst(rst), + .take_alt(take_alt) + ); + + inline_empty_reset inline_u ( + .clk(clk), + .rst(rst), + .input_valid(input_valid), + .input_last(input_last), + .output_ready(output_ready), + .input_data(input_data), + .output_data(output_data), + .output_valid(output_valid) + ); + +%000001 initial begin +%000001 rst = 1'b1; +%000001 go = 1'b0; +%000001 take_alt = 1'b0; +%000001 input_valid = 1'b0; +%000001 input_last = 1'b0; +%000001 output_ready = 1'b0; +%000001 input_data = 7'h12; +%000001 cyc = 0; + end + +%000008 always @(posedge clk) begin +%000008 cyc <= cyc + 1; +%000007 if (cyc == 1) begin +%000001 rst <= 1'b0; +%000001 go <= 1'b1; +%000001 take_alt <= 1'b0; +%000001 input_valid <= 1'b1; +%000001 input_last <= 1'b0; + end +%000007 if (cyc == 2) begin +%000001 go <= 1'b0; +%000001 input_valid <= 1'b1; +%000001 input_last <= 1'b1; +%000001 input_data <= 7'h35; + end +%000007 if (cyc == 3) begin +%000001 input_valid <= 1'b0; +%000001 output_ready <= 1'b1; + end +%000007 if (cyc == 4) begin +%000001 output_ready <= 1'b0; +%000001 take_alt <= 1'b1; + end +%000007 if (cyc == 7) begin +%000001 $write("*-* All Finished *-*\n"); +%000001 $finish; + end + end + endmodule + diff --git a/test_regress/t/t_cover_fsm_empty_reset.py b/test_regress/t/t_cover_fsm_empty_reset.py new file mode 100755 index 000000000..ed8f5fd29 --- /dev/null +++ b/test_regress/t/t_cover_fsm_empty_reset.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 +# DESCRIPTION: Verilator: FSM coverage ignores empty reset branches +# +# 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.top_filename = "t/t_cover_fsm_empty_reset.sv" + +test.compile(verilator_flags2=["--cc --coverage-line --coverage-fsm"]) + +test.execute() + +test.run(cmd=[ + os.environ["VERILATOR_ROOT"] + "/bin/verilator_coverage", + "--include-reset-arcs", + "--annotate", + test.obj_dir + "/annotated", + test.obj_dir + "/coverage.dat", +], + verilator_run=True) + +test.files_identical(test.obj_dir + "/annotated/" + test.name + ".sv", test.golden_filename) + +test.passes() diff --git a/test_regress/t/t_cover_fsm_empty_reset.sv b/test_regress/t/t_cover_fsm_empty_reset.sv new file mode 100644 index 000000000..8ad762c58 --- /dev/null +++ b/test_regress/t/t_cover_fsm_empty_reset.sv @@ -0,0 +1,192 @@ +// DESCRIPTION: Verilator: FSM coverage ignores empty reset branches +// +// This file ONLY is placed under the Creative Commons Public Domain. +// SPDX-FileCopyrightText: 2026 Wilson Snyder +// SPDX-License-Identifier: CC0-1.0 + +module two_proc_empty_reset ( + input logic clk, + input logic rst, + input logic go +); + typedef enum logic { + IDLE, + BUSY + } state_t; + + state_t state = IDLE; + state_t state_next; + + always_ff @(posedge clk) begin + if (rst) begin + end + else state <= state_next; + end + + always_comb begin + state_next = state; + case (state) + IDLE: state_next = go ? BUSY : IDLE; + default: state_next = IDLE; + endcase + end +endmodule + +module oneblock_empty_reset ( + input logic clk, + input logic rst, + input logic take_alt +); + typedef enum logic [1:0] { + WAITING, + COLLECTING, + SENDING + } state_t; + + state_t state = WAITING; + + always_ff @(posedge clk) begin + if (rst) begin + end + else + case (state) + WAITING: state <= take_alt ? SENDING : COLLECTING; + COLLECTING: state <= SENDING; + SENDING: state <= WAITING; + default: state <= WAITING; + endcase + end +endmodule + +module inline_empty_reset ( + input logic clk, + input logic rst, + input logic input_valid, + input logic input_last, + input logic output_ready, + input logic [6:0] input_data, + output logic [6:0] output_data, + output logic output_valid +); + typedef enum logic [1:0] { + WAITING, + COLLECTING, + SENDING + } state_t; + + state_t state = WAITING; + logic [6:0] saved_data = '0; + + always_ff @(posedge clk) begin + if (rst) begin + end + else if (state == WAITING) begin + if (input_valid) begin + saved_data <= input_data; + state <= input_last ? SENDING : COLLECTING; + end + else begin + state <= WAITING; + end + end + else if (state == COLLECTING) begin + if (input_valid && input_last) begin + state <= SENDING; + end + else begin + state <= COLLECTING; + end + end + else if (state == SENDING) begin + if (output_ready) begin + state <= WAITING; + end + else begin + state <= SENDING; + end + end + end + + always_comb begin + output_valid = state == SENDING; + output_data = saved_data; + end +endmodule + +module t ( + input logic clk +); + logic rst; + logic go; + logic take_alt; + logic input_valid; + logic input_last; + logic output_ready; + logic [6:0] input_data; + logic [6:0] output_data; + logic output_valid; + int cyc; + + two_proc_empty_reset two_proc_u ( + .clk(clk), + .rst(rst), + .go(go) + ); + + oneblock_empty_reset oneblock_u ( + .clk(clk), + .rst(rst), + .take_alt(take_alt) + ); + + inline_empty_reset inline_u ( + .clk(clk), + .rst(rst), + .input_valid(input_valid), + .input_last(input_last), + .output_ready(output_ready), + .input_data(input_data), + .output_data(output_data), + .output_valid(output_valid) + ); + + initial begin + rst = 1'b1; + go = 1'b0; + take_alt = 1'b0; + input_valid = 1'b0; + input_last = 1'b0; + output_ready = 1'b0; + input_data = 7'h12; + cyc = 0; + end + + always @(posedge clk) begin + cyc <= cyc + 1; + if (cyc == 1) begin + rst <= 1'b0; + go <= 1'b1; + take_alt <= 1'b0; + input_valid <= 1'b1; + input_last <= 1'b0; + end + if (cyc == 2) begin + go <= 1'b0; + input_valid <= 1'b1; + input_last <= 1'b1; + input_data <= 7'h35; + end + if (cyc == 3) begin + input_valid <= 1'b0; + output_ready <= 1'b1; + end + if (cyc == 4) begin + output_ready <= 1'b0; + take_alt <= 1'b1; + end + if (cyc == 7) begin + $write("*-* All Finished *-*\n"); + $finish; + end + end +endmodule