rsz: exclude delay/clock-delay cells from data buffering (#10622)

[saurav-fermions]

Summary

buffer_ports/repair_design were selecting clock-delay buffers (*clkdlybuf*) and dedicated delay cells (*dlygate*, *dlymetal*, gf180 dly*) as general-purpose data buffers. This restores filtering so the data-buffer candidate set excludes delay/clock-delay cells.

Type of Change

• Bug fix

Impact

Data buffering no longer uses delay cells; affected golden outputs updated.

Verification

• Local build succeeds.
• Relevant tests pass (rebuilt from source): ctest -R '^rsz\.' 238/238; real fail/pass.
• Code follows the repository's formatting guidelines.
• I have signed my commits (DCO).

Related Issues

Fixes #10622

---

Developed with SAIGE, Fermions' autonomous RTL/EDA debugging agent; root-caused, tested, and signed off by the submitter (@saurav-fermions).

[gemini-code-assist]

Code Review

This pull request prevents dedicated delay cells and clock-delay buffers from being selected for general-purpose data buffering. It introduces a new BufferUse::DELAY enum and the isDelayCell helper to identify these cells by name or footprint patterns, updating several tests and adding a new regression test. Feedback on the changes suggests replacing the substring checks for footprints with exact case-insensitive comparisons to avoid redundant checks and prevent false positives on common substrings like "del" matching "model" or "delta".

[gemini-code-assist]

The check containsIgnoreCase(footprint, "delay") is completely redundant because "delay" contains "del" (case-insensitive), which is already matched by containsIgnoreCase(footprint, "DEL").

Furthermore, performing a generic substring match with "DEL" is highly risky and prone to false positives, as it will match any footprint containing "del" (such as "model", "delta", etc.).

Since standard cell footprints are typically exact generic names (e.g., "delay", "DEL", "DLY"), it is much safer and cleaner to use exact case-insensitive comparisons (e.g., using strcasecmp) to avoid these false positives.

  const std::string& footprint = buffer->footprint();
  if (!footprint.empty()
      && (strcasecmp(footprint.c_str(), "delay") == 0
          || strcasecmp(footprint.c_str(), "del") == 0
          || strcasecmp(footprint.c_str(), "dly") == 0)) {
    return true;
  }

[mole99]

Hello, I have doubt about this PR.

Instead of filtering the buffers by name, shouldn't OpenROAD separate data and hold buffers by their cell delay?

As I understand it, the hold buffer is already selected based on the cell delay:

OpenROAD/src/rsz/src/RepairHold.cc

Line 170 in 3a15695

const float buffer_cost = bufferHoldDelay(buffer) / buffer_area;

Why not consider the cell delay when selecting "normal" buffers too?

I imagine a distribution for a number of buffers would look like this:

The metric for selecting a data buffer could be something like: drive strength / footprint / cell delay

@povik what do you think?

[povik]

@mole99 I agree selecting buffers by characteristics is better than pattern matching on name. The filtering in Resizer::findBuffers is meant to do this but fails us. We need a different fix for #10622.

[saurav-fermions]

Thanks — agreed, name matching is fragile. I prototyped the characteristic-based approach: intrinsic stage delay × drive resistance (∝ 1 / (drive/delay), i.e. the drive/delay idea you sketched). On sky130hd it cleanly ranks all delay cells above all normal buffers — but neither axis works alone: bufbuf_* has delay-cell-like intrinsic delay (it's a 2-stage buffer) and buf_1 has delay-cell-like weak drive; only the product separates them.

The concern is the margin. Sorted delay × drive_res on sky130hd:

• normal buffers: buf_16 1.05e-7 … buf_2 4.4e-7 … buf_1 7.6e-7 (highest normal)
• delay cells: clkdlybuf4s15 9.1e-7 (lowest) … dlymetal6s2s 1.06e-6 … dlygate4sd3 5.0e-6

So there's a real gap, but it's only ~1.2× between buf_1 and the lowest delay cell, and I've only validated sky130hd. Since findBuffers is on the path for every design/PDK, I'd rather not hard-code a threshold that might drop a legitimate buf_1 (or admit a delay cell) on another library.

How would you prefer to make this robust across PDKs — a relative/outlier cutoff against the candidate population, or normalize differently (e.g. the drive / footprint / delay you mentioned)? I'll implement whichever you'd like.

(Confirming @povik's point: the existing footprint filter already excludes dlymetal* via their distinct delay footprint; it's dlygate* and clkdlybuf* that share the buf footprint and leak through — those are what need the characteristic check.)