YosysHQ / arachne-pnr

Place and route tool for FPGAs
MIT License
413 stars 73 forks source link

Check that boundary nets are connected to a physical pin and nothing else. #118

Closed tomverbeure closed 5 years ago

tomverbeure commented 6 years ago

The 'boundary_nets' method uses 'connection_other_port' to check if a net is connected to a physical pin: https://github.com/cseed/arachne-pnr/blob/5d830dd94ad956d17d77168fe7718f22f8b55b33/src/netlist.cc#L480

However, this method assumes that there are a maximum of 2 ports connected to the net.

When a netlist is given that violates this assumption, the net is not listed as a boundary net which will result in internal check failures later on.

This commit adds a 'check_boundary_nets' method that ensures that boundary nets meet the expectations. If they do not, it fails gracefully.

This commit fixes both issue #77 and #100.

Tom

daveshah1 commented 6 years ago

Hi Tom

Thanks for submitting the PR! Please note that we now have fixed the CI on arachne-pnr and are requiring it to pass for pull requests. In this case, it is failing with GCC because of a compiler warning.

src/netlist.cc: In member function ‘bool Model::is_physical_port(Models&, const Port*) const’:
src/netlist.cc:472:8: error: declaration of ‘is_physical_port’ shadows a member of 'this' [-Werror=shadow]
   bool is_physical_port = p
tomverbeure commented 6 years ago

Done. Looks like my g++ version (5.4.0 that comes with Ubuntu 16.0.4) isn't as picky as the one that Travis is using.

Tom

microbuilder commented 5 years ago

This has already been merged in but appears to have introduced issues with tristate buffers, see:

For example: fatal error: Top level port 'sda' assigned to an IO pad 'Y' and internal nodes