vtr-verilog-to-routing icon indicating copy to clipboard operation
vtr-verilog-to-routing copied to clipboard

Router exit condition

Open rfungquicklogic opened this issue 5 years ago • 3 comments

#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)
  • [x] New feature (change which adds functionality)
  • [x] 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
  • [x] All new and existing tests passed

rfungquicklogic avatar Apr 22 '19 15:04 rfungquicklogic

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).

kmurray avatar Apr 25 '19 19:04 kmurray

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?

rfungquicklogic avatar Apr 26 '19 15:04 rfungquicklogic

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

mithro avatar Jun 03 '21 21:06 mithro