Fix incorrect IO tile corner detection in nextpnr-aegis#13
Open
hobostay wants to merge 1 commit intoMidstallSoftware:masterfrom
Open
Fix incorrect IO tile corner detection in nextpnr-aegis#13hobostay wants to merge 1 commit intoMidstallSoftware:masterfrom
hobostay wants to merge 1 commit intoMidstallSoftware:masterfrom
Conversation
The previous corner detection used `x != y` / `x == y` which only excluded the (0,0) and (W-1,H-1) diagonal corners. The corners (W-1,0) and (0,H-1) were incorrectly treated as valid IO tiles, causing IO BELs, wires, and pips to be created at positions that don't correspond to actual IO pads in the FPGA hardware. This mismatch between the nextpnr model and the simulator/hardware could cause the placer to assign IO cells to non-existent pad positions, producing a bitstream that doesn't work on the device. Fix by adding an is_corner() helper that correctly identifies all four grid corners, and use it in init_wires, init_bels, and init_pips. Co-Authored-By: Claude Opus 4.6 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The corner detection logic in
nextpnr-aegis/aegis.ccusedx != y/x == yto identify which IO tiles should have wires, BELs, and pips. This condition only excludes the diagonal corners(0,0)and(W-1,H-1), while incorrectly treating corners(W-1,0)and(0,H-1)as valid IO tiles.Bug
For a grid of W×H (including IO ring), the four corners are:
(0, 0)— excluded byx == y✓(W-1, 0)—W-1 != 0, so incorrectly gets IO resources ✗(0, H-1)—0 != H-1, so incorrectly gets IO resources ✗(W-1, H-1)— excluded byx == y✓This causes the nextpnr model to create IO BELs at two corners that don't correspond to actual IO pads in the FPGA hardware (as modeled by the Rust simulator). The placer could then assign IO cells to these non-existent pad positions, producing a bitstream that doesn't work on the device.
Fix
Add an
is_corner()helper that correctly checks all four grid corners:Replace the three occurrences of the incorrect condition in
init_wires,init_bels, andinit_pips.Test plan
io_pad_posconstruction🤖 Generated with Claude Code