fix(xilinx): DSP48E1 packing crash on xc7 + cluster registration

[agent-maestro]

Summary

Fixes two bugs in xilinx/pack_dsp_xc7.cc that crash nextpnr-xilinx during DSP packing on xc7 parts whenever synth_xilinx leaves DSP48E1 cells in the netlist (i.e. anything other than -nodsp).

Tested with a 16-DSP48E1 design (Q16.16 fixed-point expm1 kernel) on xc7a100tcsg324-1. Before this patch: terminates with std::out_of_range during DSP packing. After: routing completes; post-route fmax 168.12 MHz (PASS at 100 MHz), 414 SLICE_LUTX (vs 7279 with -nodsp).

The two bugs

1. walk_dsp dereferenced users.end() instead of users.begin()

// before — at xilinx/pack_dsp_xc7.cc:34 and again at :48
auto user_port = ni->users.end();
... user_port->cell ...

indexed_store<PortRef>::end() returns an iterator whose index equals slots.size(), and operator* calls slots.at(index). That throws std::out_of_range: __n (which is 1) >= size() (which is 1) the instant any DSP has a cascade net wired up (COUT / PCOUT / ACOUT / BCOUT) — which is every multi-DSP design.

The existing check_illegal_fanout(ni) call right above already guarantees ni->users.entries() == 1, so users.begin() is the correct dereference.

2. pack_dsps set cluster on cascaded children but not on the root

// before — at xilinx/pack_dsp_xc7.cc:155 (roughly)
walk_dsp(root, root, "COUT");
walk_dsp(root, root, "PCOUT");
walk_dsp(root, root, "ACOUT");
walk_dsp(root, root, "BCOUT");
// (root->cluster is never set)

walk_dsp correctly sets child->cluster = root->name and pushes the child onto root->constr_children. But it never sets root->cluster = root->name. HeAPPlacer::HeAPPlacer populates cluster2cells only from cells with cluster != ClusterId(). Result: cascaded DSPs get no cell_locs entry, and total_hpwl() aborts via dict::at() the first time it touches a net user that is a cascaded child.

The xcup variant already does the equivalent (pack_dsp_xcup.cc:69: subcell->cluster = subcell->name;). This patch mirrors that for xc7 by setting root->cluster = root->name whenever walk_dsp actually attached children.

Patch

--- a/xilinx/pack_dsp_xc7.cc
+++ b/xilinx/pack_dsp_xc7.cc
@@ -31,7 +31,7 @@ ...walk_dsp...
-        auto user_port = ni->users.end();
+        auto user_port = ni->users.begin();
@@ -45,7 +45,7 @@ ...walk_dsp...
-        auto user_port = ni->users.end();
+        auto user_port = ni->users.begin();
@@ -152,6 +152,12 @@ ...pack_dsps...
         walk_dsp(root, root, "COUT");
         walk_dsp(root, root, "PCOUT");
         walk_dsp(root, root, "ACOUT");
         walk_dsp(root, root, "BCOUT");
+        if (!root->constr_children.empty()) {
+            root->cluster = root->name;
+        }

8 lines total, no behavioral change for designs without DSP cascades.

Test results

Test design: 16-DSP48E1 expm1_pipeline (Q16.16 fixed-point exp(x)-1), xc7a100tcsg324-1, target 100 MHz.

Metric	DSP run (this PR)	-nodsp baseline
Post-route fmax (clk)	168.12 MHz	79.95 MHz (FAIL @ 100 MHz)
Pre-route fmax (clk)	112.25 MHz (PASS)	57.93 MHz (FAIL)
SLICE_LUTX	414 (0%)	7279 (5%)
SLICE_FFX	258 (0%)	356 (0%)
CARRY4	36	71
DSP48E1	16/240 (6%)	0
FASM bytes	584,209	5,881,004

2.10x fmax, 17.6x LUT reduction.

Test plan

• Build at HEAD (master = 8f178fc); confirm crash reproduces with the test design.
• Apply patch; confirm "Routing complete." and Program finished normally.
• Verify post-route fmax meets 100 MHz target.
• Verify cell count matches expected: 16 DSP48E1, 414 LUTX, 258 FFX.
• (Upstream reviewer): sanity-check on a known-good xc7 DSP design (e.g. picorv32 with multiplier inferred).

🤖 Generated with Claude Code