mirror of
https://github.com/retroSoC/retroSoC.git
synced 2026-11-06 14:55:14 +00:00
refactor: refactor RTL style checker and update test cases
- Enhanced the RTL style checker script to include new diagnostic rules for function declarations, primary design units, and synthesis exclusions. - Added functions to handle audit issues, ensuring that the audit records match the current inventory of RTL files. - Updated test cases to reflect changes in RTL file names and paths, ensuring consistency with the new naming conventions. - Introduced additional tests for style violations, including mismatched primary design units and synthesizable initial blocks. - Adjusted file paths in various test files to align with the new RTL structure and naming conventions.
This commit is contained in:
116 files changed
+3310
-2624
No files matched your search
+1
-1
@@ -44,7 +44,7 @@ def test_dma_reports_bus_errors_and_transfers_exact_word_count(tmp_path: Path) -
|
||||
str(ROOT / "rtl/mini/top/rib_if.sv"),
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/utils/register.sv"),
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/utils/fifo.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/dma.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/ribp_dma.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/dma_core.sv"),
|
||||
str(ROOT / "tests/rtl/dma_error_tb.sv"),
|
||||
"",
|
||||
|
||||
+1
-1
@@ -47,7 +47,7 @@ def test_dvp_capture_framing(tmp_path: Path) -> None:
|
||||
str(ROOT / "rtl/tech/tc_clk.sv"),
|
||||
str(ip / "dvp_core.sv"),
|
||||
str(ip / "dvp_reg.sv"),
|
||||
str(ip / "dvp.sv"),
|
||||
str(ip / "axi4s_dvp.sv"),
|
||||
str(ROOT / "tests/rtl/dvp_tb.sv"),
|
||||
]
|
||||
source_list = tmp_path / "dvp.fl"
|
||||
|
||||
@@ -158,7 +158,7 @@ def test_bus_fault_responder_handles_reserved_and_unmapped_addresses(tmp_path: P
|
||||
str(ROOT / "rtl/mini/top/rib2ribp.sv"),
|
||||
str(ROOT / "rtl/mini/top/rib_error_slave.sv"),
|
||||
str(ROOT / "rtl/mini/top/rib2ram.sv"),
|
||||
str(ROOT / "rtl/mini/top/bus.sv"),
|
||||
str(ROOT / "rtl/mini/top/rib_bus.sv"),
|
||||
str(ROOT / "tests/rtl/bus_fault_tb.sv"),
|
||||
"",
|
||||
]
|
||||
@@ -212,7 +212,7 @@ def test_sysctrl_fault_registers_record_and_clear_pending(tmp_path: Path) -> Non
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/utils/register.sv"),
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/cdc/cdc_sync.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/pll_ctrl_if.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/sysctrl.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/ribp_sysctrl.sv"),
|
||||
str(ROOT / "tests/rtl/sysctrl_fault_tb.sv"),
|
||||
"",
|
||||
]
|
||||
@@ -267,10 +267,11 @@ def test_pll_controller_reconfigures_and_falls_back_to_the_safe_clock(tmp_path:
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/utils/edge_det.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/pll_ctrl_if.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/clint_timebase.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/sysctrl.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/ribp_sysctrl.sv"),
|
||||
str(ROOT / "rtl/tech/tc_clk.sv"),
|
||||
str(ROOT / "rtl/tech/tc_pll.sv"),
|
||||
str(ROOT / "rtl/mini/top/rcu.sv"),
|
||||
str(ROOT / "rtl/mini/top/pll_rcu_controller.sv"),
|
||||
str(ROOT / "tests/rtl/pll_ctrl_tb.sv"),
|
||||
"",
|
||||
]
|
||||
@@ -338,7 +339,7 @@ def test_pll_controller_reconfigures_and_falls_back_to_the_safe_clock(tmp_path:
|
||||
|
||||
|
||||
def test_sysctrl_does_not_expose_unused_i2c_or_qspi_select_registers() -> None:
|
||||
rtl = (ROOT / "rtl/ip/peripheral/sysctrl.sv").read_text(encoding="utf-8")
|
||||
rtl = (ROOT / "rtl/ip/peripheral/ribp_sysctrl.sv").read_text(encoding="utf-8")
|
||||
header = (ROOT / "crt/include/retrosoc/core/soc.h").read_text(encoding="utf-8")
|
||||
|
||||
for symbol in (
|
||||
|
||||
+1
-1
@@ -49,7 +49,7 @@ def test_bus_preserves_incr4_and_rejects_illegal_bursts(tmp_path: Path) -> None:
|
||||
str(ROOT / "rtl/mini/top/rib2ribp.sv"),
|
||||
str(ROOT / "rtl/mini/top/rib_error_slave.sv"),
|
||||
str(ROOT / "rtl/mini/top/rib2ram.sv"),
|
||||
str(ROOT / "rtl/mini/top/bus.sv"),
|
||||
str(ROOT / "rtl/mini/top/rib_bus.sv"),
|
||||
str(ROOT / "tests/rtl/rib_bus_tb.sv"),
|
||||
"",
|
||||
]
|
||||
|
||||
+120
-3
@@ -8,6 +8,7 @@ from pathlib import Path
|
||||
|
||||
ROOT = Path(__file__).resolve().parents[1]
|
||||
SCRIPT = ROOT / "scripts/check_rtl_style.py"
|
||||
AUDIT = ROOT / "rtl/rtl_style_audit.json"
|
||||
|
||||
|
||||
def run_style(
|
||||
@@ -119,7 +120,7 @@ def test_owned_style_accepts_project_naming_contract(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"`define RETROSOC_TEST__VALUE 1\n"
|
||||
"module test_module(\n"
|
||||
"module test(\n"
|
||||
" input logic clk_i,\n"
|
||||
" output logic data_o\n"
|
||||
");\n"
|
||||
@@ -135,7 +136,7 @@ def test_owned_style_accepts_project_naming_contract(tmp_path: Path) -> None:
|
||||
def test_owned_style_allows_protocol_localparams(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"module test_module #(parameter int DataWidth = 32) ("
|
||||
"module test #(parameter int DataWidth = 32) ("
|
||||
"input logic clk_i, output logic data_o);\n"
|
||||
" localparam logic [1:0] FSM_IDLE = 2'd0;\n"
|
||||
" localparam logic [1:0] FSM_RESP = 2'd1;\n"
|
||||
@@ -151,7 +152,7 @@ def test_owned_style_allows_interface_members_and_include_guards(tmp_path: Path)
|
||||
tmp_path,
|
||||
"`ifndef GPIO_DEFINE_SVH\n"
|
||||
"`define GPIO_DEFINE_SVH\n"
|
||||
"interface test_if ();\n"
|
||||
"interface test ();\n"
|
||||
" logic cmd_valid;\n"
|
||||
" modport dut(input cmd_valid);\n"
|
||||
"endinterface\n"
|
||||
@@ -159,3 +160,119 @@ def test_owned_style_allows_interface_members_and_include_guards(tmp_path: Path)
|
||||
enforce_naming=True,
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
|
||||
|
||||
def test_owned_style_rejects_mismatched_primary_design_unit(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"module wrong_name(input logic clk_i, output logic data_o);\n"
|
||||
" assign data_o = clk_i;\n"
|
||||
"endmodule\n",
|
||||
)
|
||||
assert result.returncode == 1
|
||||
assert "RTL-FILE-002" in result.stderr
|
||||
|
||||
|
||||
def test_owned_style_rejects_synthesizable_initial_block(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"module test(input logic clk_i, output logic data_o);\n"
|
||||
" initial begin\n"
|
||||
" data_o = clk_i;\n"
|
||||
" end\n"
|
||||
"endmodule\n",
|
||||
)
|
||||
assert result.returncode == 1
|
||||
assert "RTL-SV-007" in result.stderr
|
||||
|
||||
|
||||
def test_owned_style_allows_verification_only_initial_block(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"module test(input logic clk_i, output logic data_o);\n"
|
||||
" assign data_o = clk_i;\n"
|
||||
"`ifndef SYNTHESIS\n"
|
||||
" initial begin\n"
|
||||
" if (clk_i) $error(\"unexpected clock\");\n"
|
||||
" end\n"
|
||||
"`endif\n"
|
||||
"endmodule\n",
|
||||
)
|
||||
assert result.returncode == 0, result.stderr
|
||||
|
||||
|
||||
def test_owned_style_rejects_unpaired_format_directive(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"module test(input logic clk_i, output logic data_o);\n"
|
||||
" // verilog_format: on\n"
|
||||
" assign data_o = clk_i;\n"
|
||||
"endmodule\n",
|
||||
)
|
||||
assert result.returncode == 1
|
||||
assert "RTL-FMT-005" in result.stderr
|
||||
|
||||
|
||||
def test_owned_style_requires_formatter_exception_rationale(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"module test(input logic clk_i, output logic data_o);\n"
|
||||
" // verilog_format: off\n"
|
||||
" assign data_o = clk_i;\n"
|
||||
" // verilog_format: on\n"
|
||||
"endmodule\n",
|
||||
)
|
||||
assert result.returncode == 1
|
||||
assert "RTL-FMT-007" in result.stderr
|
||||
|
||||
|
||||
def test_owned_style_requires_automatic_typed_functions(tmp_path: Path) -> None:
|
||||
result = run_style(
|
||||
tmp_path,
|
||||
"module test(input logic clk_i, output logic data_o);\n"
|
||||
" function logic invert(input logic value_i);\n"
|
||||
" return !value_i;\n"
|
||||
" endfunction\n"
|
||||
" assign data_o = invert(clk_i);\n"
|
||||
"endmodule\n",
|
||||
)
|
||||
assert result.returncode == 1
|
||||
assert "RTL-SV-008" in result.stderr
|
||||
|
||||
|
||||
def test_owned_style_audit_matches_current_inventory() -> None:
|
||||
audit = json.loads(AUDIT.read_text(encoding="utf-8"))
|
||||
assert audit["schema_version"] == 1
|
||||
assert audit["policy"] == "docs/rtl-coding-style.md"
|
||||
assert audit["profile"] == "owned"
|
||||
assert {
|
||||
"owner",
|
||||
"related_commit",
|
||||
"expiry",
|
||||
"removal_plan",
|
||||
}.issubset(audit["reviewed_boundary_record"])
|
||||
|
||||
audited_paths = {
|
||||
str(Path(directory) / name)
|
||||
for directory, names in audit["inventory"].items()
|
||||
for name in names
|
||||
}
|
||||
actual_paths = {
|
||||
str(path.relative_to(ROOT))
|
||||
for root in (ROOT / "rtl/ip", ROOT / "rtl/mini/top")
|
||||
for path in root.rglob("*")
|
||||
if path.is_file() and path.suffix in {".sv", ".svh"}
|
||||
}
|
||||
assert audited_paths == actual_paths
|
||||
assert {
|
||||
"RTL-FILE",
|
||||
"RTL-SV",
|
||||
"RTL-NAME",
|
||||
"RTL-STRUCT",
|
||||
"RTL-COMB",
|
||||
"RTL-SEQ",
|
||||
"RTL-WIDTH",
|
||||
"RTL-CDC",
|
||||
"RTL-VERIFY",
|
||||
"RTL-FMT",
|
||||
}.issubset({rule["id"] for rule in audit["rules"]})
|
||||
@@ -246,7 +246,7 @@ def test_formal_filelists_are_scoped_to_the_protocol_duts(tmp_path: Path) -> Non
|
||||
i2c = parse_filelists([i2c_filelist], require_files=False)
|
||||
clint = parse_filelists([clint_filelist], require_files=False)
|
||||
assert "+define+SV_ASSRT_DISABLE" in bus.defines
|
||||
assert ROOT / "rtl/mini/top/bus.sv" in bus.files
|
||||
assert ROOT / "rtl/mini/top/rib_bus.sv" in bus.files
|
||||
assert ROOT / "rtl/mini/top/rib_if.sv" in bus.files
|
||||
assert ROOT / "rtl/mini/top/ribp2rib.sv" in bus.files
|
||||
assert ROOT / "rtl/mini/top/rib_error_slave.sv" in bus.files
|
||||
@@ -256,10 +256,11 @@ def test_formal_filelists_are_scoped_to_the_protocol_duts(tmp_path: Path) -> Non
|
||||
assert ROOT / "rtl/mini/top/rib2apb.sv" in rib2apb.files
|
||||
assert ROOT / "rtl/mini/formal/rib2apb_formal.sv" in rib2apb.files
|
||||
assert ROOT / "rtl/managed/clusterip/common/rtl/interface/apb4_pure_if.sv" in rib2apb.files
|
||||
assert ROOT / "rtl/ip/peripheral/sysctrl.sv" in sysctrl.files
|
||||
assert ROOT / "rtl/ip/peripheral/ribp_sysctrl.sv" in sysctrl.files
|
||||
assert ROOT / "rtl/managed/clusterip/common/rtl/cdc/cdc_sync.sv" in sysctrl.files
|
||||
assert ROOT / "rtl/mini/formal/sysctrl_formal.sv" in sysctrl.files
|
||||
assert ROOT / "rtl/mini/top/rcu.sv" in pll_rcu.files
|
||||
assert ROOT / "rtl/mini/top/pll_rcu_controller.sv" in pll_rcu.files
|
||||
assert ROOT / "rtl/managed/clusterip/common/rtl/cdc/cdc_2phase.sv" in pll_rcu.files
|
||||
assert ROOT / "rtl/managed/clusterip/common/rtl/clkrst/rst_sync.sv" in pll_rcu.files
|
||||
assert ROOT / "rtl/ip/peripheral/gpio_core.sv" in gpio.files
|
||||
@@ -586,7 +587,7 @@ def test_format_file_scope_is_tracked_and_self_owned() -> None:
|
||||
Path("Makefile"),
|
||||
Path("configs/ci/example.mk"),
|
||||
Path("rtl/mini/top/retrosoc.sv"),
|
||||
Path("rtl/ip/peripheral/sysctrl.sv"),
|
||||
Path("rtl/ip/peripheral/ribp_sysctrl.sv"),
|
||||
Path("rtl/tech/tc_clk.sv"),
|
||||
Path("rtl/demo/reference.v"),
|
||||
Path("tests/rtl/bus_fault_tb.sv"),
|
||||
@@ -601,7 +602,7 @@ def test_format_file_scope_is_tracked_and_self_owned() -> None:
|
||||
]
|
||||
assert format_files(paths, "rtl") == [
|
||||
Path("rtl/demo/reference.v"),
|
||||
Path("rtl/ip/peripheral/sysctrl.sv"),
|
||||
Path("rtl/ip/peripheral/ribp_sysctrl.sv"),
|
||||
Path("rtl/mini/top/retrosoc.sv"),
|
||||
Path("rtl/tech/tc_clk.sv"),
|
||||
Path("tests/rtl/bus_fault_tb.sv"),
|
||||
|
||||
+2
-2
@@ -43,7 +43,7 @@ def test_sdram_controller_preserves_full_and_masked_writes(tmp_path: Path) -> No
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/utils/register.sv"),
|
||||
str(ROOT / "rtl/mini/top/axi42ribp_burst.sv"),
|
||||
str(ROOT / "rtl/mini/top/ribp_arbiter2.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram.sv"),
|
||||
str(ROOT / "rtl/ip/memory/axi4_sdram.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram_clkgen.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram_reg.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram_core.sv"),
|
||||
@@ -109,7 +109,7 @@ def test_sdram_controller_preserves_data_with_micron_timing_model(tmp_path: Path
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/utils/register.sv"),
|
||||
str(ROOT / "rtl/mini/top/axi42ribp_burst.sv"),
|
||||
str(ROOT / "rtl/mini/top/ribp_arbiter2.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram.sv"),
|
||||
str(ROOT / "rtl/ip/memory/axi4_sdram.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram_clkgen.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram_reg.sv"),
|
||||
str(ROOT / "rtl/ip/memory/sdram_core.sv"),
|
||||
|
||||
@@ -100,7 +100,7 @@ def test_ws2812_accepts_dma_fixed_destination_backpressure(tmp_path: Path) -> No
|
||||
str(ROOT / "rtl/mini/top/rib_if.sv"),
|
||||
str(common / "utils/register.sv"),
|
||||
str(common / "utils/fifo.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/dma.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/ribp_dma.sv"),
|
||||
str(ROOT / "rtl/ip/peripheral/dma_core.sv"),
|
||||
str(serial / "ws2812_if.sv"),
|
||||
str(serial / "ws2812_reg.sv"),
|
||||
|
||||
@@ -39,7 +39,7 @@ def test_xpi_releases_data_pads_while_receiving(tmp_path: Path) -> None:
|
||||
f"+incdir+{ROOT / 'rtl/managed/clusterip/common/rtl'}",
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/interface/ribp_if.sv"),
|
||||
str(ROOT / "rtl/managed/clusterip/common/rtl/utils/register.sv"),
|
||||
str(ROOT / "rtl/ip/storage/xpi.sv"),
|
||||
str(ROOT / "rtl/ip/storage/ribp_xpi.sv"),
|
||||
str(ROOT / "rtl/ip/storage/xpi_clkgen.sv"),
|
||||
str(ROOT / "rtl/ip/storage/xpi_core.sv"),
|
||||
str(ROOT / "tests/rtl/xpi_io_tb.sv"),
|
||||
|
||||
Reference in new issue
Block a user