Skip to content

rsz: BufferToInverters move#10662

Open
erendn wants to merge 5 commits into
The-OpenROAD-Project:masterfrom
erendn:inv_buffer
Open

rsz: BufferToInverters move#10662
erendn wants to merge 5 commits into
The-OpenROAD-Project:masterfrom
erendn:inv_buffer

Conversation

@erendn

@erendn erendn commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR introduces a new inverter buffering move, which replaces a buffer with two inverters uniformly placed along the fanout (--(1/3 wire)--> inv1 --(1/3 wire)--> inv2 --(1/3 wire)-->).

Type of Change

  • New feature

Impact

The new move isn't enabled by default. It can be enabled by adding it to the move sequence, like repair_timing -sequence "unbuffer vt_swap sizeup swap buffer clone split buffer_to_inverters". I have run public CI benchmarks and have seen an average improvement when this move is appended to the default sequence in post-CTS and post-GRT timing repair.

Verification

  • I have verified that the local build succeeds (./etc/Build.sh).
  • I have run the relevant tests and they pass.
  • My code follows the repository's formatting guidelines.
  • I have included tests to prevent regressions.
  • I have signed my commits (DCO).

Signed-off-by: Eren Dogan <erendogan@google.com>
@erendn erendn requested a review from a team as a code owner June 15, 2026 17:22
@erendn erendn requested a review from jhkim-pii June 15, 2026 17:22

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new optimization move, InvBuffer, which replaces a single buffer instance with a pair of cascaded inverters to reduce wire delay. The implementation includes the new InvBufferCandidate and InvBufferGenerator classes, integration into the optimization policies, CLI/Python interface updates, and new unit tests for flat and hierarchical designs. The review feedback highlights several critical improvement opportunities: resolving a logical bug where footprint matching prevents finding valid inverter replacements when -match_cell_footprint is enabled, handling block terminals (BTerms) to ensure accurate physical placement when drivers or loads are ports, and adding defensive nullptr checks on instance creation to prevent potential crashes.

Comment on lines +102 to +106
if (run_config_.match_cell_footprint && !drvr_footprint.empty()
&& !cell->footprint().empty()
&& drvr_footprint != cell->footprint()) {
continue;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

In InvBufferGenerator::generate, the footprint matching check compares the footprint of the original buffer (drvr_footprint) with the footprint of the candidate inverter (cell->footprint()). Since buffers and inverters have different functions, they will almost always have different footprints in Liberty libraries (e.g., buf vs inv). If -match_cell_footprint is enabled, this check will prevent the generator from finding any valid inverter replacement, effectively disabling the InvBuffer move. This check should be removed, or replaced with a Vt-matching check if the goal is to match the threshold voltage class.

Comment thread src/rsz/src/move/InvBufferCandidate.cc Outdated
Comment thread src/rsz/src/move/InvBufferCandidate.cc Outdated
Comment thread src/rsz/src/move/BufferToInvertersCandidate.cc
Signed-off-by: Eren Dogan <erendogan@google.com>
Comment thread src/rsz/include/rsz/Resizer.hh Outdated
erendn added 2 commits June 16, 2026 13:56
Signed-off-by: Eren Dogan <erendogan@google.com>
Signed-off-by: Eren Dogan <erendogan@google.com>
@erendn erendn changed the title rsz: InvBuffer move rsz: BufferToInverters move Jun 16, 2026
Signed-off-by: Eren Dogan <erendogan@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants