Add constant network support to FPGA interchange arch #591

litghost · 2021-02-19T01:00:50Z

This adds initial constant network support (along side changes in chipsalliance/python-fpga-interchange#23). I believe logic needs to be added to combine GND/VCC cells/nets, but this doesn't appear to matter with the initial const wire designs. Once I have an example netlist with multiple GND/VCC cells/nets, I'll add the remaining logic.

gatecat · 2021-02-19T10:09:58Z

Approach here looks good. Some more work might be needed in the future to improve performance of constant routing where you have a single wire with a very large number of pips fanning off it; the backwards routing heuristic in router2 works well for this but doesn't currently deal with congestion which will fall back to regular forwards routing which will have to iterate over a lot of pips.

This was more of a problem for xcup rather than xc7 in my nextpnr-xilinx experience, due to GND constants needing to come from a nearby LUT creating more possibilities for congestion (also xcup devices just being larger). One thing I did was to have a three level "global->row->tile" type structure for constant wires so the router could make targeted progress; but another solution would be to special-case this in the router, for example always using backwards routing for constants and extending the backwards router to work around congestion.

litghost · 2021-02-19T15:45:37Z

the backwards routing heuristic in router2 works well for this but doesn't currently deal with congestion which will fall back to regular forwards routing which will have to iterate over a lot of pips.

My plan is to have the site router push all constants to the site pins, which will address this explicitly. Because the site router ensures a congestion free site routing, and dedicated site constant sources are preferred over routed constant sources, and because of site inverter <-> constant net relationships, handling this in the site router should avoid any congestion.

litghost · 2021-02-19T15:47:38Z

This was more of a problem for xcup rather than xc7 in my nextpnr-xilinx experience, due to GND constants needing to come from a nearby LUT creating more possibilities for congestion (also xcup devices just being larger). One thing I did was to have a three level "global->row->tile" type structure for constant wires so the router could make targeted progress; but another solution would be to special-case this in the router, for example always using backwards routing for constants and extending the backwards router to work around congestion.

Ya, this might cause some problems. The current constant network does not have pseudo site pips to the LUT outputs, though it needs them for US. xc7 graphs are easier in this regard. I think this is a case of "crawl", "walk", "run", e.g. fix it once we can demostrate the issue.

Overall I think the constant network structure is transparent to the arch, and can be tweaked as needed as problems arise.

litghost · 2021-02-23T22:04:13Z

@gatecat This is ready for review when you get a chance. I've gotten a counter design mostly working, but I need to fix LUT rotation / port sharing before that is ready.

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Also add debug_test target to debug archcheck. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Fixes: - Only use map constant pins during routing, and not during placement. - Unmapped cell ports have no BEL pins. - Fix SiteRouter congestion not taking into account initial expansion. - Fix psuedo-site pip output. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

gatecat · 2021-02-24T11:15:07Z

fpga_interchange/arch.cc

+    delay_t base = 30 * std::min(std::abs(dst_x - src_x), 18) + 10 * std::max(std::abs(dst_x - src_x) - 18, 0) +
+                   60 * std::min(std::abs(dst_y - src_y), 6) + 20 * std::max(std::abs(dst_y - src_y) - 6, 0) + 300;
+
+    base = (base * 3) / 2;


for this lookahead to be more effective, I would also suggest returning a pip delay on the order of 100ps

gatecat · 2021-02-24T11:19:00Z

fpga_interchange/dedicated_interconnect.cc

+                case IN_ROUTING:
+                    NPNR_ASSERT(wire_data.site != -1);
+                    if (wire.tile == src_wire.tile && wire_data.site == src_wire_data.site) {
+                        // Dedicated routing won't have straight loops,


Are we sure this is a universal truth? It does seem like a good heuristic, but I'm wondering if there are places where combinations of routethroughs/loopbacks might violate this.

I'd need to see an example of where a straight loop forms a dedicated interconnect (e.g. placement dependent). All the cases I know are site to another site. Do you know of any counter examples?

The good news is if this logic fails on an arch, it will manifest eventually as a routing failure, which will generate a bug report, rather than an optimization failure.

gatecat · 2021-02-24T11:20:42Z

fpga_interchange/dedicated_interconnect.cc

+                // Do detailed routing check to ensure driver can reach sink.
+                //
+                // FIXME: This might be too slow, but it handles a case on
+                // SLICEL.COUT -> SLICEL.CIN has delta_y = {1, 2}, but the


This is going to be a problem when we start inserting relative placement constraints (which should give much better performance than essentially relying on trial and error) as these require fixed dx/dy values. nextpnr-xilinx punted on this but I think a way of specifying the "next" bel/site might make sense as a future API improvement.

Agree that relative placement constraints should be emitted where possible. Something that is tricky here is that the use of dedicated interconnect is not 1-to-1 with placement constraints. Carry chains and DSP chains are good candidates for relative placement constraints, but the IFF/OFF cases are a good example where it should be up to the placer to determine if it wants to use the IFF/OFF. It might make more sense to use a SLICEL/M FF over an IO FF depending on the shape and fanout of the signals. Just because an IO FF is possible doesn't mean we should constraint the solution to that.

Yes, this is an interesting thing to think about. Somewhat related is the dedicated LUT->FF routing which you want to use if you can; but definitely don't want to force all the time.

One idea I had in Nexus was a separate post-place pass that performs low-distance swaps only with the goal of using this dedicated interconnect (currently only LUT-FF, I've been meaning to look at doing similar with the dedicated fast LUT-LUT pattern that Nexus has too): https://github.com/YosysHQ/nextpnr/blob/master/nexus/post_place.cc

A generic version of that pass as a post-place optimisation to opportunistically exploit dedicated routing, by swapping things within a site or between adjacent sites, could definitely improve things from a routeability/timing point of view down the line.

Yes, this is an interesting thing to think about. Somewhat related is the dedicated LUT->FF routing which you want to use if you can; but definitely don't want to force all the time.

Yep, and same story with LUT -> CARRY and CARRY -> FF placement. In general, there needs to be a way to align site local placement to exploit specialized site routing to achieve maximal packing density. Once I debug the FPGA arch site router for correctness, I believe this is going to be an important improvement during HeAP legalization to achieve really good results.

gatecat · 2021-02-24T11:22:00Z

fpga_interchange/dedicated_interconnect.cc

+            continue;
+        }
+
+        // This net doesn't have a driver, probably not valid?


Undriven nets aren't necessarily a problem (for example if you connect a net to an input but don't drive it); in other arches we tend to skip over them.

I believe you mean to an output, but don't drive it. A disconnected cell primitive input is a bug in most cases I can think of. It must be either 1/0/signal, rather than "unknown".

gatecat · 2021-02-24T11:23:18Z

fpga_interchange/dedicated_interconnect.cc

+            continue;
+        }
+
+        for (size_t i = 0; i < bel_data.num_bel_wires; ++i) {


mixed signedness compare warning (I'm minded to tighten up the compiler flags so more of these things become CI failures, not sure what your opinion is on that)

Please tighten up the flags. We also should emit -Wxxx on local builds. I'm using clang and it is capable of emitting the same warnings as the CI, but that isn't the current default (I believe).

gatecat · 2021-02-24T11:25:01Z

fpga_interchange/fpga_interchange.cpp

+        auto downhill_iter = pip_downhill.find(wire);
+        if(downhill_iter == pip_downhill.end()) {
+            if(root_wire != wire) {
+                log_warning("Wire %s never entered the real fabric?\n",


should this be a warning or an error?

This is a warning right now because the constant merging logic always emits the VCC/GND root, even if unused. Then when walking that root, it won't enter the real fabric. In the long term we can prune away an unused VCC/GND root and make this an error again.

That's fair as a temporary thing; from an end-user point of view I don't much like the idea of a warning that isn't immediately end-user-actionable but I imagine it will be long gone by the time this is end-user-read.

gatecat · 2021-02-24T11:55:25Z

my attempt to build the bba locally is failing with:

capnp.lib.capnp.KjException: home/david/nextpnr-master/fpga_interchange/examples/create_bba/build/fpga-interchange-schema/interchange/DeviceResources.capnp:546: failed: Union must have at least two members.

I think this is related to the LUT init stuff based on the line number; and checking out the fpga-interchange-schema commit before that PR was merged seems to work OK.

litghost · 2021-02-24T15:46:09Z

my attempt to build the bba locally is failing with:
capnp.lib.capnp.KjException: home/david/nextpnr-master/fpga_interchange/examples/create_bba/build/fpga-interchange-schema/interchange/DeviceResources.capnp:546: failed: Union must have at least two members.
I think this is related to the LUT init stuff based on the line number; and checking out the fpga-interchange-schema commit before that PR was merged seems to work OK.

Ya, I found this too and need to fix it. Need to stand up a CI on fpga-interchange-schema to catch this kind of stuff.

litghost · 2021-02-24T19:05:48Z

my attempt to build the bba locally is failing with:
capnp.lib.capnp.KjException: home/david/nextpnr-master/fpga_interchange/examples/create_bba/build/fpga-interchange-schema/interchange/DeviceResources.capnp:546: failed: Union must have at least two members.
I think this is related to the LUT init stuff based on the line number; and checking out the fpga-interchange-schema commit before that PR was merged seems to work OK.

I've fixed the bug and stood up a simple CI to find these issues with chipsalliance/fpga-interchange-schema#13

litghost changed the title ~~Add constant network~~ Add constant network support to FPGA interchange arch Feb 19, 2021

litghost force-pushed the add_constant_network branch 4 times, most recently from 94a0495 to cd6d05d Compare February 23, 2021 21:58

litghost added 15 commits February 23, 2021 14:08

Change CellInfo in getBelPinsForCellPin to be const.

423a10b

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Update archapi.md with latest signature.

0758f68

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Add initial constant network support to FPGA interchange arch.

40df4f4

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Correct some bugs in the create_bba Makefile.

761d9d9

Also add debug_test target to debug archcheck. Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Add tests to confirm constant routing import.

3e5a23e

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Add constant network test case.

cf554f9

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Initial working constant network support!

15459ca

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Run "make clangformat".

3ccb164

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Fix reference copy.

46b38f8

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Remove some signedness warnings.

5c6e231

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Move RapidWright git URI back to upstream.

cd8297f

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Add initial logic for handling dedicated interconnect situations.

2fc353d

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Working FF example now that constant merging is done.

5574455

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

Finish dedicated interconnect implementation.

1846656

Signed-off-by: Keith Rothman <537074+litghost@users.noreply.github.com>

litghost force-pushed the add_constant_network branch from cd6d05d to a30043c Compare February 23, 2021 22:09

gatecat reviewed Feb 24, 2021

View reviewed changes

gatecat merged commit ab8dfcf into YosysHQ:master Feb 25, 2021

litghost deleted the add_constant_network branch February 25, 2021 16:54

tcal-x mentioned this pull request Oct 26, 2021

High-fanout nets in HPS design google/CFU-Playground#331

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add constant network support to FPGA interchange arch #591

Add constant network support to FPGA interchange arch #591

litghost commented Feb 19, 2021 •

edited

gatecat commented Feb 19, 2021

litghost commented Feb 19, 2021

litghost commented Feb 19, 2021 •

edited

litghost commented Feb 23, 2021

gatecat Feb 24, 2021

litghost Feb 24, 2021

gatecat Feb 24, 2021

litghost Feb 24, 2021

gatecat Feb 24, 2021

litghost Feb 24, 2021 •

edited

gatecat Feb 24, 2021

litghost Feb 24, 2021

gatecat Feb 24, 2021

litghost Feb 24, 2021

gatecat Feb 24, 2021

litghost Feb 24, 2021

gatecat Feb 24, 2021

litghost Feb 24, 2021

gatecat Feb 24, 2021

gatecat commented Feb 24, 2021 •

edited

litghost commented Feb 24, 2021

litghost commented Feb 24, 2021

Add constant network support to FPGA interchange arch #591

Add constant network support to FPGA interchange arch #591

Conversation

litghost commented Feb 19, 2021 • edited

gatecat commented Feb 19, 2021

litghost commented Feb 19, 2021

litghost commented Feb 19, 2021 • edited

litghost commented Feb 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

litghost Feb 24, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gatecat commented Feb 24, 2021 • edited

litghost commented Feb 24, 2021

litghost commented Feb 24, 2021

litghost commented Feb 19, 2021 •

edited

litghost commented Feb 19, 2021 •

edited

litghost Feb 24, 2021 •

edited

gatecat commented Feb 24, 2021 •

edited