gpl: fix incremental mode by gudeh · Pull Request #9610 · The-OpenROAD-Project/OpenROAD

gudeh · 2026-03-03T16:30:13Z

This PR fixes multiple issues occurring in gpl incremental mode.

Changes include:

Initial placement:
- Properly consider locked instances during initial placement.
Nesterov:
- If instance is locked, do not use small perturbation in nesterov instance initiation.
- Do not allow locked instances to move during nesterov runs.
- Properly refresh isConverged_ variable (previously we would never run the second nesterov run if we reach rough target overflow of 20% before the fixed 300 iterations).
Special options for incremental mode:
- Use uniform density instead of 0.7 default for incremental mode nesterov runs.
- Increase init density penalty to 1 for second nesterov run (avoid restarting placement).
Debug mode:
- Generate one gif for each nesterov run separately.

This PR addresses issue #9542

refresh isConverged for second incremental stage anticipate setting instances as locked, fix considering locked instances at initial placement Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>

… all unlocked) Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>

Signed-off-by: Augusto Berndt <augusto.berndt@precisioninno.com>

gudeh · 2026-03-03T16:34:41Z

This changes should be no-op for master (since during stage 3-4 we rsz is not performing any modification on replace_arith_modules before when gpl incremental mode is called).

I triggered a secure-CI at: https://jenkins.openroad.tools/blue/organizations/jenkins/OpenROAD-flow-scripts-Private/detail/secure-gpl-fix-incremental/4/pipeline

gemini-code-assist

Code Review

This pull request introduces several fixes and improvements for the incremental global placement mode. The changes to handle locked instances within the Nesterov placer and the enhancements to debug visualization appear correct and well-implemented. However, I've identified a potential issue in the initial placement logic that may not correctly fix locked instances to their positions. Additionally, there's a minor point of improvement in the heuristics for the final placement stage.

gemini-code-assist · 2026-03-03T16:35:23Z

src/gpl/src/initialPlace.cpp

+    if (inst->isLocked()) {
+      listX.emplace_back(idx, idx, 1.0f);
+      listY.emplace_back(idx, idx, 1.0f);
+      fixedInstForceVecX_(idx) = inst->cx();
+      fixedInstForceVecY_(idx) = inst->cy();
+    }


While adding a diagonal term to the matrix for locked instances is a step towards fixing their positions, this implementation is incomplete and may not work as intended. The subsequent loop that processes nets will add significant forces from connected movable instances, effectively pulling the 'locked' instances away from their fixed positions. With large net weights, a locked instance's position will be dominated by its neighbors rather than its specified fixed location.

To correctly fix the instances, they should be treated as fixed terminals throughout the matrix construction. This means the net processing loop should not add any matrix terms for connections involving locked instances; instead, they should contribute to the right-hand side (force vector) for any movable instances they are connected to.

The secure-CI run with ID 5 should address this. I am waiting for it to finish to make sure it is no-op, as expected.

secure-CI proves no-op

src/gpl/src/replace.cpp

github-actions · 2026-03-03T16:36:12Z