Skip to content

verilog: preserve input aliases when port and net names differ - #420

Open
wellitsabhinav wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
wellitsabhinav:fix/verilog-input-alias
Open

wellitsabhinav wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
wellitsabhinav:fix/verilog-input-alias

Conversation

@wellitsabhinav

@wellitsabhinav wellitsabhinav commented Oct 3, 2026 •

Copy link
Copy Markdown

When an input port and its connected net have different names, VerilogWriter::writeAssigns omits the alias. The exported Verilog declares an input port and an undriven internal wire, losing the connection to the input. For example, an input port named external_input connected to internal_input needs assign internal_input = external_input;.

Include input ports in the alias condition and select the assignment operands according to port direction before calling sta::print once. Input aliases drive the net from the port; output aliases keep driving the port from the net. Existing inout and power/ground handling is preserved.

Validation:

  • This PR's integration build 2 compiled OpenSTA and passed all 6,122 OpenSTA tests. The reported failure is in its OpenROAD integration branch.
  • The corrected combined builds in OpenROAD #11624 use this PR's exact OpenSTA revision. Every one of the 436 failing Bazel targets reported by the OpenSTA integration build has an explicit passing verdict in both OpenROAD head build 2 and OpenROAD merge build 2.
  • Both complete combined CI jobs pass. Each full Bazel suite reports 4,451 passing tests, zero failures, and four unchanged GPU skips; cached results are included. All 14 flow tests pass in both jobs.
  • A native renamed-port check verifies both input and output assignment directions and confirms that reading and linking the written Verilog preserves the output's fanin from the input.

CI dependency: this PR's CI-Public/pr-merge reports the original failed integration build 2. That build's trusted pipeline checks out an OpenROAD master revision that does not include #11624. The public job provides an openstaVersion override but no OpenROAD-reference override; editing this PR's Jenkinsfile also does not replace the trusted pipeline loaded from the base branch.

The OpenROAD repair retains each top-level BTerm's connection to its own module net instead of following child feedthroughs through connectedPinIterator, updates four buffer-insertion goldens with their required input aliases, and promotes the cases proved passing by the correction. The original 436 failed targets comprise 214 hierarchy structural targets, 220 conformance targets (including 17 obsolete flat expected failures), and two buffer-insertion targets.

To resolve the integration failure, incorporate the reader and test changes from #11624 into OpenROAD, then rerun this PR's integration check against the corrected dependency. The linked OpenROAD combined builds are green; this PR's original integration check is not yet green.

Signed-off-by: abhinav <abhinav24167@iiitd.ac.in>
@CLAassistant

CLAassistant commented Oct 3, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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

Copy link
Copy Markdown

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 updates VerilogWriter::writeAssigns to handle input ports with names differing from their connected nets, reversing the assignment direction (assign net = port;) for input ports. The feedback suggests refactoring the logic to determine the left-hand side and right-hand side variables beforehand, which avoids duplicating the sta::print call and improves maintainability.

Comment thread verilog/VerilogWriter.cc Outdated
Signed-off-by: abhinav <abhinav24167@iiitd.ac.in>
@wellitsabhinav

wellitsabhinav commented Oct 4, 2026 •

Copy link
Copy Markdown
Author

The red CI-Public/pr-merge status comes from OpenSTA integration build 2. OpenSTA compiled successfully and all 6,122 OpenSTA tests passed; the failure is in the OpenROAD integration tests.

That build combines this PR's OpenSTA changes with an OpenROAD master revision that lacks the companion reader repair. Its 436 failing targets comprise 214 hierarchy structural targets, 220 conformance targets, and two buffer-insertion targets.

The tested repair is in OpenROAD #11624: retain BTerm connections on their exact parent module nets instead of following child feedthroughs, update four buffer goldens with the required input aliases, and remove obsolete expected-failure entries for cases verified passing with the existing assertions.

Both OpenROAD head CI build 2 and OpenROAD merge CI build 2 pass with the repair, using this PR's exact OpenSTA revision. Each full suite reports 4,451 passing tests, zero failures, and four unchanged GPU skips (including cached results), and all 14 flow tests pass. Every one of the original 436 failing targets has an explicit passing verdict in both corrected CI logs.

To make this check green: incorporate the OpenROAD reader and test updates from #11624 into OpenROAD master, then rerun #420's integration CI. This job selects OpenROAD master and exposes no public OpenROAD-reference override, so rerunning before the dependency is incorporated would test the same failing combination.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants