Skip to content

Fix inverted reachable wire check in SOURCE/OPIN lookahead sampling - #3774

Open
soheilshahrouz wants to merge 15 commits into
masterfrom
lookahead_fix_inverted_reachable_wire_check
Open

Fix inverted reachable wire check in SOURCE/OPIN lookahead sampling#3774
soheilshahrouz wants to merge 15 commits into
masterfrom
lookahead_fix_inverted_reachable_wire_check

Conversation

@soheilshahrouz

Copy link
Copy Markdown
Contributor

compute_router_src_opin_lookahead iterates over all instances of a tile type and recomputes src_opin_lookahead in each iteration. This happens because the if statement that checks whether there is a route from SRC/OPIN to CHANX\CHANY is wrong. After fixing this, compute_router_src_opin_lookahead uses a single instance from each tile type to populate src_opin_lookahead.

@github-actions github-actions Bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Aug 24, 2026
@soheilshahrouz

Copy link
Copy Markdown
Contributor Author

Impact on router lookahead construction for Stratix-IV architecture:

Device width Timer Before (s) After (s) Ratio
100 src/opin lookahead 0.92 0.00 0.000
100 router lookahead map 17.65 16.84 0.954
500 src/opin lookahead 22.56 0.00 0.000
500 router lookahead map 591.68 571.51 0.966

@soheilshahrouz

Copy link
Copy Markdown
Contributor Author

I am investigating why some CI tests failed.

@soheilshahrouz

Copy link
Copy Markdown
Contributor Author

It seems that the segment type reserved for direct connections between tiles is missing from the src_opin_lookahead.

With this chagne, we sample tiles near the borrom left corner. They lack direct connections to tiles below themselves.

@soheilshahrouz

soheilshahrouz commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

I now take a few representative samples for each tile type instead of using samples taken from the bottom left corner.

Impact on router lookahead construction for Stratix-IV architecture:

Device width Timer Before (s) After (s) Ratio
100 src/opin lookahead 0.93 0.03 0.032
100 router lookahead map 16.90 15.80 0.935
200 src/opin lookahead 3.82 0.04 0.011
200 router lookahead map 94.57 86.80 0.918
300 src/opin lookahead 7.93 0.05 0.007
300 router lookahead map 206.65 197.60 0.956

@vaughnbetz vaughnbetz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK with me, but a couple of minor comments embedded.

rr_node_typename[rr_type],
tile_type.name.c_str(),
from_layer_num,
rr_node_arch_name(unreachable_nodes.front(), is_flat).c_str());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we get spurious warnings from this in the default flow when we have carry chains (directly connect OPIN->IPIN) or when edges are missing at the edge of the FPGA? We should avoid issuing warnings for the default flow for normal things like the array having edges cut off.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check is done after we call dijkstra_flood_to_wires() for all sample locations. If only some samples miss an edge, the warning is not triggered.

dijkstra_flood_to_wires() treats OPIN-->IPIN-->SINK as a reachable wire with DIRECT_CONNECT_SPECIAL_SEG_TYPE segment type.

grid.get_width_offset(neighbour_loc),
grid.get_height_offset(neighbour_loc));
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd comment this a bit more. You go through the whole perimeter of the block, recording all its neighbours I think.

I guess if there were switch blocks inside a large block this would miss them? (probably doesn't cause any big problems, but may be worth mentioning in the comment).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments inside the function definition.

@soheilshahrouz

Copy link
Copy Markdown
Contributor Author

@vaughnbetz If you don't have further comments, I think we can merge this.

@vaughnbetz vaughnbetz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AmirhosseinPoolad

Copy link
Copy Markdown
Contributor

Are you sure this gives equivalent QoR? Might be good to do a heap pops comparison.

@soheilshahrouz

Copy link
Copy Markdown
Contributor Author

Are you sure this gives equivalent QoR? Might be good to do a heap pops comparison.

It changes QoR on architectures where OPIN-CHAN connectivity pattern/delay depends on grid location. For Stratix-IV, QoR stays the same because OPIN only connect to L4 whose mux delay is independent of fan-in.

@soheilshahrouz

Copy link
Copy Markdown
Contributor Author

In src_opin_lookahead construction, the RR switch delay of a CHAN node is used, but in wire lookahead we use T_linear which is the average delay across wires of a given segment type. The latter seems more reasonable as the RR switch delay might be lower for wires located at the periphery of the device.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants