gpl: TD reset momentum and properly update gradients (callbacks)

[gudeh]

The introduction of the momentum reset when changes are performed with the TD iteration showed improved slack, and avoided a divergence with the nangate45/Leon design (large empty space) #11553. Although, two private designs (bp_single and bp_dual) never moved instances after a second TD, this happened because the gradients update was performed only with new instances, old ones had stale gradient values, which lead to the symptom of minimal step length and no movement.,

This PR includes the following changes:

• Bring back TD momentum reset. Undo revert gpl: revert PR #11553 for momentum reset #11578.
• Replace the single gradient update introduced at Integrate rsz repair design with gpl #6165 with the new refreshCurGradients(). Which recomputes gradients for all instances after the TD iteration.
• new_instances_ is now a set, and destroyCbkGCell() erases from it, avoiding duplicates and stale pointers.
• Debug:
  • Selected instance found again after TD reorder GCells. Before, selected_ could point to the wrong cell.
  • reportSelected() now prints each GCell's previous and current position and gradient, using the new NesterovBase::getSlpState().

[gemini-code-assist]

Code Review

This pull request introduces a FISTA momentum restart mechanism in Nesterov global placement after non-virtual timing-driven iterations to prevent placement divergence, alongside tracking selected instances by identity to handle GCell reordering during repairs. Feedback on these changes highlights several critical safety and numerical issues: first, selected_ can go out of bounds or result in a null gcell dereference in reportSelected(); second, calling gcell->insts().front() requires an emptiness check to prevent undefined behavior; and third, leaving prevSLPCoordi_ uninitialized for newly added GCells can heavily distort step length estimation in getStepLength().

[gemini-code-assist]

Timing-driven repairs can reorder, create, or delete GCells, which means selected_ can become out of bounds. Accessing nbc_->getGCellByIndex(selected_) without a bounds check can lead to undefined behavior or a crash. Additionally, if gcell is not found, it will remain nullptr (or invalid), and dereferencing it on the subsequent line (gcell->getName()) will cause a crash. Checking bounds and ensuring gcell is non-null before use prevents these issues.

  const GCell* gcell = (selected_ < nbc_->getGCells().size()) ? nbc_->getGCellByIndex(selected_) : nullptr;
  if (selected_inst_ && (!gcell || !gcell->contains(selected_inst_))) {
    gcell = nullptr;
    for (size_t idx = 0; idx < nbc_->getGCells().size(); ++idx) {
      if (nbc_->getGCellByIndex(idx)->contains(selected_inst_)) {
        selected_ = idx;
        gcell = nbc_->getGCellByIndex(idx);
        break;
      }
    }
  }
  if (!gcell) {
    return;
  }

[gemini-code-assist]

Calling gcell->insts().front() without checking if gcell->insts() is empty can lead to undefined behavior or a crash if the selected GCell does not contain any instances (e.g., if it is a filler or empty GCell). Adding a check for !gcell->insts().empty() ensures safety.

Suggested change

	const auto state
	= nbVec_[nb_index]->getSlpState(gcell->insts().front()->dbInst());
	if (state) {
	const auto state = (!gcell->insts().empty())
	? nbVec_[nb_index]->getSlpState(gcell->insts().front()->dbInst())
	: std::nullopt;
	if (state) {

[gemini-code-assist]

While the momentum restart sets beta = 0 (which ignores prevSLPCoordi_ when computing the next coordinates), prevSLPCoordi_ is still read by NesterovBase::getStepLength() to compute the Barzilai-Borwein step size $t_k$. If prevSLPCoordi_[gcells_index] is left uninitialized (defaulting to (0,0)), the coordinate difference curSLPCoordi - prevSLPCoordi for newly added GCells will be extremely large (equal to their absolute coordinates). This will heavily distort the step length estimation in the first iteration after a timing-driven repair. Initializing prevSLPCoordi_[gcells_index] to the GCell's center coordinates prevents this distortion.

          = prevSLPCoordi_[gcells_index] = FloatPoint(gcell->dCx(), gcell->dCy());

[maliberty]

If accurate this is an issue

[gudeh]

Secure-CI showed a timeout on a private PDK with ibex, we seem to have a really large RSZ table. http://secure-ci:8080/job/SB/job/secure-gpl-gradients-update/3/stages/?selected-node=1100

[maliberty]

getGCellByIndex(selected_) runs before the re-find loop, and it raises GPL 315 when the index is out of range. If a timing-driven repair deletes more cells than it creates (for example, it removes buffers), selected_ can be past the end of the GCell storage. The report then errors out before the recovery code can run. Consider bounds-checking selected_ first and going straight to the search when it's out of range.

[maliberty]

selected_inst_ is never cleared when that instance is deleted. If repair_design removes the selected buffer, the search finds nothing and the report silently shows whichever GCell now sits at the old selected_ index. And if odb reuses the freed dbInst slot, contains(selected_inst_) can match an unrelated instance. Could we reset selected_/selected_inst_ when the search fails, or clear them from the inst-destroy callback?

[maliberty]

getSlpState() reads the host copies of prevSLPCoordi_/curSLPCoordi_ and prevSLPSumGrads_/curSLPSumGrads_. GPU runs that use neither timing-driven nor routability mode keep these on the device, so the new prev/cur pos and grad lines will show zeros or stale values. Should this sync from the device first (pullCoordsFromDevice() plus the sum gradients), as saveSnapshot() does?

[maliberty]

Those three are from codex (all minor)