Skip to content

rsz: Preserve port-backed ModNet names during buffer removal - #11258

Open
jhkim-pii wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-fix-hier-flow-issue
Open

rsz: Preserve port-backed ModNet names during buffer removal#11258
jhkim-pii wants to merge 3 commits into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-fix-hier-flow-issue

Conversation

@jhkim-pii

Copy link
Copy Markdown
Contributor

Summary

  • Preserve input/inout port-backed hierarchical ModNet names when removing timing-repair buffers.
  • Prevent write_verilog from emitting kept-module boundary loads without a driver.

Problem

  • BufRemTest3.HierInputPortNamePreservedAfterBufferRemoval reproduces the failure through the same insert/remove ECO APIs used by timing repair. The snippets below show the relevant Verilog at each operation.
  1. Initial netlist

    module top (a, seed, out0, out1);
      input a;
      input seed;
      output out0;
      output out1;
      wire signal;
      Source source (.in(a), .out(signal));
      Sink sink (.seed(seed), .out0(out0), .out1(out1));
    endmodule
    
    module Sink (seed, out0, out1);
      input seed;
      Load0 load0 (.i(seed), .out(out0));
      Load1 load1 (.i(seed), .out(out1));
    endmodule
  2. u_buf is inserted by Rebuffer.

    module top (a, seed, out0, out1);
      input a;
      input seed;
      output out0;
      output out1;
      wire signal;
      Source source (.in(a), .out(signal));
      Sink sink (.net(signal), .seed(seed), .out0(out0), .out1(out1));
    endmodule
    
    module Sink (net, seed, out0, out1);
      input net;
      input seed;
      wire load_net;
      BUF_X2 u_buf (.A(net), .Z(load_net));     // INSERTED BUFFER
      Load0 load0 (.i(load_net), .out(out0));   // Net name changes: seed -> load_net
      Load1 load1 (.i(load_net), .out(out1));
    endmodule
    • This intermediate netlist is correct: input port net drives u_buf, and load_net is driven by the buffer output.
  3. u_buf is removed by remove_buffer. In the output verilog, the undriven net load_net remains.

    module Sink (net, seed, out0, out1);
      input net;
      input seed;
      wire load_net;
      Load0 load0 (.i(load_net), .out(out0));    // BUFFER u_buf is removed. But net name "load_net" remains  --> UNDRIVEN
      Load1 load1 (.i(load_net), .out(out1));
      // Missing: assign load_net = net;
    endmodule
  • The net name should be seed instead of load_net.

Cause

  • In the regression, when u_buf is removed, one of two nets (input net net and output net load_net) should survive.
  • net is selected as the surviving net, but its corresponding ModNet name is renamed to load_net, which is wrong.
  • removeBuffer() selects source/driver/out and its input-port ModNet net as the survivors, while sink/load_net and its ModNet load_net are removed.
  • Because the surviving flat net is deeper, the old depth-based naming logic copies the shallower removed names to both survivors: sink/load_net to the flat net and load_net to the ModNet.
  • After the ModNets are merged, the Sink.net ModBTerm remains connected but its surviving ModNet is renamed from net to load_net.
  • write_verilog then declares input net, connects the loads to load_net, and emits no input-to-net assign, producing the undriven loads.

Solution

  • Add dbModNet::isConnectedToInputPort() and isConnectedToOutputPort() to detect top-level BTerms and hierarchical ModBTerms, including inout/feedthrough directions.
  • In Resizer::removeBuffer(), detect when the deeper surviving ModNet is connected to an input-capable port before applying the depth-based rename.
  • Preserve that ModNet name instead of copying the removed output ModNet name. In the regression, the surviving ModNet therefore remains net and continues to represent Sink.net correctly.
  • Copy the shallower flat-net name only when it remains represented after the ModNet merge. Since the removed ModNet owns sink/load_net in the regression, neither survivor adopts that name.

Related

Add a hierarchy-faithful reproducer that inserts a buffer across sibling modules through the production rebuffer API. Verify targeted removal preserves the input-port ModNet name and emitted Verilog connectivity.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Detect top-level and hierarchical input, output, and inout ports symmetrically when removing feedthrough buffers. Keep input/inout survivor ModNet names and adopt a shallower flat name only when it remains represented, preventing write_verilog from emitting undriven loads.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
Add reusable input/output port connectivity queries that cover top-level BTerms and hierarchical ModBTerms, including inout/feedthrough directions. Use the OpenDB API in Resizer buffer-removal logic to remove duplicated topology checks without changing behavior.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
@jhkim-pii jhkim-pii self-assigned this Aug 28, 2026

@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 isConnectedToInputPort() and isConnectedToOutputPort() helper methods to dbModNet to check for connections to input/output ports. These methods are utilized in the Resizer to correctly preserve input/inout port names during buffer removal, ensuring that write_verilog retains the necessary feedthrough assigns. A new unit test has also been added to verify this behavior. I have no feedback to provide.

@oharboe

oharboe commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

🥳

@jhkim-pii
jhkim-pii marked this pull request as ready for review August 31, 2026 04:01
@jhkim-pii
jhkim-pii requested review from a team as code owners August 31, 2026 04:01
@jhkim-pii

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-31T04:03:38.987285Z f6de0bc Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: f6de0bc8c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

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.

2 participants