Script line coverage reports for core/pipeline

Signed-off-by: Daniel Bates <Daniel.Bates@cl.cam.ac.uk>

Author: db434

.github/workflows/ci.yml

@@ -47,7 +47,7 @@ jobs:
           curl -Ls https://download.opensuse.org/repositories/home:phiwag:edatools/xUbuntu_20.04/Release.key | sudo apt-key add -
           sudo sh -c "echo 'deb http://download.opensuse.org/repositories/home:/phiwag:/edatools/xUbuntu_20.04/ /' > /etc/apt/sources.list.d/edatools.list"
           sudo apt-get update
-          sudo apt-get install verilator-4.040
+          sudo apt-get install verilator-4.200
 
       - name: Install Python dependencies
         run: pip3 install setuptools wheel

flows/muntjac_core_tb.core

@@ -115,6 +115,8 @@ targets:
           - "-o muntjac_core"
           - "--trace --trace-structs"
           - "--assert"
+          - "--coverage-user --coverage-line"
+          # coverage-toggle is also available, but is very slow for little gain
           - "-O3"  # Verilator optimisation
           - "-CFLAGS -O3"  # compiler optimisation
           - "-CFLAGS -DFST_ENABLE" # Either VCD_ENABLE or FST_ENABLE

flows/muntjac_pipeline_tb.core

@@ -109,6 +109,8 @@ targets:
           - "-o muntjac_pipeline"
           - "--trace --trace-structs"
           - "--assert"
+          - "--coverage-user --coverage-line"
+          # coverage-toggle is also available, but is very slow for little gain
           - "-O3"  # Verilator optimisation
           - "-CFLAGS -O3"  # compiler optimisation
           - "-CFLAGS -DFST_ENABLE" # Either VCD_ENABLE or FST_ENABLE

flows/verilator/src/simulation.h

