Skip to content

Router exit condition #542

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rfungquicklogic
Copy link
Contributor

#define guarded change to modify router loop exit behavior (not enabled by default). Instead of exiting at max_router_iterations, if IGNORE_ITERATION_LIMIT_HEAP_PUSH_FACTOR is defined, the router will continue to run iterations if the number of heap pushes per iteration is less than the average number of heap pushes per iteration by the factor specified.

Motivation and Context

With a hard iteration limit, sometimes the router aborts even though it appears to be converging because it is down to only a few overused nodes. When the number of heap pushes per iteration is small, each iteration runs quickly relative to earlier iterations, so it may be advantageous to let the router continue in the hopes that with a little extra time, the router will converge.

This change is guarded so that further evaluation can be done (over a period of time) in the context of other VTR experiments. If the change is generally helpful, then we might want to enable it by default -- though finding a clean way to do it might require some thought as simply ignoring max_router_iterations can be confusing.

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

Sorry, something went wrong.

@probot-autolabeler probot-autolabeler bot added lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool labels Apr 22, 2019
@kmurray
Copy link
Contributor

kmurray commented Apr 25, 2019

The code looks good to me.

The next step is probably to evaluate the impact of enabling this option.

One potential issue with controlling this by a define is that it will not be compiled/tested, and hence is at a high risk of breaking in the future.
One way to address this would be to control it by a command-line option (which would be disabled by default until you've had time to further develop/evaluate its impact).

@rfungquicklogic
Copy link
Contributor Author

My intent was to pass this on to someone doing the next round of router development or the next round of architecture experiments that will stress test the router. This is an option they can try out, if they think it will help, in the context of other development. I don't plan to do additional VTR development, and the test cases I can run on my end are quite limited.

My opinion would be leaving it as a #define with the risk that it would break is fine, as it isn't a lot of code, and it is just meant to act as a reference. When someone is looking to try it, it shouldn't be too hard to fix the code if something broke.

Does this seem reasonable?

@mithro
Copy link
Contributor

mithro commented Jun 3, 2021

@rfungquicklogic -- Do you want to rebase this change onto master and see if we can get it merged?

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.

None yet

4 participants