Skip to content

FIx coverage for generate scopes #1184

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
non-equal coverage databases.
- Fixed a debug assertion failure when initialising an array with more
than 2**32 elements (#1196).
- `generate` statements now create separate hierarchy in the code
coverage report

## Version 1.16.0 - 2025-04-21
- Added support for PSL `prev()`, `stable()`, `rose()` and `fell()`
Expand Down
41 changes: 41 additions & 0 deletions nvc.1
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,47 @@ to
.Bd -literal -offset indent
fold TB_TOP_LIB.TB_TOP.DUT.I_CPU_DATAPATH.I_INSTR_CACHE UNIT_TEST_LIB.CACHE_UNIT_TEST.DUT
.Ed
.Ss Code coverage limitations
When part of the design hierarchy is formed by an if-generate-else or
case-generate statement, the hierarchical path of implicit block statements
for each of the if-else branches / case choices is the same (unless
user uses alternative label for the branch or choice). Therefore, code
coverage items in such distinc blocks will have equal hierarchical path.
Consider following example:
.Bd -literal -offset indent
my_gen : case (op_kind) generate
when op_add => x <= a + b;
when op_sub => x <= a - b;
end generate my_gen;
.Ed
Both assign statements above have the following hierarchical path:
.Bd -literal -offset indent
<PLACE_IN_HIER>.MY_GEN._P0._SO
.Ed
When merging code coverage and generating code coverage report, NVC recognizes
these two coverage items are distinc despite their equal hierarchical path.
NVC distinguishes these two coverage items based on different location in the
source code. NVC does not merge such two items, and displays them separately
in the code coverage report (no aliasing will occur). However, since these two
coverage items have equal hierarchical paths, it is not possible to distinguish
them in the exclude file. Either both or none can be excluded.
Consider another example:
.Bd -literal -offset indent
my_gen : case (op_kind) generate
when op_add => my_inst : entity work.ent
generic map (op => op_add);
when op_sub => my_inst : entity work.ent
generic map (op => op_sub);
end generate my_gen;
.Ed
The two instances will have completely equal hierarchical paths. Since
they are both implemented by the same entity, coverage items in these two
instances will have equal hierarchical paths as well as locations
in the source code. Coverage data from these two instances will alias
and NVC will merge them together. To allow fine-grained control of exlcusions
or prevent aliasing in corner-cases, it is recommended to use alternative
labels for branches / choices of if-generage-else and case-generate
statements.
.Ss Additional Information
In coverage specification file and Exclude file
.Ql <ENTITY_NAME>
Expand Down
1 change: 1 addition & 0 deletions src/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ bool is_concurrent_block(tree_t t)
case T_BLOCK:
case T_IF_GENERATE:
case T_FOR_GENERATE:
case T_CASE_GENERATE:
return true;
default:
return false;
Expand Down
8 changes: 5 additions & 3 deletions src/cov/cov-data.c
Original file line number Diff line number Diff line change
Expand Up @@ -993,8 +993,7 @@ cover_scope_t *cover_create_instance(cover_data_t *data, cover_scope_t *parent,
parent->name = parent->hier = lib_name(lib_work());
}

// TODO: do not emit scopes for components
assert(tree_kind(unit) == T_ARCH);
assert(is_concurrent_block(unit));
assert(tree_kind(block) == T_BLOCK);

cover_scope_t *s = xcalloc(sizeof(cover_scope_t));
Expand Down Expand Up @@ -1152,7 +1151,10 @@ static void cover_merge_scope(cover_scope_t *old_s, cover_scope_t *new_s,
if (n_old_visits == old_s->items.count)
break;

if ((new->hier == old->hier) && (new->flags == old->flags)) {
if ((new->hier == old->hier) &&
(new->flags == old->flags) &&
(loc_eq(&(new->loc), &(old->loc)))) {

assert(new->kind == old->kind);
#ifdef COVER_DEBUG_MERGE
printf("Merging coverage item: %s\n", istr(old->hier));
Expand Down
12 changes: 9 additions & 3 deletions src/cov/cov-report.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,19 @@ static void cover_print_html_header(FILE *f)
" height: 2px;\n"
" background: black;\n"
" }\n"
" footer{\n"
" display: table;\n"
" text-align: center;\n"
" margin-left: auto;\n"
" margin-right: auto;\n"
" }\n"
" nav {\n"
" float: left;\n"
" background-color: #ccc;\n"
" width: " SIDEBAR_WIDTH ";\n"
" height: 100%%;\n"
" overflow: auto; \n"
" padding: 10px;\n"
" margin-top: 100px;\n"
" word-wrap: break-word;\n"
" }\n"
" table {\n"
" table-layout: fixed;"
Expand Down Expand Up @@ -1327,9 +1332,10 @@ static char *cover_get_report_name(const char *in)
SHA1_CTX ctx;
unsigned char buf[SHA1_LEN];
char *rv = xcalloc(2 * SHA1_LEN + 1);
char *in_cpy LOCAL = xstrdup(in);

SHA1Init(&ctx);
SHA1Update(&ctx, (const char unsigned*)in, strlen(in));
SHA1Update(&ctx, (const char unsigned*)in_cpy, strlen(in_cpy));
SHA1Final(buf, &ctx);

for (int i = 0; i < SHA1_LEN; i++)
Expand Down
2 changes: 1 addition & 1 deletion src/lower.c
Original file line number Diff line number Diff line change
Expand Up @@ -13045,7 +13045,7 @@ lower_unit_t *lower_instance(unit_registry_t *ur, lower_unit_t *parent,
lu->cscope = cover_create_instance(cover, parent->parent->cscope,
parent->container, unit);
}
else if (tree_kind(unit) == T_ARCH)
else if (is_concurrent_block(unit))
lu->cscope = cover_create_instance(cover, parent->cscope, block, unit);
else
lu->cscope = cover_create_scope(cover, parent->cscope, block, NULL);
Expand Down
2 changes: 1 addition & 1 deletion test/regress/cover26.vhd
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ begin

sub_inst_long_path : entity work.sub
generic map (
nest => 16
nest => 60
);

end architecture;
26 changes: 26 additions & 0 deletions test/regress/cover27.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
set -xe

nvc -a $TESTDIR/regress/cover27.vhd -e -gG_PAR=0 --cover=statement --cover-file=cover27_a.ncdb cover27 -r
nvc --cover-export --format=xml -o cover27_a.xml cover27_a.ncdb
sed -i -e "s/[^ ]*regress//g" cover27_a.xml

nvc -a $TESTDIR/regress/cover27.vhd -e -gG_PAR=1 --cover=statement --cover-file=cover27_b.ncdb cover27 -r
nvc --cover-export --format=xml -o cover27_b.xml cover27_b.ncdb
sed -i -e "s/[^ ]*regress//g" cover27_b.xml

nvc -a $TESTDIR/regress/cover27.vhd -e -gG_PAR=2 --cover=statement --cover-file=cover27_c.ncdb cover27 -r
nvc --cover-export --format=xml -o cover27_c.xml cover27_c.ncdb
sed -i -e "s/[^ ]*regress//g" cover27_c.xml

nvc -a $TESTDIR/regress/cover27.vhd -e -gG_PAR=3 --cover=statement --cover-file=cover27_d.ncdb cover27 -r
nvc --cover-export --format=xml -o cover27_d.xml cover27_d.ncdb
sed -i -e "s/[^ ]*regress//g" cover27_d.xml

nvc --cover-merge cover27*.ncdb --output=merged.covdb
nvc --cover-report --output=html merged.covdb 2>&1 | tee out.txt

diff -u $TESTDIR/regress/gold/cover27.txt out.txt
diff -u $TESTDIR/regress/gold/cover27_a.xml cover27_a.xml
diff -u $TESTDIR/regress/gold/cover27_b.xml cover27_b.xml
diff -u $TESTDIR/regress/gold/cover27_c.xml cover27_c.xml
diff -u $TESTDIR/regress/gold/cover27_d.xml cover27_d.xml
37 changes: 37 additions & 0 deletions test/regress/cover27.vhd
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
library ieee;
use ieee.std_logic_1164.all;

entity cover27 is
generic (
G_PAR : integer
);
end entity;

architecture test of cover27 is

signal a,b,c,d,e,f : std_logic;
signal vec : std_logic_vector(3 downto 0);

begin

G_IF_GEN : if (G_PAR = 0) generate
a <= '0';
elsif (G_PAR = 1) generate
b <= '0';
end generate;

T_FOR_GEN : for i in 0 to 3 generate
vec(i) <= a when (i = 0) else
b when (i = 1) else
c when (i = 2) else
d;
end generate;

G_CASE_GEN : case (G_PAR) generate
when 0 => c <= '1';
when G_CASE_1: 1 => d <= '0';
when G_CASE_2: 2 => e <= '1';
when 3 => f <= '0';
end generate;

end architecture;
9 changes: 9 additions & 0 deletions test/regress/gold/cover27.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
** Note: Code coverage report folder: html.
** Note: Code coverage report contains: covered, uncovered, excluded coverage details.
** Note: code coverage results for: WORK.COVER27
** Note: statement: 100.0 % (10/10)
** Note: branch: N.A.
** Note: toggle: N.A.
** Note: expression: N.A.
** Note: FSM state: N.A.
** Note: functional: N.A.
55 changes: 55 additions & 0 deletions test/regress/gold/cover27_a.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?xml version="1.0"?>
<scope name="WORK" /cover27.vhd">
<scope name="COVER27" block_name="COVER27-TEST" line="10">
<scope name="G_IF_GEN" block_name="G_IF_GEN" line="17">
<scope name="_P0" line="18">
<scope name="_S0">
<statement hier="WORK.COVER27.G_IF_GEN._P0._S0" data="1"/>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(0)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0">
<statement hier="WORK.COVER27.T_FOR_GEN(0)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(1)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="25">
<statement hier="WORK.COVER27.T_FOR_GEN(1)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(2)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="26">
<statement hier="WORK.COVER27.T_FOR_GEN(2)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(3)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="27">
<statement hier="WORK.COVER27.T_FOR_GEN(3)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="G_CASE_GEN" block_name="G_CASE_GEN" line="31">
<scope name="_P0">
<scope name="_S0">
<statement hier="WORK.COVER27.G_CASE_GEN._P0._S0" data="1"/>
</scope>
</scope>
</scope>
</scope>
</scope>
55 changes: 55 additions & 0 deletions test/regress/gold/cover27_b.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?xml version="1.0"?>
<scope name="WORK" /cover27.vhd">
<scope name="COVER27" block_name="COVER27-TEST" line="10">
<scope name="G_IF_GEN" block_name="G_IF_GEN" line="17">
<scope name="_P0" line="20">
<scope name="_S0">
<statement hier="WORK.COVER27.G_IF_GEN._P0._S0" data="1"/>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(0)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0">
<statement hier="WORK.COVER27.T_FOR_GEN(0)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(1)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="25">
<statement hier="WORK.COVER27.T_FOR_GEN(1)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(2)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="26">
<statement hier="WORK.COVER27.T_FOR_GEN(2)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(3)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="27">
<statement hier="WORK.COVER27.T_FOR_GEN(3)._P0._S0._S0" data="2"/>
</scope>
</scope>
</scope>
</scope>
<scope name="G_CASE_1" block_name="G_CASE_1" line="32">
<scope name="_P0">
<scope name="_S0">
<statement hier="WORK.COVER27.G_CASE_1._P0._S0" data="1"/>
</scope>
</scope>
</scope>
</scope>
</scope>
48 changes: 48 additions & 0 deletions test/regress/gold/cover27_c.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?xml version="1.0"?>
<scope name="WORK" /cover27.vhd">
<scope name="COVER27" block_name="COVER27-TEST" line="10">
<scope name="T_FOR_GEN(0)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0">
<statement hier="WORK.COVER27.T_FOR_GEN(0)._P0._S0._S0" data="1"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(1)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="25">
<statement hier="WORK.COVER27.T_FOR_GEN(1)._P0._S0._S0" data="1"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(2)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="26">
<statement hier="WORK.COVER27.T_FOR_GEN(2)._P0._S0._S0" data="1"/>
</scope>
</scope>
</scope>
</scope>
<scope name="T_FOR_GEN(3)" block_name="T_FOR_GEN" line="23">
<scope name="_P0" line="24">
<scope name="_S0">
<scope name="_S0" line="27">
<statement hier="WORK.COVER27.T_FOR_GEN(3)._P0._S0._S0" data="1"/>
</scope>
</scope>
</scope>
</scope>
<scope name="G_CASE_2" block_name="G_CASE_2" line="33">
<scope name="_P0">
<scope name="_S0">
<statement hier="WORK.COVER27.G_CASE_2._P0._S0" data="1"/>
</scope>
</scope>
</scope>
</scope>
</scope>
Loading
Loading