From 04e9be668ba8ea0ab36a6c3e702679f16e6135c6 Mon Sep 17 00:00:00 2001 From: Kelvin Chung Date: Tue, 9 Jun 2026 21:48:24 +0100 Subject: [PATCH 1/3] feat(pathgraph): add traverse_registers option to TimingGraph SDF models a register's clock-to-output arc as an IOPATH only, so a pure delay graph dead-ends at every flop data pin. With traverse_registers=True, each SETUP/SETUPHOLD timing check adds a data-to-clock edge carrying the check's setup time, modelling the forward cost of crossing the register boundary (data stable setup-before-clock, then propagation continues through the clock-to-output IOPATH). HOLD checks add no edge: hold is a minimum-arrival constraint with typically negative values that must not enter path delay sums. --- src/sdf_toolkit/core/pathgraph.py | 95 ++++++++++++++++++++++-- tests/test_pathgraph.py | 116 +++++++++++++++++++++++++++++- 2 files changed, 203 insertions(+), 8 deletions(-) diff --git a/src/sdf_toolkit/core/pathgraph.py b/src/sdf_toolkit/core/pathgraph.py index f4e2564..a634bc3 100644 --- a/src/sdf_toolkit/core/pathgraph.py +++ b/src/sdf_toolkit/core/pathgraph.py @@ -11,7 +11,16 @@ import networkx as nx -from sdf_toolkit.core.model import DelayField, DelayFieldLike, DelayMetric, DelayMetricLike, DelayPaths, EntryType, SDFFile +from sdf_toolkit.core.model import ( + BaseEntry, + DelayField, + DelayFieldLike, + DelayMetric, + DelayMetricLike, + DelayPaths, + EntryType, + SDFFile, +) @dataclass(frozen=True) @@ -152,30 +161,48 @@ class TimingGraph: ---------- sdf : SDFFile The parsed SDF file to build the graph from. + traverse_registers : bool, optional + When True, every SETUP/SETUPHOLD timing check additionally adds a + data-pin to clock-pin edge whose delay is the check's setup time. + Sequential cells then become traversable: a path can continue + from a register's data input through its clock pin and on through + the cell's clock-to-output IOPATH. By default False, which keeps + the graph a pure delay graph where registers end timing paths. """ - def __init__(self, sdf: SDFFile) -> None: + def __init__(self, sdf: SDFFile, traverse_registers: bool = False) -> None: self._graph: nx.MultiDiGraph = nx.MultiDiGraph() - self._build(sdf) + self._build(sdf, traverse_registers) - def _build(self, sdf: SDFFile) -> None: + def _build(self, sdf: SDFFile, traverse_registers: bool) -> None: """Populate the graph from SDF cells. For each cell in the SDF file, IOPATH and INTERCONNECT entries are converted to directed edges. IOPATH pin names are qualified with the instance hierarchy path, while INTERCONNECT pin names - are used as-is (they are already fully qualified). + are used as-is (they are already fully qualified). With + ``traverse_registers``, SETUP/SETUPHOLD checks are converted to + data-to-clock edges as well. Parameters ---------- sdf : SDFFile The parsed SDF file. + traverse_registers : bool + Whether to add data-to-clock edges for setup-style checks. """ divider = sdf.header.divider or "/" for cell_type, instances in sdf.cells.items(): for instance, entries in instances.items(): for _entry_name, entry in entries.items(): + if traverse_registers and entry.type in ( + EntryType.SETUP, + EntryType.SETUPHOLD, + ): + self._add_register_edge(entry, cell_type, instance, divider) + continue + if entry.type not in (EntryType.IOPATH, EntryType.INTERCONNECT): continue @@ -201,6 +228,64 @@ def _build(self, sdf: SDFFile) -> None: instance=instance, ) + def _add_register_edge( + self, + entry: "BaseEntry", + cell_type: str, + instance: str, + divider: str, + ) -> None: + """Add a data-to-clock edge for a setup-style timing check. + + Timing checks are clock-relative: ``from_pin`` is the clock pin + and ``to_pin`` the data pin. The edge runs data to clock and its + delay is the setup time, the physically meaningful forward cost + of crossing the register boundary (data must be stable that long + before the clock edge; propagation then continues through the + cell's clock-to-output IOPATH). HOLD checks add no edge: hold is + a minimum-arrival constraint, not a propagation cost, and its + typically negative values would corrupt path delay sums. + + SDF allows several setup checks per pin pair (edge or condition + variants); each becomes its own parallel edge, consistent with + how conditional IOPATH variants are kept as parallel edges. + + Parameters + ---------- + entry : BaseEntry + A SETUP or SETUPHOLD timing check entry. + cell_type : str + The cell type from the SDF file. + instance : str + The instance name from the SDF file. + divider : str + The hierarchy divider character. + """ + if entry.from_pin is None or entry.to_pin is None: + return + if entry.delay_paths is None: + return + + # SETUP stores its value in `nominal`; SETUPHOLD splits into + # `setup`/`hold`, of which only the setup half is a forward cost. + if entry.type == EntryType.SETUP: + setup_values = entry.delay_paths.nominal + else: + setup_values = entry.delay_paths.setup + if setup_values is None: + return + + data_pin = _qualify_pin(instance, entry.to_pin, divider) + clock_pin = _qualify_pin(instance, entry.from_pin, divider) + self._graph.add_edge( + data_pin, + clock_pin, + delay=DelayPaths(nominal=setup_values), + entry_type=entry.type, + cell_type=cell_type, + instance=instance, + ) + @property def graph(self) -> nx.MultiDiGraph: """Expose the underlying NetworkX MultiDiGraph for advanced analysis. diff --git a/tests/test_pathgraph.py b/tests/test_pathgraph.py index a9bbe7a..f2c4df7 100644 --- a/tests/test_pathgraph.py +++ b/tests/test_pathgraph.py @@ -218,7 +218,7 @@ def test_empty_sdf(self) -> None: class TestParallelEdges: """Bug #1: find_paths overcounts when parallel edges exist.""" - @pytest.fixture() + @pytest.fixture def parallel_graph(self) -> TimingGraph: """Graph with two parallel IOPATH edges from A to Y in one cell.""" sdf = ( @@ -285,7 +285,7 @@ def test_batch_endpoint_path_count_with_parallel_edges( class TestParallelEdgesMultiHop: """Parallel edges on multi-hop paths.""" - @pytest.fixture() + @pytest.fixture def multi_hop_parallel_graph(self) -> TimingGraph: """a/Y --(2 edges)--> b/A -> b/Y with 1 edge.""" sdf = ( @@ -314,7 +314,7 @@ def test_multi_hop_parallel_count( class TestNoneScalarSorting: """Bug #3: None scalars should sort last in rank_paths, not first.""" - @pytest.fixture() + @pytest.fixture def mixed_field_graph(self) -> TimingGraph: """Graph where one path yields a scalar and another yields None.""" sdf = ( @@ -448,3 +448,113 @@ def test_source_equals_sink_nonexistent(self, spec1_graph: TimingGraph) -> None: """find_paths with source==sink on nonexistent node should raise.""" with pytest.raises(nx.NodeNotFound): spec1_graph.find_paths("NONEXISTENT", "NONEXISTENT") + + +def _register_sdf() -> SDFFile: + """One flop between two top-level pins: din -> ff/D, ff/Q -> dout.""" + return ( + SDFBuilder() + .set_header(timescale="1ns") + .add_cell("DFF", "ff") + .add_iopath("CLK", "Q", {"nominal": {"min": 0.3, "max": 0.3}}) + .add_interconnect("din", "ff/D", {"nominal": {"min": 0.1, "max": 0.1}}) + .add_interconnect("ff/Q", "dout", {"nominal": {"min": 0.2, "max": 0.2}}) + .add_setup("CLK", "D", {"nominal": {"min": 0.05, "max": 0.05}}) + .add_hold("CLK", "D", {"nominal": {"min": -0.02, "max": -0.02}}) + .build() + ) + + +class TestTraverseRegisters: + """Optional data-to-clock edges for setup-style timing checks.""" + + def test_default_graph_stops_at_register(self) -> None: + """Without the option, registers end timing paths.""" + g = TimingGraph(_register_sdf()) + assert "dout" not in nx.descendants(g.graph, "din") + assert not g.graph.has_edge("ff/D", "ff/CLK") + + def test_register_becomes_traversable(self) -> None: + """The data-to-clock edge lets paths continue through the flop.""" + g = TimingGraph(_register_sdf(), traverse_registers=True) + assert "dout" in nx.descendants(g.graph, "din") + + def test_edge_carries_setup_time(self) -> None: + """The pass-through edge's delay is the setup time.""" + g = TimingGraph(_register_sdf(), traverse_registers=True) + arcs = g.graph.get_edge_data("ff/D", "ff/CLK") + assert len(arcs) == 1 + (arc,) = arcs.values() + assert arc["entry_type"] == EntryType.SETUP + assert arc["delay"].get_scalar("nominal", "max") == 0.05 + + def test_composed_path_includes_setup_and_clk_to_q(self) -> None: + """din -> dout composes interconnects, setup, and CLK->Q IOPATH.""" + g = TimingGraph(_register_sdf(), traverse_registers=True) + paths = g.find_paths("din", "dout") + assert len(paths) == 1 + delay = g.compose_delay(paths[0]) + # 0.1 (din->D) + 0.05 (setup) + 0.3 (CLK->Q) + 0.2 (Q->dout) + assert delay.get_scalar("nominal", "max") == pytest.approx(0.65) + + def test_hold_check_adds_no_edge(self) -> None: + """HOLD is a minimum-arrival constraint, not a propagation cost.""" + sdf = ( + SDFBuilder() + .set_header(timescale="1ns") + .add_cell("DFF", "ff") + .add_hold("CLK", "D", {"nominal": {"min": -0.02, "max": -0.02}}) + .build() + ) + g = TimingGraph(sdf, traverse_registers=True) + assert not g.graph.has_edge("ff/D", "ff/CLK") + + def test_setuphold_uses_setup_half(self) -> None: + """SETUPHOLD contributes its setup value, never the hold value.""" + sdf = ( + SDFBuilder() + .set_header(timescale="1ns") + .add_cell("DFF", "ff") + .add_setuphold( + "CLK", + "D", + { + "setup": {"min": 0.07, "max": 0.07}, + "hold": {"min": -0.03, "max": -0.03}, + }, + ) + .build() + ) + g = TimingGraph(sdf, traverse_registers=True) + arcs = g.graph.get_edge_data("ff/D", "ff/CLK") + assert len(arcs) == 1 + (arc,) = arcs.values() + assert arc["entry_type"] == EntryType.SETUPHOLD + assert arc["delay"].get_scalar("nominal", "max") == 0.07 + assert arc["delay"].get_scalar("hold", "max") is None + + def test_parsed_setup_variants_become_parallel_edges(self) -> None: + """Edge-specific SETUP variants each add their own parallel edge.""" + sdf_text = """(DELAYFILE + (SDFVERSION "3.0") + (DESIGN "top") + (DIVIDER /) + (TIMESCALE 1ns) + (CELL + (CELLTYPE "DFF") + (INSTANCE ff) + (TIMINGCHECK + (SETUP (posedge D) (posedge CLK) (0.11::0.11)) + (SETUP (negedge D) (posedge CLK) (0.13::0.13)) + ) + ) + )""" + from sdf_toolkit.io import parse + + g = TimingGraph(parse(sdf_text), traverse_registers=True) + arcs = g.graph.get_edge_data("ff/D", "ff/CLK") + assert len(arcs) == 2 + scalars = sorted( + arc["delay"].get_scalar("nominal", "max") for arc in arcs.values() + ) + assert scalars == [0.11, 0.13] From fad6cda256e1a10c57113163791aea3ac38363a7 Mon Sep 17 00:00:00 2001 From: Kelvin Chung Date: Tue, 9 Jun 2026 22:20:32 +0100 Subject: [PATCH 2/3] chore: default to build full graph to true --- src/sdf_toolkit/core/pathgraph.py | 6 +++--- tests/test_pathgraph.py | 9 ++++----- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/sdf_toolkit/core/pathgraph.py b/src/sdf_toolkit/core/pathgraph.py index a634bc3..3b22e41 100644 --- a/src/sdf_toolkit/core/pathgraph.py +++ b/src/sdf_toolkit/core/pathgraph.py @@ -166,11 +166,11 @@ class TimingGraph: data-pin to clock-pin edge whose delay is the check's setup time. Sequential cells then become traversable: a path can continue from a register's data input through its clock pin and on through - the cell's clock-to-output IOPATH. By default False, which keeps - the graph a pure delay graph where registers end timing paths. + the cell's clock-to-output IOPATH. By default True, which makes + registers traversable so timing paths may cross sequential cells. """ - def __init__(self, sdf: SDFFile, traverse_registers: bool = False) -> None: + def __init__(self, sdf: SDFFile, traverse_registers: bool = True) -> None: self._graph: nx.MultiDiGraph = nx.MultiDiGraph() self._build(sdf, traverse_registers) diff --git a/tests/test_pathgraph.py b/tests/test_pathgraph.py index f2c4df7..c91ab07 100644 --- a/tests/test_pathgraph.py +++ b/tests/test_pathgraph.py @@ -246,14 +246,13 @@ def test_find_paths_count_with_parallel_edges( paths = parallel_graph.find_paths("b0/A", "b0/Y") assert len(paths) == 2 - def test_find_paths_no_duplicate_delays( - self, parallel_graph: TimingGraph - ) -> None: + def test_find_paths_no_duplicate_delays(self, parallel_graph: TimingGraph) -> None: """Each parallel edge path should have a distinct delay.""" paths = parallel_graph.find_paths("b0/A", "b0/Y") delays = [parallel_graph.compose_delay(p) for p in paths] scalars = sorted( - d.get_scalar("slow", "max") for d in delays # type: ignore[type-var] + d.get_scalar("slow", "max") + for d in delays # type: ignore[type-var] ) assert scalars == [3.0, 6.0] @@ -470,7 +469,7 @@ class TestTraverseRegisters: def test_default_graph_stops_at_register(self) -> None: """Without the option, registers end timing paths.""" - g = TimingGraph(_register_sdf()) + g = TimingGraph(_register_sdf(), traverse_registers=False) assert "dout" not in nx.descendants(g.graph, "din") assert not g.graph.has_edge("ff/D", "ff/CLK") From afe0f1871c0af790f26276957ef54bc3eec23690 Mon Sep 17 00:00:00 2001 From: Kelvin Chung Date: Tue, 9 Jun 2026 22:47:03 +0100 Subject: [PATCH 3/3] chore: fix test --- tests/test_pathgraph.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pathgraph.py b/tests/test_pathgraph.py index c91ab07..175a172 100644 --- a/tests/test_pathgraph.py +++ b/tests/test_pathgraph.py @@ -150,7 +150,7 @@ def test_setup_hold_entries_skipped(self) -> None: .add_iopath("D", "Q", {"nominal": {"min": 1.0, "avg": 1.0, "max": 1.0}}) .build() ) - graph = TimingGraph(sdf) + graph = TimingGraph(sdf, traverse_registers=False) # Only the IOPATH should create edges; SETUP/HOLD are skipped assert len(graph.edges()) == 1