Skip to content

Timing-driven placer bug when a block is not connectable #512

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
angl-dev opened this issue Mar 21, 2019 · 6 comments
Closed

Timing-driven placer bug when a block is not connectable #512

angl-dev opened this issue Mar 21, 2019 · 6 comments
Labels
bug Incorrect behaviour VPR VPR FPGA Placement & Routing Tool

Comments

@angl-dev
Copy link
Contributor

Context

The IO block type in the architecture has capacity = 2. The first IO block at (0, 0) is reserved for global input clk, and --fix_pins is used to force placing clk at this block location. To further constrain VPR, custom rr_graph.xml is used, in which the pins of the first IO block at (0, 0) are not connected to any routing segments. The second IO block is still connectable through routing segments in the custom rr_graph.xml. In addition, two types of wire segments exist in the architecture, one with length 1, the other with length 2.

Expected Behaviour

VPR should be able to place and route a design onto this architecture given the correct io.pads file for --fix_pins.

Current Behaviour

VPR fails at initial placement step, when trying to estimate the delay to route a net from the second IO block at (0, 0), because the estimated delay for delta_x=width-2, delta_y=0 is inf. This is because when initializing place/timing_place_lookup.cpp: f_delta_delay, the first output pin of the first IO block is used to estimate the inter-block routing delay, which is inf because the pin is not connected to any wire segments. Delays with smaller delta_x are fixed later, but delta_x=width-2 remains inf.

Possible Solution

In function place/timing_place_lookup.cpp: route_connection_delay, if the pin returned by get_base_class cannot be successfully routed, try another one.

Steps to Reproduce

vpr arch.vpr.xml bcd2bin.blif --net_file bcd2bin.net --place --route --route_chan_width 24 --read_rr_graph rrg.vpr.xml --fix_pins io.pads
See attachments for the files.
files.tar.gz

Your Environment

  • VTR revision used: 8.0.0-dev+0b0ef1b-dirty
  • Operating System and version: Springdale Linux 7.6 (Verona)
  • Compiler version: gcc [7.3.1]
@kmurray
Copy link
Contributor

kmurray commented Mar 22, 2019

Thanks for the report!

Since 0b0ef1b there have been some enhancements to the placer delay lookup to make it more robust. With the latest master code I don't run into the placement failure caused by an inf delta distance (a reasonable value is found instead).

I'll look into the .pads issue. But a temporary work around may be to introduce a unique netlist primtive to represent the clock input and map that to a dedicated block type (e.g. a clock inpad).

@kmurray kmurray added bug Incorrect behaviour VPR VPR FPGA Placement & Routing Tool labels Mar 22, 2019
@kmurray
Copy link
Contributor

kmurray commented Mar 22, 2019

OK I've tracked down the issue.

At a high-level, your clock input got mapped into the wrong block type (IO_TOP) when the location specified in the .pads file is an IO_LEFT.

Since these are different block types the placement consistency check complains:

Error 1: Block 53 type (IO_TOP) does not match grid location (0,1) type (IO_LEFT).
# Placement took 0.06 seconds (max_rss 100.9 MiB, delta_rss +0.0 MiB)
Error 2: 
Type: Placement
File: /project/trees/vtr/vpr/src/place/place.cpp
Line: 3291
Message: 
Completed placement consistency check, 1 errors found.
Aborting program.

This is a known limitation, the pad file only effects placement, not packing -- see #268 (and #349).

Short Term Workaround
Define a single IO type for your architecture, instead of using unique types for each side.
This will ensure that the clock input ends up in a valid IO type which can be placed at your special location.

(Alternately you could manually edit the .net file to ensure the clk was an IO_LEFT.)

Long Term Fix
We plan to eventually add support for equivalent placement sites, which would fix this issue.
For example, the packer would pack to a generic IO type which could then be l placed at any IO_LEFT/IO_RIGHT/IO_TOP/IO_BOTTOM.

@mithro
Copy link
Contributor

mithro commented Mar 22, 2019

We are working on the equivalent placement sites issue and have a document about it here. It is like this work will be done over the next couple of weeks.

@angl-dev
Copy link
Contributor Author

angl-dev commented Mar 23, 2019

@kmurray thanks for looking into it. I have workarounds, so this is not a big issue to me right now. I just found this and I think it's better to report to you. Also as you said, I'm using a version that's a bit old.

In addition, I think I attached the wrong bcd2bin.net file. Sorry for the confusion. I have a script which edits the .net file to make the packing result consistent with the pin assignments. If you look into the file I attached, different types of IO blocks are used. The file I attached was the wrong one which I tried to assign clk to IO block (6, 7, 0) and it worked with the corresponding io.pads file. You might want to manually change the block type of clk in the .net file to IO_LEFT and you may see the same problem as I encountered.

On the "dedicated clock block type" topic, I would suggest that non-clock global wires are treated similarly if generated clock, global routing wires and global buffers are to be supported in the future.

@kmurray
Copy link
Contributor

kmurray commented Mar 25, 2019

You might want to manually change the block type of clk in the .net file to IO_LEFT and you may see the same problem as I encountered.

I tried that and everything worked for me with 0ef9c1c.

On the "dedicated clock block type" topic, I would suggest that non-clock global wires are treated similarly if generated clock, global routing wires and global buffers are to be supported in the future.

The way we model this is to just embed the appropriate edges/nodes into the RR graph (potentially with non-configurable switches where appropriate) to model the global routing network. #405 added support for defining clock networks in the architecture file and VPR generated RR graphs.

While that is functionally correct with the router as-is (if clocks are set to be routed) at the moment, it may generate sub-optimal clock trees (e.g. split between the dedicated and local routing). @mustafabbas is working to enhance the router so it understands how to use these types of global networks appropriately.

@kmurray
Copy link
Contributor

kmurray commented Dec 11, 2019

Long Term Fix
We plan to eventually add support for equivalent placement sites, which would fix this issue.
For example, the packer would pack to a generic IO type which could then be l placed at any IO_LEFT/IO_RIGHT/IO_TOP/IO_BOTTOM.

This was added with #988, so closing this issue.

@kmurray kmurray closed this as completed Dec 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behaviour VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

No branches or pull requests

3 participants