Skip to content

ci: Lint the testbenches too and clear what that exposes #192

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

Merged
DanielKellerM merged 4 commits into from
Aug 17, 2026
Merged

ci: Lint the testbenches too and clear what that exposes #192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
647629f
ci: Lint the testbenches too and clear what that exposes
DanielKellerM Aug 13, 2026
4969e0a
test: Cut the multi-line comments the lint pass left behind
DanielKellerM Aug 17, 2026
03e6252
ci: Cut the multi-line comment on the Verible waiver
DanielKellerM Aug 17, 2026
a4fa02b
test: Give the mxquant vector case an explicit default
DanielKellerM Aug 17, 2026
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
3 changes: 3 additions & 0 deletions .github/verible.waiver
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@
# - Thomas Benz <tbenz@iis.ee.ethz.ch>

waive --rule=package-filename --location="src/frontend/inst64/idma_inst64_opcode.sv"

# IDMA_DV / IDMA_ND_DV are the exported verification API; renaming breaks downstream
waive --rule=interface-name-style --location="test/idma_intf.sv"
47 changes: 20 additions & 27 deletions idma.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,7 @@ VLIB ?= vlib
# iDMA root, resolved via bender so integrators can host iDMA anywhere
IDMA_ROOT ?= $(shell $(BENDER) path idma)

# All generator/doc tooling runs through uv against the locked environment
# (pyproject.toml + uv.lock are the single source of truth).
# All tooling runs through uv against the locked environment
UV_RUN := $(UV) run --locked --project $(IDMA_ROOT)
PYTHON ?= $(UV_RUN) python
PEAKRDL ?= $(UV_RUN) peakrdl
Expand Down Expand Up @@ -62,8 +61,7 @@ IDMA_JOBS_JSON := jobs/jobs.json
IDMA_BENDER_FILES := $(IDMA_ROOT)/Bender.yml \
$(IDMA_ROOT)/Bender.lock

# Helper functions
# Relative paths for VLOGAN
# Helper functions Relative paths for VLOGAN
IDMA_VLOGAN_REL_PATHS := | grep -v "ROOT=" | sed '3 i ROOT="../../.."'

# Ensure half-built targets are purged
Expand Down Expand Up @@ -181,8 +179,7 @@ IDMA_FE_DIR := $(IDMA_ROOT)/src/frontend
IDMA_FE_REGS := desc64
IDMA_FE_REGS += $(IDMA_FE_IDS)

# Config-bus CPUIF for the register frontend: PeakRDL regblock --cpuif + matching wrapper
# packing. apb4-flat (default, industry standard); also obi-flat / passthrough / axi4-lite-flat.
# Config-bus CPUIF: apb4-flat default; also obi-flat, axi4-lite-flat, passthrough
IDMA_REG_CPUIF ?= apb4-flat


