Skip to content
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.

Sign up for GitHub

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

Restore Verilator compile/simulation #2585

Open
wants to merge 2 commits into
base: cv32e40p/dev
Choose a base branch
from
Open

Restore Verilator compile/simulation #2585

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0e22da6
Restore Verilator compile/simulation
MikeOpenHWGroup Feb 10, 2025
a3d5c66
Module name must match filename
MikeOpenHWGroup Feb 11, 2025
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
23 changes: 16 additions & 7 deletions cv32e40p/sim/core/Makefile
View file
Original file line number Diff line number Diff line change
@@ -134,15 +134,15 @@ SIMV = ./simv
# dsim is the Metrics Technologies SystemVerilog simulator (https://metrics.ca/)
DSIM = dsim
DSIM_HOME = /tools/Metrics/dsim
DSIM_CMP_FLAGS = +define+CORE_TB -timescale 1ns/1ps $(SV_CMP_FLAGS) -suppress MultiBlockWrite +define+CV32E40P_APU_TRACE
DSIM_CMP_FLAGS = +define+CORE_TB +define+CV32E40P_APU_TRACE +acc+b -timescale 1ns/1ps $(SV_CMP_FLAGS)
DSIM_SUPPRESS = -suppress MultiBlockWrite -suppress IneffectiveDynamicCast
DSIM_RUN_FLAGS =
DSIM_UVM_ARGS = +incdir+$(UVM_HOME)/src $(UVM_HOME)/src/uvm_pkg.sv
DSIM_RESULTS ?= $(PWD)/dsim_results
DSIM_WORK ?= $(DSIM_RESULTS)/dsim_work
DSIM_IMAGE = dsim.out

ifneq (${WAVES}, 0)
DSIM_CMP_FLAGS += +acc
DSIM_DMP_FILE ?= dsim.fst
DSIM_RUN_FLAGS += -waves $(DSIM_DMP_FILE)
endif
@@ -155,7 +155,8 @@ XRUN_DIR = xcelium.d
# verilator configuration
VERILATOR = verilator
VERI_FLAGS +=
VERI_COMPILE_FLAGS += -Wno-BLKANDNBLK $(SV_CMP_FLAGS) # hope this doesn't hurt us in the long run
VERI_COMPILE_FLAGS += $(SV_CMP_FLAGS)
VERI_WARN_WAIVERS ?= --Wno-lint --Wno-UNOPTFLAT --Wno-MODDUP --Wno-MULTIDRIVEN --Wno-COMBDLY --Wno-BLKANDNBLK
VERI_TRACE ?=
VERI_OBJ_DIR ?= cobj_dir
#VERI_LOG_DIR ?= cobj_dir/logs
@@ -276,9 +277,13 @@ mk_results:

# Metrics dsim compile targets
dsim-comp: mk_results CV_CORE_pkg tbsrc_pkg tbsrc
@echo "$(BANNER)"
@echo "* Compiling with Metrics DSIM"
@echo "$(BANNER)"
cd $(DSIM_RESULTS) && \
$(DSIM) \
$(DSIM_CMP_FLAGS) \
$(DSIM_SUPPRESS) \
$(DSIM_UVM_ARGS) \
-f $(CV_CORE_MANIFEST) \
$(TBSRC_PKG) \
@@ -287,8 +292,12 @@ dsim-comp: mk_results CV_CORE_pkg tbsrc_pkg tbsrc
-genimage $(DSIM_IMAGE)

dsim-comp-rtl-only: mk_results $(CV_CORE_PKG)
@echo "$(BANNER)"
@echo "* Compiling (only) with Metrics DSIM"
@echo "$(BANNER)"
$(DSIM) \
$(DSIM_CMP_FLAGS) \
$(DSIM_SUPPRESS) \
-f $(CV_CORE_MANIFEST) \
-work $(DSIM_WORK) \
-genimage $(DSIM_IMAGE)
@@ -313,7 +322,7 @@ dsim-test: dsim-comp $(TEST_PROGRAM_PATH)/$(TEST)/$(TEST).hex
-work $(DSIM_WORK) \
$(DSIM_RUN_FLAGS) \
-sv_lib $(UVM_HOME)/src/dpi/libuvm_dpi.so \
+firmware=$(VERI_CUSTOM)/$(TEST)/$(TEST).hex
+firmware=../../$(TEST_PROGRAM_RELPATH)/$(TEST)/$(TEST).hex

# Metrics dsim cleanup
.PHONY: dsim-clean
@@ -490,13 +499,13 @@ testbench_verilator: CV_CORE_pkg $(TBSRC_VERI) $(TBSRC_PKG)
@echo "$(BANNER)"
$(VERILATOR) --cc --sv --exe \
$(VERI_TRACE) \
--Wno-lint --Wno-UNOPTFLAT \
--Wno-MODDUP --top-module \
$(VERI_WARN_WAIVERS) \
--top-module \
tb_top_verilator $(TBSRC_VERI) \
-f $(CV_CORE_MANIFEST) \
$(CV_CORE_PKG)/bhv/$(CV_CORE_LC)_core_log.sv \
$(TBSRC_CORE)/tb_top_verilator.cpp --Mdir $(VERI_OBJ_DIR) \
-CFLAGS "-std=gnu++11 $(VERI_CFLAGS)" \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose here "-std=gnu++11" was removed for it is no necessary on Verilator supported (or recent) versions.