@@ -36,9 +36,11 @@ class Simulation {
   Simulation(string name) {
     this->name = name;
     timeout = 1000000;
+    coverage_on = false;
     cycle = 0.0;
 
     args.add_argument("--timeout", "Force end of simulation after fixed number of cycles", ArgumentParser::ARGS_ONE);
+    args.add_argument("--coverage", "Dump coverage information to a file", ArgumentParser::ARGS_ONE);
     args.add_argument("-v", "Display basic logging information as simulation proceeds");
     args.add_argument("-vv", "Display detailed logging information as simulation proceeds");
     args.add_argument("--help", "Display this information and exit");
@@ -106,6 +108,8 @@ class Simulation {
       fst_trace.close();
     }
 #endif
+    if (coverage_on)
+      Verilated::threadContextp()->coveragep()->write(coverage_file.c_str());
   }
 
 public:
@@ -137,6 +141,11 @@ class Simulation {
 
     if (args.found_arg("--timeout"))
       timeout = std::stoi(args.get_arg("--timeout"));
+
+    if (this->args.found_arg("--coverage")) {
+      coverage_on = true;
+      coverage_file = this->args.get_arg("--coverage");
+    }
 
 #ifdef FST_ENABLE
     if (args.found_arg("--fst")) {
@@ -196,6 +205,10 @@ class Simulation {
   VerilatedFstC fst_trace;
 #endif
 
+  // Generate coverage report?
+  bool coverage_on;
+  string coverage_file;
+
 };
 
 

test/coverage/Makefile

@@ -0,0 +1,69 @@
+# Copyright lowRISC contributors.
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+
+MUNTJAC_ROOT  ?= ../..
+
+SIM           ?= $(MUNTJAC_ROOT)/bin/muntjac_core
+BUILD_DIR     ?= $(MUNTJAC_ROOT)/build/lowrisc_muntjac_core_tb_0.1/sim-verilator
+
+COVERAGE       = $(addsuffix .cov, $(TESTS))
+COV_TOTAL      = total.cov
+
+# Source files we are interested in the coverage of.
+# Ignore axi (unused) and tilelink (tested separately).
+PIPELINE_SRC   = $(notdir $(wildcard $(MUNTJAC_ROOT)/ip/pipeline/rtl/*.sv))
+CORE_SRC       = $(notdir $(wildcard $(MUNTJAC_ROOT)/ip/core/rtl/*.sv))
+FPU_SRC        = $(notdir $(wildcard $(MUNTJAC_ROOT)/ip/fpu/rtl/*.sv))
+ALL_SRC        = $(PIPELINE_SRC) $(CORE_SRC) $(FPU_SRC)
+
+ANNOTATION_DIR = coverage
+ANNOTATED_SRC  = $(addprefix $(ANNOTATION_DIR)/, $(ALL_SRC))
+
+.PHONY: pipeline_coverage core_coverage fpu_coverage coverage all
+all: coverage
+coverage: pipeline_coverage core_coverage fpu_coverage
+
+# Simulate one test to generate one coverage report.
+# The name of the test is the name of the coverage report without ".cov".
+# Ignore errors.
+$(COVERAGE):
+	-$(SIM) --coverage $@ $(basename $@)
+
+# Merge coverage reports from all simulations.
+$(COV_TOTAL): $(COVERAGE)
+ifndef TESTS
+	$(error Please set the TESTS variable to a list of binaries to execute)
+endif
+	verilator_coverage -write $@ $^
+
+# Annotate each line of source code with how many times it was reached.
+# verilator_coverage claims to have a command line option to tell it where the
+# Verilog source is, but I haven't found it, so this needs to run in the
+# directory where the simulator was originally built.
+# (Using PHONY annotate because we don't know that all files will be annotated.)
+.PHONY: annotate
+annotate: $(COV_TOTAL)
+	cd $(BUILD_DIR) && \
+	verilator_coverage $(CURDIR)/$< --annotate $(CURDIR)/$(ANNOTATION_DIR) \
+	--annotate-all --annotate-min 1
+
+pipeline_coverage: annotate
+	@echo -n "ip/pipeline line coverage: "
+	@python3 $(MUNTJAC_ROOT)/test/coverage/coverage_filter.py --annotation-dir \
+	$(ANNOTATION_DIR) --files $(PIPELINE_SRC)
+
+core_coverage: annotate
+	@echo -n "ip/core line coverage:     "
+	@python3 $(MUNTJAC_ROOT)/test/coverage/coverage_filter.py --annotation-dir \
+	$(ANNOTATION_DIR) --files $(CORE_SRC)
+
+fpu_coverage: annotate
+	@echo -n "ip/fpu line coverage:      "
+	@python3 $(MUNTJAC_ROOT)/test/coverage/coverage_filter.py --annotation-dir \
+	$(ANNOTATION_DIR) --files $(FPU_SRC)
+
+.PHONY: clean
+clean:
+	rm -f $(COVERAGE) $(COV_TOTAL)
+	rm -rf $(ANNOTATION_DIR)

test/coverage/README.md

@@ -0,0 +1,23 @@
+# Coverage
+
+Coverage is a useful way of tracking how well a design has been tested. It works by defining a set of *coverpoints*, and counting how many of them are reached during testing.
+
+Verilator provides us with *line coverage*, where each significant line of SystemVerilog is tracked. We can also define our own *functional coverage* by specifying particular states which we would like to see. See the [TileLink testing](../tilelink) for an example of functional coverage.
+
+Neither of these approaches is perfect: it is possible to execute every line of code while producing an incorrect result, and functional coverage is only as useful as the set of coverpoints we define.
+
+To generate a coverage report for the Muntjac core across a range of test binaries, use:
+
+```
+make coverage TESTS="test1 test2 test3"
+```
+
+This will produce a report which looks like:
+
+```
+ip/pipeline line coverage: 2099/2250 (93%)
+ip/core line coverage:     2568/2861 (90%)
+ip/fpu line coverage:      483/504 (96%)
+```
+
+You can see which coverpoints were reached by inspecting the annotated source files in the `coverage` directory. Each line containing a coverpoint will be prefixed with a number indicating how many times that line was reached during testing.

test/coverage/coverage_filter.py

@@ -0,0 +1,55 @@
+# Copyright lowRISC contributors.
+# Licensed under the Apache License, Version 2.0, see LICENSE for details.
+# SPDX-License-Identifier: Apache-2.0
+
+import argparse
+import re
+
+def report_coverage(hit, total):
+    if total == 0:
+        print("No coverpoints found")
+    else:
+        print(f"{hit}/{total} ({hit/total:.0%})")
+
+
+def collect_coverage(annotation_dir, files):
+    coverpoints_total = 0
+    coverpoints_hit = 0
+
+    # All missed coverpoint lines start with "%000000".
+    # Hit coverpoint lines start with e.g.   " 173092".
+    # Lines with no coverpoints start with   "       ".
+    missed = re.compile("^%000000")
+    hit = re.compile("^ [0-9]{6}")
+
+    for filename in files:
+        try:
+            with open(annotation_dir + "/" + filename, "r") as f:
+                for line in f:
+                    if re.match(missed, line):
+                        coverpoints_total += 1
+                    elif re.match(hit, line):
+                        coverpoints_total += 1
+                        coverpoints_hit += 1
+        except FileNotFoundError:
+            # Verilator doesn't annotate a file if there are no coverpoints.
+            # This is not an issue.
+            pass
+    
+    return coverpoints_hit, coverpoints_total
+
+
+def main():
+    parser = argparse.ArgumentParser(description="Aggregate coverage information from a subset of the available files")
+    parser.add_argument("--annotation-dir", type=str, required=True,
+                        help="Directory containing Verilator coverage annotations")
+    parser.add_argument("--files", nargs="+", required=True,
+                        help="List of filenames to accumulate coverage data from")
+    args = parser.parse_args()
+
+    results = collect_coverage(args.annotation_dir, args.files)
+    report_coverage(*results)
+
+
+if __name__ == "__main__":
+    main()

test/tilelink/Makefile

@@ -45,8 +45,6 @@ define config_name
 $(word 2,$(subst -, ,$1))
 endef
 
-# TODO: add a proper testing command which runs all simulations without coverage.
-
 MUNTJAC_ROOT = ../..
 BUILD_DIR    = build/lowrisc_tl_test_$(DUT)_0.1/sim-verilator
 FUSESOC     ?= ~/.local/bin/fusesoc
@@ -99,21 +97,16 @@ $(FCOV_FILES): $(TOTAL_COV)
 
 # Summarise functional coverage results.
 functional_coverage: $(FCOV_FILES)
-	$(eval TOTAL := $(shell grep -Eh ^.[0-9]{6} $^ | wc -l))
-	$(eval REACHED := $(shell grep -Eh ^.[0-9]{6} $^ | grep -v ^.000000 | wc -l))
-	@echo "Functional coverage ($(REACHED)/$(TOTAL)) $$(( $(REACHED) * 100 / $(TOTAL) ))%"
+	@echo -n "Functional coverage: "
+	@python3 $(MUNTJAC_ROOT)/test/coverage/coverage_filter.py --annotation-dir $(ANNOTATION_DIR) --files $(COVERAGE_SRC)
 
 # Summarise line coverage results.
 # Ignore coverage results for all files except the DUT. Depending on how the DUT
 # uses its subcomponents, it may not be possible to achieve a high coverage.
 # Assume that subcomponents will be tested in isolation elsewhere.
-# TODO: this can fail if there are no cover points in the DUT (i.e. it delegates
-#       to subcomponents).
 line_coverage: $(FCOV_FILES)
-	$(eval FILES := $(ANNOTATION_DIR)/$(DUT).sv)
-	$(eval TOTAL := $(shell grep -Eh ^.[0-9]{6} $(FILES) | wc -l))
-	$(eval REACHED := $(shell grep -Eh ^.[0-9]{6} $(FILES) | grep -v ^.000000 | wc -l))
-	@echo "Line coverage, DUT only ($(REACHED)/$(TOTAL)) $$(( $(REACHED) * 100 / $(TOTAL) ))%"
+	@echo -n "Line coverage, DUT only: "
+	@python3 $(MUNTJAC_ROOT)/test/coverage/coverage_filter.py --annotation-dir $(ANNOTATION_DIR) --files $(DUT).sv
 
 .PHONY: clean
 clean:

test/tilelink/src/tl_harness.h

@@ -30,16 +30,13 @@ class TileLinkSimulation : public Simulation<DUT> {
 public:
   TileLinkSimulation(string name, vector<tl_test>& tests) :
       Simulation<DUT>(name),
-      tests(tests),
-      coverage_file("coverage.dat") {
-    coverage_on = false;
+      tests(tests) {
     sim_duration = 0;
     randomise = false;
 
     this->args.set_description("Usage: " + name + " [simulator args] [tests to run]");
     this->args.add_argument("--list-tests", "List all available tests");
     this->args.add_argument("--config", "Load host/device configuration from a file", ArgumentParser::ARGS_ONE);
-    this->args.add_argument("--coverage", "Dump coverage information to a file", ArgumentParser::ARGS_ONE);
     this->args.add_argument("--random-seed", "Set the random seed", ArgumentParser::ARGS_ONE);
     this->args.add_argument("--run", "Generate random traffic for the given duration (in cycles)", ArgumentParser::ARGS_ONE);
   }
@@ -135,9 +132,6 @@ class TileLinkSimulation : public Simulation<DUT> {
 
     this->trace_close();
 
-    if (coverage_on)
-      Verilated::threadContextp()->coveragep()->write(coverage_file.c_str());
-
     cout << "No assertions triggered" << endl;
   }
 
@@ -157,11 +151,6 @@ class TileLinkSimulation : public Simulation<DUT> {
     if (this->args.found_arg("--config"))
       config_file = this->args.get_arg("--config");
 
-    if (this->args.found_arg("--coverage")) {
-      coverage_on = true;
-      coverage_file = this->args.get_arg("--coverage");
-    }
-
     if (this->args.found_arg("--random-seed"))
       srand(std::stoi(this->args.get_arg("--random-seed")));
 
@@ -237,9 +226,6 @@ class TileLinkSimulation : public Simulation<DUT> {
   // Configuration of hosts/devices connected to the TileLink network.
   string config_file = "configs/default/config.yaml";
 
-  bool coverage_on;
-  string coverage_file;
-
   vector<TileLinkHost*> hosts;
   vector<TileLinkDevice*> devices;
 };