Skip to content

Conversation

haydar-c
Copy link
Contributor

@haydar-c haydar-c commented Sep 19, 2025

This PR fixes a segmentation fault when using manual moves in placement and enables using manual moves in the inner annealing loop as well. Previously, manual moves worked only before the inner annealing loop and would segfault, since reward calculation was still invoked even though no move generator was used.

Changes

  • Guard reward calculation when manual_move_enabled is true
  • Add explicit checking of the manual move option inside the inner annealing loop.

Related Issue

#3275

… a move_generator and check the manual moves option in the inner loop as well.
@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Sep 19, 2025
Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks!

@vaughnbetz
Copy link
Contributor

Just to be paranoid, can you run one design before and after this change (a reasonable size one) and compare CPU time in placement to make sure it doesn't slow down? I think the overhead of the extra check should be minimal but it is good to be paranoid.

@vaughnbetz
Copy link
Contributor

vaughnbetz commented Sep 19, 2025

The pause button and the debug buttons basically worked.
One issue with the debug button: breaking at router iteration 3 didn't actually stop at iteration 3 most of the time, although it did say it stopped sometimes. Stopping in placement seemed OK.
==> Debug the router iteration / debug options.

==> Other issue: update the graphics documentation and debug documentation on the vtr website to explain these buttons.
==> A few pictures at the start of the graphics documentation are also out of date (old menu bar) --> would be good to update them. Later pictures look OK.

@haydar-c
Copy link
Contributor Author

Adding the largest 2 VTR Chain benchmark place times normalized to without change. They seem negligible.

Circuit Normalized Placement Time (With Change / Without Change)
LU32PEEng.v 0.9972
mcml.v 1.0086

Planning to raise another PR for the debug/breakpoint options and graphics documentation (including updating the start of the graphics documentation).

@AlexandreSinger AlexandreSinger merged commit c2cce9a into master Sep 25, 2025
30 checks passed
@AlexandreSinger AlexandreSinger deleted the manual_moves_fix branch September 25, 2025 17:13
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