Expand Down Expand Up @@ -212,8 +209,7 @@ $(IDMA_RTL_DIR)/idma_reg%d_reg_pkg.sv $(IDMA_RTL_DIR)/idma_reg%d_reg_top.sv $(ID
-P Log2NumDims=$(call log2dimension,$(call dimension,$*))

$(IDMA_RTL_DIR)/idma_desc64_reg_pkg.sv $(IDMA_RTL_DIR)/idma_desc64_reg_top.sv $(IDMA_RTL_DIR)/idma_desc64_addrmap_pkg.sv:
# desc64 has static, hand-written APB reg wrappers (idma_desc64_reg_wrapper.sv); it is
# APB-native and not part of the CPUIF selector — keep its reg_top apb4-flat.
# desc64 is APB-native with hand-written wrappers, outside the CPUIF selector
$(PEAKRDL) regblock $(IDMA_FE_DIR)/desc64/idma_desc64_reg.rdl -o $(IDMA_RTL_DIR) \
--default-reset arst_n --cpuif apb4-flat \
--module-name idma_desc64_reg_top \
Expand Down Expand Up @@ -388,9 +384,7 @@ endef
$(IDMA_VSIM_DIR)/compile.tcl: $(IDMA_BENDER_FILES) $(IDMA_FULL_TB) $(IDMA_FULL_RTL) $(IDMA_INCLUDE_ALL) $(IDMA_WAVE_ALL)
$(call idma_generate_vsim, $@, -t sim -t test -t idma_test -t synth -t rtl -t asic -t snitch_cluster,../../..)

# Standalone self-checking transpose-engine regression (DPI-C golden, no backend deps).
# Run with the Questa SEPP wrapper, e.g.:
# make idma_sim_tb_idma_otf_transpose VSIM="questa-2023.4 vsim" VLOG="questa-2023.4 vlog" VLIB="questa-2023.4 vlib"
# Standalone transpose-engine regression against the DPI-C golden
IDMA_OTF_TP_RTL := $(abspath $(IDMA_ROOT)/src/backend/idma_otf_transpose.sv)
IDMA_OTF_TP_TB := $(abspath $(IDMA_ROOT)/test/tb_idma_otf_transpose.sv)
IDMA_OTF_TP_DPI := $(abspath $(IDMA_ROOT)/test/idma_transpose_dpi.c)
Expand All @@ -403,25 +397,20 @@ idma_sim_tb_idma_otf_transpose:
cd $(IDMA_OTF_TP_DIR); $(VLOG) -sv $(IDMA_OTF_TP_DPI)
cd $(IDMA_OTF_TP_DIR); $(VLOG) -sv -svinputport=compat -timescale "1ns/1fs" $(IDMA_OTF_TP_RTL) $(IDMA_OTF_TP_TB)
# the TB sweeps the geometry list internally; one run per StrbWidth x FullDuplex
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=8 -gFullDuplex=1 tb_idma_otf_transpose +BP -do "run -all; quit"
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=8 -gFullDuplex=0 tb_idma_otf_transpose +BP -do "run -all; quit"
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=64 -gFullDuplex=1 tb_idma_otf_transpose +BP -do "run -all; quit"
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=64 -gFullDuplex=0 tb_idma_otf_transpose +BP -do "run -all; quit"
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=8 -gFullDuplex=1 tb_idma_otf_transpose +BP=1 -do "run -all; quit"
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=8 -gFullDuplex=0 tb_idma_otf_transpose +BP=1 -do "run -all; quit"
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=64 -gFullDuplex=1 tb_idma_otf_transpose +BP=1 -do "run -all; quit"
cd $(IDMA_OTF_TP_DIR); $(VSIM) -c -t 1ps -gStrbWidth=64 -gFullDuplex=0 tb_idma_otf_transpose +BP=1 -do "run -all; quit"

# Multi-tile transpose via the ND midend -> rw_axi backend -> axi_sim_mem.
# Run with the Questa SEPP wrapper: make idma_sim_tb_idma_transpose_nd VSIM="questa-2023.4 vsim"
# Multi-tile transpose: ND midend to rw_axi backend to axi_sim_mem
.PHONY: idma_sim_tb_idma_transpose_nd
idma_sim_tb_idma_transpose_nd: $(IDMA_VSIM_DIR)/compile.tcl
cd $(IDMA_VSIM_DIR); $(VSIM) -c -do "source compile.tcl; quit"
# the TB sweeps the geometry list internally; one run per bus width
cd $(IDMA_VSIM_DIR); $(VSIM) -c -t 1ps -voptargs=+acc -gDataWidth=32 tb_idma_transpose_nd -do "run -all; quit"
cd $(IDMA_VSIM_DIR); $(VSIM) -c -t 1ps -voptargs=+acc -gDataWidth=64 tb_idma_transpose_nd -do "run -all; quit"

# Back-to-back regressions: the ND midend must reload each new transfer's base
# address (it does, for a protocol-compliant producer that drops nd_req_valid on
# accept). tb_idma_nd_midend_b2b checks the midend's burst-address sequence under
# backpressure; tb_idma_transpose_b2b checks two end-to-end transposes to distinct
# destinations. Run with the Questa SEPP wrapper.
# Back-to-back: the ND midend must reload each transfer base address
.PHONY: idma_sim_tb_idma_nd_midend_b2b
idma_sim_tb_idma_nd_midend_b2b: $(IDMA_VSIM_DIR)/compile.tcl
cd $(IDMA_VSIM_DIR); $(VSIM) -c -do "source compile.tcl; quit"
Expand Down Expand Up @@ -450,9 +439,7 @@ idma_sim_tb_idma_transpose_b2b: $(IDMA_VSIM_DIR)/compile.tcl
cd $(IDMA_VSIM_DIR); $(VSIM) -c -t 1ps -voptargs=+acc -gDataWidth=32 tb_idma_transpose_b2b -do "run -all; quit"
cd $(IDMA_VSIM_DIR); $(VSIM) -c -t 1ps -voptargs=+acc -gDataWidth=64 tb_idma_transpose_b2b -do "run -all; quit"

# Run a self-checking MX sim across data widths. Questa does not propagate
# $fatal to the exit code, so fail on any Error:/Fatal: in the run log.
# $(1) = testbench, $(2) = space-separated data widths.
# MX sim across data widths; Questa hides $fatal, so grep the transcript
define idma_run_mx_sim
cd $(IDMA_VSIM_DIR); set -e; for dw in $(2); do \
$(VSIM) -c -t 1ps -voptargs=+acc -gDataWidth=$$dw $(1) -do "run -all; quit" > $(1)_$$dw.log 2>&1 || true; \
Expand Down Expand Up @@ -624,13 +611,19 @@ IDMA_LINT_TOPS ?= $(addprefix idma_backend_synth_,$(IDMA_BACKEND_IDS)) \
idma_mp_midend_synth \
idma_rt_midend_synth

# lint-sv is -diff scoped; this checks all of src/
# lint-sv is -diff scoped; this checks all of src/ and test/
VERIBLE ?= verible-verilog-lint

IDMA_LINT_SV_DIRS := $(IDMA_ROOT)/src $(IDMA_ROOT)/test

# Vendored third-party sources, exempt here and in authors-cfg.yaml
IDMA_LINT_SV_VENDORED := test/future/TLToAXI4.v test/future/idma_tb_per2axi.sv

.PHONY: idma_lint_sv
idma_lint_sv:
$(VERIBLE) --waiver_files $(IDMA_ROOT)/.github/verible.waiver \
$$(find $(IDMA_ROOT)/src -name '*.sv' -o -name '*.svh' | sort)
$$(find $(IDMA_LINT_SV_DIRS) \( -name '*.sv' -o -name '*.svh' \) \
$(foreach f,$(IDMA_LINT_SV_VENDORED),! -path '*/$(f)') | sort)

.PHONY: idma_lint_elab
idma_lint_elab:
Expand Down
22 changes: 8 additions & 14 deletions test/frontend/idma_inst64_drv_if.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,16 @@ interface idma_inst64_drv_if #(
//--------------------------------------
// Response capture
//--------------------------------------
// acc_res_o comes out of a 2-deep cc_spill_register; with acc_res_ready_i tied high
// it pops on the posedge, so sample it in a clocked process instead of reading it
// combinationally from a task.
// acc_res_o pops on the posedge, so sample it in a clocked process
acc_rsp_item_t rsp_queue [$];

always_ff @(posedge clk) begin : proc_capture_rsp
// built in a variable first: verilator rejects an assignment pattern as an argument
automatic acc_rsp_item_t rsp_item;
if (rst_n && acc_res_valid && acc_res_ready) begin
rsp_item = '{id: acc_res.id, data: acc_res.data, error: acc_res.error};
rsp_queue.push_back(rsp_item);
// typed pattern: verilator rejects a bare assignment pattern as a call argument
rsp_queue.push_back(
acc_rsp_item_t'{id: acc_res.id, data: acc_res.data, error: acc_res.error});
end
end

Expand All @@ -84,9 +83,7 @@ interface idma_inst64_drv_if #(
//--------------------------------------
// Low-level accelerator request driver
//--------------------------------------
// Drive at ApplDelay, sample ready at AcqDelay of the SAME cycle, then release on the
// handshake edge. Sampling ready in the same delta as the drive reads the stale value
// and holds valid across two edges, which issues every instruction twice.
// Sampling ready in the drive delta issues every instruction twice
task automatic acc_issue(
input logic [31:0] data_op,
input logic [63:0] data_arga,
Expand All @@ -113,8 +110,7 @@ interface idma_inst64_drv_if #(
acc_req_valid = 1'b0;
endtask

/// Pop the response the DUT produced for the last issued request; fails on timeout,
/// on an id mismatch or on a flagged error.
// / Pop the response for the last request; fails on timeout, id mismatch or error
task automatic acc_get_rsp(output acc_rsp_item_t item);
int unsigned waited;
waited = 0;
Expand Down Expand Up @@ -203,8 +199,7 @@ interface idma_inst64_drv_if #(
transfer_id = item.data[31:0];
endtask

/// Register-form status read; argb[1:0] = index, argb[4:2] = channel.
/// index 0 = completed_id, 1 = next_id, 2 = busy, 3 = fifo full
// / Status read; index 0 = completed_id, 1 = next_id, 2 = busy, 3 = fifo full
task automatic dma_poll_status(
input logic [1:0] status_idx,
input logic [2:0] channel,
Expand All @@ -217,8 +212,7 @@ interface idma_inst64_drv_if #(
status_value = item.data;
endtask

/// Wait for `transfer_id` to retire. The id generator resets to next=2/completed=1, so
/// the first transfer gets id 2 and the compare below is not vacuous.
// / Wait for retire; ids start at 2, so the compare is not vacuous
task automatic dma_wait(
input tf_id_t transfer_id,
input logic [2:0] channel
Expand Down
Loading
Loading