I imagine this modification should also be done to the same targets for core-v-verif/cv32e40s/sim/core/Makefile:468 and core-v-verif/cv32e40x/sim/core/Makefile

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done as per the discussion of Verilator Issue #4898 which recommends removing -std=c++11.

You are right that we should do the same for cv32e40s and cv32e40x - those should be done on their respective cv32e40[s|x]/dev branches.

-CFLAGS "$(VERI_CFLAGS)" \
$(VERI_COMPILE_FLAGS)
$(MAKE) -C $(VERI_OBJ_DIR) -f Vtb_top_verilator.mk
mkdir -p $(SIM_RESULTS)
2 changes: 1 addition & 1 deletion cv32e40p/tb/core/cv32e40p_tb_wrapper.sv
View file
Original file line number Diff line number Diff line change
@@ -15,7 +15,7 @@
//
// SPDX-License-Identifier: Apache-2.0 WITH SHL-0.51

module cv32e40p_tb_wrapper
module cv32e40p_core_tb_wrapper
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a particular reason for the renaming? I agree that adding _core looks better, but now the name of the file should also follow it :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! You are right, the filename should have been updated to match the module name. That also means updating the file-list in the Makefile. I have pushed in these changes on this pull-request.

#(parameter // Parameters used by TB
INSTR_RDATA_WIDTH = 32,
RAM_ADDR_WIDTH = 20,
6 changes: 3 additions & 3 deletions cv32e40p/tb/core/tb_top.sv
View file
Original file line number Diff line number Diff line change
@@ -70,7 +70,7 @@ module tb_top
if($test$plusargs("verbose"))
$display("[TESTBENCH] @ t=%0t: loading firmware %0s",
$time, firmware);
$readmemh(firmware, cv32e40p_tb_wrapper_i.ram_i.dp_ram_i.mem);
$readmemh(firmware, cv32e40p_core_tb_wrapper_i.ram_i.dp_ram_i.mem);
end else begin
$display("No firmware specified");
$finish;
@@ -145,13 +145,13 @@ module tb_top
end

// wrapper for CV32E40P, the memory system and stdout peripheral
cv32e40p_tb_wrapper
cv32e40p_core_tb_wrapper
#(
.INSTR_RDATA_WIDTH (INSTR_RDATA_WIDTH),
.RAM_ADDR_WIDTH (RAM_ADDR_WIDTH),
.BOOT_ADDR (BOOT_ADDR)
)
cv32e40p_tb_wrapper_i
cv32e40p_core_tb_wrapper_i
(
.clk_i ( core_clk ),
.rst_ni ( core_rst_n ),
4 changes: 2 additions & 2 deletions cv32e40p/tb/core/tb_top_verilator.cpp
View file
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ int main(int argc, char **argv, char **env)
top = new Vtb_top_verilator();

svSetScope(svGetScopeFromName(
"TOP.tb_top_verilator.cv32e40p_tb_wrapper_i.ram_i.dp_ram_i"));
"TOP.tb_top_verilator.cv32e40p_core_tb_wrapper_i.ram_i.dp_ram_i"));
Verilated::scopesDump();

#ifdef VCD_TRACE
@@ -56,7 +56,7 @@ int main(int argc, char **argv, char **env)

#ifdef MCY
svSetScope(svGetScopeFromName(
"TOP.tb_top_verilator.cv32e40p_tb_wrapper_i.riscv_core_i.ex_stage_i.alu_i.int_div.div_i"));
"TOP.tb_top_verilator.cv32e40p_core_tb_wrapper_i.riscv_core_i.ex_stage_i.alu_i.int_div.div_i"));
svLogicVecVal idx = {0};
idx.aval = mutidx;
set_mutidx(&idx);
6 changes: 3 additions & 3 deletions cv32e40p/tb/core/tb_top_verilator.sv
View file
Original file line number Diff line number Diff line change
@@ -39,7 +39,7 @@ module tb_top_verilator
if($test$plusargs("verbose"))
$display("[TESTBENCH] %t: loading firmware %0s ...",
$time, firmware);
$readmemh(firmware, cv32e40p_tb_wrapper_i.ram_i.dp_ram_i.mem);
$readmemh(firmware, cv32e40p_core_tb_wrapper_i.ram_i.dp_ram_i.mem);

end else begin
$display("No firmware specified");
@@ -85,7 +85,7 @@ module tb_top_verilator
end

// wrapper for cv32e40p, the memory system and stdout peripheral
cv32e40p_tb_wrapper
cv32e40p_core_tb_wrapper
#(.INSTR_RDATA_WIDTH (INSTR_RDATA_WIDTH),
.RAM_ADDR_WIDTH (RAM_ADDR_WIDTH),
.BOOT_ADDR (BOOT_ADDR),
@@ -94,7 +94,7 @@ module tb_top_verilator
.ZFINX (0),
.DM_HALTADDRESS (32'h1A110800)
)
cv32e40p_tb_wrapper_i
cv32e40p_core_tb_wrapper_i
(.clk_i ( clk_i ),
.rst_ni ( rst_ni ),
.fetch_enable_i ( fetch_enable_i ),