Add backward routing for RapidWright.#1346
Add backward routing for RapidWright.#1346kylepham-amd wants to merge 7 commits intoXilinx:masterfrom
Conversation
* This backward routing will route a connection from its sink node to its source node instead of routing from its source node to its sink node. To explore and expand the next node, this backward routing will find the current node's parents using its uphill nodes. A test case has been added in order to validate a single connection. * Codes in this change were generated by Github Copilot with AI agent mode using Gemini 3 Pro model and was guided by me. Prompts are as follows: + To generate the backward routing functions: This current RWRoute implements a forward routing from a source node to a sink node, utilizing downhill nodes. Write me an alternative backward routing, which routes from a sink node to its corresponding source node and utilizises uphill nodes. + To generate a test: Write a test case to test the backward routing algorithm with the same source and sink as the current testSingleConnection. + The test was failed many times and the AI agent was struggling to figure out the root cause for a couple of days. Particularly, an assertion in the getBaseCost function kept failing. It turned out that because the router routed a connection from sink to source, the source node was also passed into getBaseCost, and that failed the assertion. A check was applied to the router to not pass the source node to that getBaseCost function. + Another issue in the generated finishRouteConnectionBackward was that the nodes from the backward search, which naturally produces Source -> ... -> Sink, were unmatched RapidWright's Connection convention, which is Sink -> ... -> Source. That caused another assertion failed. After reverse the list in that finishRouteConnectionBackward, the test has passed. Signed-off-by: Kyle Pham <kyle.pham@amd.com>
eddieh-xlnx
left a comment
There was a problem hiding this comment.
Excellent first go at the problem @kylepham-amd, thanks! I've made an overarching suggestion to improve this PR, after which it'd be much easier to understand and I can pick it at it some more, as well as battle-test/quantify it on more complicated designs than those with just a single short connection.
| PartialRouter router = new PartialRouter(design, config, pinsToRoute, softPreserve) { | ||
| @Override | ||
| protected void routeIndirectConnection(Connection connection) { | ||
| routeIndirectConnectionBackward(connection); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Oh my word! This is such an elegant way of doing things!
* Add a new field into Connection class to indicate which routing direction is: forward (Source->...->Sink) or backward (Sink->...->Source). * Use that new field to merge forward and backward routing methods together to reduce replications. * Update RWRouteConfig, PartialRouter to hook the backward router to the high-level router. * Modify tests to verify the new changes. The test for backward routing has passed. Signed-off-by: Kyle Pham <kyle.pham@amd.com>
|
Hi @eddieh-xlnx, |
Signed-off-by: Eddie Hung <eddie.hung@amd.com>
Signed-off-by: Eddie Hung <eddie.hung@amd.com>
Signed-off-by: Eddie Hung <eddie.hung@amd.com>
Signed-off-by: Eddie Hung <eddie.hung@amd.com>
Signed-off-by: Eddie Hung <eddie.hung@amd.com>
|
I've cleaned up this PR to minimize the changes against original RWRoute (in turn maximizing reuse of existing code paths) -- for example, swapping "source" and "sink" nodes and reversing children to mean uphill when doing backward routing. Currently, it continues to pass the one explicitly-backward test case. One thing we should focus on is that backward routing does not appear to be close in terms of efficiency. Forward routing on the same test case requires just 70 pops, but backward routing requires 341 pops. I'd like to see this number go down before we go any further. I suggest taking a look at what nodes are being popped from the queue, alongside the review comments above (more broadly, revisiting all the |
| case NODE_GLOBAL_LEAF: | ||
| case NODE_INT_INTERFACE: | ||
| case NODE_DEDICATED: | ||
| case NODE_OPTDELAY: |
There was a problem hiding this comment.
We should be excluding these nodes from the routing graph entirely (they weren't needed during forward routing)
| (connection.isCrossSLRsouth() && childType == RouteNodeType.NON_LOCAL_LEADING_TO_SOUTHBOUND_LAGUNA))) { | ||
| // Only lookahead beyond child nodes leading to a Laguna if we require an SLR crossing in that direction, | ||
| // and it won't get overused | ||
| if (!config.isBackwardRouting()) { |
There was a problem hiding this comment.
What you've done here is eliminate this optimization from being applied during backward routing entirely. Let's leave a TODO behind to say that this should be revisited in the future.
@eddieh-xlnx
This backward routing will route a connection from its sink node to its source node instead of routing from its source node to its sink node. To explore and expand the next node, this backward routing will find the current node's parents using its uphill nodes. A test case has been added in order to validate a single connection.
Codes in this change were generated by GitHub Copilot with AI agent mode using Gemini 3 Pro model and was guided by me. Prompts are as follows:
This current RWRoute implements a forward routing from a source node to a sink node, utilizing downhill nodes. Write me an alternative backward routing, which routes from a sink node to its corresponding source node and utilizises uphill nodes.
Write a test case to test the backward routing algorithm with the same source and sink as the current testSingleConnection.