ROM memory in SystemVerilog and cocotb

Question

The Verilog module describes a ROM memory. An initialization file is needed for the INIT_FILE parameter.

Below are Makefile, gtkwave.tcl to launch gtkwave, launch_tb.py (please use it to start simulation) and rom_tb.py as well as rom.sv.

Refer also to the cocotb home page.

rom.sv

module rom #
(
 parameter integer DATA_WIDTH = 8,
 parameter integer ADDR_WIDTH = 8,
 
 parameter string INIT_FILE = ""
)
(
 input logic clk_i,
 input logic [ADDR_WIDTH - 1 : 0] addr_i,
 output logic [ADDR_WIDTH - 1 : 0] data_o
);
 localparam integer MEM_DEPTH = 2 ** ADDR_WIDTH;
 logic [ADDR_WIDTH - 1 : 0] data;
 logic [ADDR_WIDTH - 1 : 0] rom_mem [0 : MEM_DEPTH - 1];
 initial 
 begin
 if (INIT_FILE != "")
 begin
 $display("loading rom");
 $readmemh(INIT_FILE, rom_mem);
 end
 else
 begin
 $error("init file is needed");
 end
 end
 always_ff @ (posedge clk_i)
 begin
 data <= rom_mem[addr_i];
 end
 always_comb
 begin
 data_o = data;
 end
 initial begin
 $dumpfile("dump.vcd");
 $dumpvars(1, rom);
 end
endmodule

Makefile

SIM?=icarus
TOPLEVEL_LANG?=verilog
WAVES?=0
COCOTB_HDL_TIMEUNIT=1ns
COCOTB_HDL_TIMEPRECISION=1ns
SRC_DIR=../../rtl
DUT=rom
TOPLEVEL=$(DUT)
MODULE=$(DUT)_tb
VERILOG_SOURCES+=$(SRC_DIR)/$(DUT).sv
DATA_WIDTH?=8
ADDR_WIDTH?=3
INIT_FILE?=$(shell pwd)/rom_init.txt
COMPILE_ARGS+=-P$(TOPLEVEL).DATA_WIDTH=$(DATA_WIDTH)
COMPILE_ARGS+=-P$(TOPLEVEL).ADDR_WIDTH=$(ADDR_WIDTH)
COMPILE_ARGS+=-P$(TOPLEVEL).INIT_FILE=$(INIT_FILE)
ifeq ($(WAVES), 1)
 VERILOG_SOURCES+=iverilog_dump.v
 COMPILE_ARGS+=-s iverilog_dump
else
 GFLAGS=-S gtkwave.tcl
endif
include $(shell cocotb-config --makefiles)/Makefile.sim
all:
 echo $(COMPILE_ARGS)
 gtkwave $(GFLAGS) *.vcd 2>/dev/null || $(GTKWAVE_OSX) $(GFLAGS) *.vcd 2>/dev/null
iverilog_dump.v:
 echo 'module iverilog_dump();' > $@
 echo 'initial begin' >> $@
 echo ' $$dumpfile("$(TOPLEVEL).fst");' >> $@
 echo ' $$dumpvars(0, $(TOPLEVEL));' >> $@
 echo 'end' >> $@
 echo 'endmodule' >> $@
clean::
 rm -rf *.vcd

rom_tb.py

#!/usr/bin/python
import random
import cocotb
from cocotb.clock import Clock
from cocotb.triggers import RisingEdge
class Rom_tb(object):
 def __init__(self, dut):
 self.dut = dut
 cocotb.start_soon(Clock(self.dut.clk_i, 10, units="ns").start())
 async def read_addr(self, addr):
 self.dut.addr_i.value = addr
 await RisingEdge(self.dut.clk_i)
 await RisingEdge(self.dut.clk_i)
 return self.dut.data_o.value
 async def read_rand_addr(self, max_range):
 rand_addr = random.randint(0, max_range)
 cocotb.log.info(f"Random address: {rand_addr} Data: {await self.read_addr(rand_addr)}")
 #todo: compare with file content 
 async def reading_whole_mem(self, max_range):
 for addr in range(0, max_range):
 cocotb.log.info(f"Address: {addr} Data: {await self.read_addr(addr)}")
 
@cocotb.test()
async def main(dut):
 highest_addr = 8
 max_rand_val = 7
 cocotb.log.info("Start of simulation")
 tb = Rom_tb(dut)
 await tb.reading_whole_mem(highest_addr)
 await tb.read_rand_addr(max_rand_val)
 await tb.read_rand_addr(max_rand_val)
 cocotb.log.info("End of simulation")
 
if __name__ == "__main__":
 sys.exit(main())

gtkwave.tcl

set nfacs [ gtkwave::getNumFacs ]
set all_facs [list]
for {set i 0} {$i < $nfacs } {incr i} {
 set facname [ gtkwave::getFacName $i ]
 lappend all_facs "$facname"
}
set num_added [ gtkwave::addSignalsFromList $all_facs ]
puts "num signals added: $num_added"
# zoom full
gtkwave::/Time/Zoom/Zoom_Full
# Print
set dumpname [ gtkwave::getDumpFileName ]
gtkwave::/File/Print_To_File PDF {Letter (8.5" x 11")} Minimal $dumpname.pdf

launch_tb.py

#!/usr/bin/python
import subprocess
print("cleaning...")
subprocess.run(["make", "clean"])
print("launching the tb")
subprocess.run(["make"])

Answer 1

This review is limited to the SystemVerilog code (rom.sv).

You declared the DATA_WIDTH parameter, but your code does not use it. I assume you really want to use it for the declarations of the data and memory variables. You probably didn't notice since you assigned the WIDTH parameters to the same value (8).

Otherwise, the code is easy to understand, uses descriptive variable names and makes good use of parameters for constant values.

As far as the code layout goes, I prefer to make better use of vertical space by having the begin and end keywords "cuddled" on lines with other meaningful code, as you did with one of your initial blocks. I also prefer 4 single spaces for indentation over 2.

module rom #
(
 parameter integer DATA_WIDTH = 8,
 parameter integer ADDR_WIDTH = 8,
 
 parameter string INIT_FILE = ""
)
(
 input logic clk_i,
 input logic [ADDR_WIDTH - 1 : 0] addr_i,
 output logic [DATA_WIDTH - 1 : 0] data_o
);
 localparam integer MEM_DEPTH = 2 ** ADDR_WIDTH;
 logic [DATA_WIDTH - 1 : 0] data;
 logic [DATA_WIDTH - 1 : 0] rom_mem [0 : MEM_DEPTH - 1];
 initial begin
 if (INIT_FILE != "") begin
 $display("loading rom");
 $readmemh(INIT_FILE, rom_mem);
 end else begin
 $error("init file is needed");
 end
 end
 always_ff @ (posedge clk_i) begin
 data <= rom_mem[addr_i];
 end
 always_comb begin
 data_o = data;
 end
 initial begin
 $dumpfile("dump.vcd");
 $dumpvars(1, rom);
 end
endmodule

• Artem Shimko: Thank you for the review. Yes, you are right an absence of DATA_WIDTH in code is bug. That has been fixed.