From 6940681c0ee949c2d8312d56013a91130e8b28dd Mon Sep 17 00:00:00 2001 From: Lucian Petrica Date: Tue, 18 Aug 2026 11:25:49 +0100 Subject: [PATCH] Fix WHS reporting --- linker/slashkit/emit/metadata/timing_freq.py | 41 ++++++++++++-- linker/test/emit/metadata/test_timing_freq.py | 56 ++++++++++++++++--- 2 files changed, 83 insertions(+), 14 deletions(-) diff --git a/linker/slashkit/emit/metadata/timing_freq.py b/linker/slashkit/emit/metadata/timing_freq.py index 4fb9dd92..e6f6146d 100644 --- a/linker/slashkit/emit/metadata/timing_freq.py +++ b/linker/slashkit/emit/metadata/timing_freq.py @@ -33,9 +33,14 @@ HW_BUILD_DIR_ENV_KEYS = ("SLASH_HW_BUILD_DIR", "slash_hw_build_dir") _FLOAT_RE = re.compile(r"[-+]?\d+(?:\.\d+)?") +# Columns are separated by two or more spaces; the multi-word headings +# ("TNS Failing Endpoints") use single spaces internally. +_COLUMN_SPLIT_RE = re.compile(r"\s{2,}") -def _find_design_timing_summary_row(report_text: str) -> Optional[list[float]]: +def _find_design_timing_summary_row( + report_text: str, +) -> Optional[tuple[list[str], list[float]]]: if not report_text: return None @@ -56,6 +61,9 @@ def _find_design_timing_summary_row(report_text: str) -> Optional[list[float]]: if header_idx is None: return None + header = lines[header_idx].strip() + columns = [c for c in _COLUMN_SPLIT_RE.split(header) if c] + for i in range(header_idx + 1, min(header_idx + 20, len(lines))): line = lines[i].strip() if not line: @@ -64,16 +72,39 @@ def _find_design_timing_summary_row(report_text: str) -> Optional[list[float]]: continue values = [float(m.group(0)) for m in _FLOAT_RE.finditer(line)] if values: - return values + return columns, values return None +def _column_value( + columns: list[str], values: list[float], name: str +) -> Optional[float]: + # Index by heading rather than by offset. Vivado interleaves endpoint counts + # between the slack columns -- WNS, TNS, TNS Failing Endpoints, TNS Total + # Endpoints, WHS, THS, ... -- so WHS sits at offset 4, not the 2 that a + # WNS/TNS/WHS/THS reading of the table would suggest. Reading offset 2 + # returned the failing-endpoint count as the hold slack, which silently + # disabled the whs >= 0 half of design_timing_met() (a count is never + # negative) and printed nonsense in the failure message. + if len(columns) != len(values): + return None + try: + return values[columns.index(name)] + except ValueError: + return None + + def extract_design_timing_slacks_ns(report_text: str) -> Optional[tuple[float, float]]: - values = _find_design_timing_summary_row(report_text) - if values is None or len(values) < 3: + row = _find_design_timing_summary_row(report_text) + if row is None: + return None + columns, values = row + wns_ns = _column_value(columns, values, "WNS(ns)") + whs_ns = _column_value(columns, values, "WHS(ns)") + if wns_ns is None or whs_ns is None: return None - return values[0], values[2] + return wns_ns, whs_ns def extract_design_wns_ns(report_text: str) -> Optional[float]: diff --git a/linker/test/emit/metadata/test_timing_freq.py b/linker/test/emit/metadata/test_timing_freq.py index 913975a2..37d41b5a 100644 --- a/linker/test/emit/metadata/test_timing_freq.py +++ b/linker/test/emit/metadata/test_timing_freq.py @@ -23,6 +23,8 @@ import textwrap from pathlib import Path +import pytest + from slashkit.emit.metadata import timing_freq @@ -34,19 +36,21 @@ def _write_system_map(path: Path, clock_hz: int) -> None: ) -def _write_timing_report(path: Path, wns_ns: float) -> None: - # Minimal shape matching what extract_design_wns_ns() scans for: a - # "Design Timing Summary" section, a WNS(ns)/TNS(ns) header, then a data row - # whose first value is WNS and third is WHS. +def _write_timing_report(path: Path, wns_ns: float, whs_ns: float = 0.100) -> None: + # The full column set Vivado emits, reproduced verbatim from a V80 static + # shell run. The endpoint-count columns interleaved between the slacks are + # the point: an abridged WNS/TNS/WHS/THS table puts WHS at offset 2 and lets + # an offset-based parser look correct while reading the wrong column off a + # real report. path.write_text( textwrap.dedent( f"""\ - Design Timing Summary - --------------------- + | Design Timing Summary + | --------------------- - WNS(ns) TNS(ns) WHS(ns) THS(ns) - ------- ------- ------- ------- - {wns_ns:.3f} 0.000 0.100 0.000 + WNS(ns) TNS(ns) TNS Failing Endpoints TNS Total Endpoints WHS(ns) THS(ns) THS Failing Endpoints THS Total Endpoints WPWS(ns) TPWS(ns) TPWS Failing Endpoints TPWS Total Endpoints + ------- ------- --------------------- ------------------- ------- ------- --------------------- ------------------- -------- -------- ---------------------- -------------------- + {wns_ns:.3f} -31.539 2049 1704696 {whs_ns:.3f} 0.000 0 1700856 0.000 0.000 0 520312 """ ), encoding="utf-8", @@ -146,3 +150,37 @@ def test_met_target_does_not_warn(tmp_path, capsys): ) assert "target not met" not in capsys.readouterr().err + + +def test_slacks_come_from_the_named_columns(tmp_path): + # WHS must be read from the WHS(ns) column, not from a fixed offset. On a + # real report offset 2 is "TNS Failing Endpoints", so an offset-based parser + # reports the failing-endpoint count as the hold slack. + report = tmp_path / "report_timing_proj.txt" + _write_timing_report(report, wns_ns=-0.030, whs_ns=0.000) + + slacks = timing_freq.extract_design_timing_slacks_ns( + report.read_text(encoding="utf-8")) + + assert slacks == (-0.030, 0.000) + + +def test_hold_violation_fails_the_gate(tmp_path): + # A design that meets setup but violates hold must not pass. This is the + # case the offset bug hid: the count it returned instead of WHS is never + # negative, so the whs >= 0 check could never fire. + build_dir = tmp_path + report = build_dir / "report_timing_proj.txt" + _write_timing_report(report, wns_ns=0.500, whs_ns=-0.012) + + assert timing_freq.extract_design_timing_slacks_ns( + report.read_text(encoding="utf-8")) == (0.500, -0.012) + assert not timing_freq.design_timing_met(0.500, -0.012) + + with pytest.raises(RuntimeError, match="timing failed"): + timing_freq.require_static_shell_timing_or_confirm( + build_dir=build_dir, + project_name="proj", + ignore_failure=False, + noninteractive=True, + )