From 67ea819d82676358a6364b6e09173813eed7eff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Ob=C5=82onczek?= <142411778+koblonczek@users.noreply.github.com> Date: Sun, 14 Jul 2024 23:05:58 +0200 Subject: [PATCH] Fix toggle coverage aggregation on same line (#5248) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documentation states that minimum of all reported coverage of all signals in a line should be taken. Previous logic would break if there were any signals with zero coverage followed by signals with nonzero coverage - a minimum from those nonzero toggle count would be taken, disregarding zero coverage of previous signals. Internal-tag: [#62193] Signed-off-by: Krzysztof Obłonczek --- docs/CONTRIBUTORS | 1 + src/VlcSource.h | 13 +++++----- test_regress/t/t_cover_line.out | 4 ++-- test_regress/t/t_cover_toggle_min.out | 6 +++++ test_regress/t/t_cover_toggle_min.pl | 34 +++++++++++++++++++++++++++ test_regress/t/t_cover_toggle_min.v | 22 +++++++++++++++++ test_regress/t/t_vlcov_info.out | 2 +- 7 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 test_regress/t/t_cover_toggle_min.out create mode 100755 test_regress/t/t_cover_toggle_min.pl create mode 100644 test_regress/t/t_cover_toggle_min.v diff --git a/docs/CONTRIBUTORS b/docs/CONTRIBUTORS index d2d69afea..0b1072cb2 100644 --- a/docs/CONTRIBUTORS +++ b/docs/CONTRIBUTORS @@ -119,6 +119,7 @@ Kritik Bhimani Krzysztof Bieganski Krzysztof Boronski Krzysztof Boroński +Krzysztof Obłonczek Kuba Ober Larry Doolittle Liam Braun diff --git a/src/VlcSource.h b/src/VlcSource.h index 070d41219..3d6b55e9b 100644 --- a/src/VlcSource.h +++ b/src/VlcSource.h @@ -20,6 +20,7 @@ #include "config_build.h" #include "verilatedos.h" +#include #include #include #include @@ -36,7 +37,7 @@ class VlcSourceCount final { // MEMBERS const int m_lineno; ///< Line number - uint64_t m_count = 0; ///< Count + uint64_t m_count = std::numeric_limits::max(); ///< Count bool m_ok = false; ///< Coverage is above threshold PointsSet m_points; // Points on this line @@ -53,13 +54,11 @@ public: // METHODS void incCount(uint64_t count, bool ok) { - if (!m_count) { - m_count = count; + if (m_count == std::numeric_limits::max()) m_ok = ok; - } else { - m_count = std::min(m_count, count); - if (!ok) m_ok = false; - } + else + m_ok = m_ok && ok; + m_count = std::min(m_count, count); } void insertPoint(const VlcPoint* pointp) { m_points.emplace(pointp); } PointsSet& points() { return m_points; } diff --git a/test_regress/t/t_cover_line.out b/test_regress/t/t_cover_line.out index dfb4ee40e..4efcf9b6a 100644 --- a/test_regress/t/t_cover_line.out +++ b/test_regress/t/t_cover_line.out @@ -221,7 +221,7 @@ +000020 point: comment=block 000020 $write(""); // Always covered +000020 point: comment=block - 000020 if (0) begin // CHECK_COVER(0,"top.t.b*",0) +%000000 if (0) begin // CHECK_COVER(0,"top.t.b*",0) -000000 point: comment=if +000020 point: comment=else // Make sure that we don't optimize away zero buckets @@ -350,7 +350,7 @@ // because under coverage_module_off %000001 $write(""); -000001 point: comment=if -%000001 if (0) ; // CHECK_COVER(0,"top.t.o1",1) +%000000 if (0) ; // CHECK_COVER(0,"top.t.o1",1) -000000 point: comment=if -000001 point: comment=else end diff --git a/test_regress/t/t_cover_toggle_min.out b/test_regress/t/t_cover_toggle_min.out new file mode 100644 index 000000000..7608c5284 --- /dev/null +++ b/test_regress/t/t_cover_toggle_min.out @@ -0,0 +1,6 @@ +TN:verilator_coverage +SF:t/t_cover_toggle_min.v +DA:10,0 +DA:11,0 +DA:12,1 +end_of_record diff --git a/test_regress/t/t_cover_toggle_min.pl b/test_regress/t/t_cover_toggle_min.pl new file mode 100755 index 000000000..62ecb6a76 --- /dev/null +++ b/test_regress/t/t_cover_toggle_min.pl @@ -0,0 +1,34 @@ +#!/usr/bin/env perl +if (!$::Driver) { use FindBin; exec("$FindBin::Bin/bootstrap.pl", @ARGV, $0); die; } +# DESCRIPTION: Verilator: Verilog Test driver/expect definition +# +# Copyright 2024 by Wilson Snyder. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +scenarios(simulator => 1); + +compile( + verilator_flags2 => ['--binary', '--coverage-toggle'], + ); + +execute( + all_run_flags => [" +verilator+coverage+file+$Self->{obj_dir}/coverage.dat"], + check_finished => 1, + ); + +if (-e ("$Self->{obj_dir}/coverage.dat")) { # Don't try to write .info if test was skipped + run(cmd => ["$ENV{VERILATOR_ROOT}/bin/verilator_coverage", + "-write-info", "$Self->{obj_dir}/coverage.info", + "$Self->{obj_dir}/coverage.dat", + ], + verilator_run => 1, + ); + + files_identical("$Self->{obj_dir}/coverage.info", $Self->{golden_filename}); +} + +ok(1); +1; diff --git a/test_regress/t/t_cover_toggle_min.v b/test_regress/t/t_cover_toggle_min.v new file mode 100644 index 000000000..25c03b721 --- /dev/null +++ b/test_regress/t/t_cover_toggle_min.v @@ -0,0 +1,22 @@ +// DESCRIPTION: Verilator: Verilog Test module +// +// Copyright 2024 by Antmicro. 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-License-Identifier: LGPL-3.0-only OR Artistic-2.0 + +module t(); + logic[1:0] a; + logic[1:0] b; + logic[1:0] c; + + initial begin + #1 a = 2'b01; + #1 b = 2'b10; + #1 c = 2'b11; + #1 c = 2'b10; + $write("*-* All Finished *-*\n"); + $finish; + end +endmodule diff --git a/test_regress/t/t_vlcov_info.out b/test_regress/t/t_vlcov_info.out index bb304d511..f34103ee6 100644 --- a/test_regress/t/t_vlcov_info.out +++ b/test_regress/t/t_vlcov_info.out @@ -1,4 +1,4 @@ TN:verilator_coverage SF:file1.sp -DA:159,1 +DA:159,0 end_of_record