Skip to content

Do not write physical-only cells in the output verilog file#8256

Closed
openroad-ci wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-no-write-physical-only
Closed

Do not write physical-only cells in the output verilog file#8256
openroad-ci wants to merge 1 commit into
The-OpenROAD-Project:masterfrom
The-OpenROAD-Project-staging:secure-no-write-physical-only

Conversation

@openroad-ci

Copy link
Copy Markdown
Member
  • Modified DbInstanceChildIterator to iterate through logical cells (exclude physical-only cells).
  • This makes STA not traverse physical-only cells.
  • In the output .v, there will be no physical-only cells.

Fixes #8254

…clude physical-only cells).

- This makes STA not traverse physical-only cells.
- In the output .v, there will be no physical-only cells.

Signed-off-by: Jaehyun Kim <jhkim@precisioninno.com>
@github-actions

github-actions Bot commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@jhkim-pii jhkim-pii requested a review from maliberty September 7, 2025 11:56
@gadfort

gadfort commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

@maliberty any concerns is changes power or other metrics? Write verilog already has an option to remove cells from the netlist.

@maliberty

Copy link
Copy Markdown
Member

@jhkim-pii please run a small design with and without this change and compare for the concerns above.

@gadfort I don't think we'll have Liberty files for physical only cells like tapcell or fillers so I don't expect it to affect power (still good to check).

@gadfort

gadfort commented Sep 7, 2025

Copy link
Copy Markdown
Contributor

@maliberty I would imagine it would effect leakage power only. If you mock the liberty file to ensure there is enough leakage power that it would be noticable.

An alternative would be to add the ability to selectively preserve the old behavior.

@maliberty

Copy link
Copy Markdown
Member

@gadfort do you have an actual case where it matters or is this theoretical?

@jhkim-pii

Copy link
Copy Markdown
Contributor

I checked the fix in ORFS.
There was no metadata check fail (no notable QoR change).
Let me check if there are small differences in any metrics.

Preserving the old behavior looks good. Do you have any recommendation for the UI?

@gadfort

gadfort commented Sep 8, 2025

Copy link
Copy Markdown
Contributor

@maliberty decap filler cells are physical only, but have leakage power

@maliberty

Copy link
Copy Markdown
Member

@maliberty decap filler cells are physical only, but have leakage power

That is a fair point. @jhkim-pii perhaps we should leave sta seeing these cells in dbNetwork and just change ORFS to use -remove_cells to drop them from Verilog.

@jhkim-pii

Copy link
Copy Markdown
Contributor

Sorry for sharing this result late.
I can see differences in the leakage power only, but they are trivial in ORFS.
Other key metrics are identical. Please refer to the below.

In the current ORFS designs, the leakage power delta is very small.
However, you may concern the bigger gate leakage power error from DECAP cells in advanced nodes. Am I correct?

Hmm... Open STA performs the leakage power calculation. Therefore, no matter how large, the leakage error exists.


ORFS dashboard - Base vs Test:
https://dashboard.precisioninno.com/compare?sourceAType=Commit&sourceBType=Commit&designs=aes%2Cgcd%2Ci2c-gpio-expander%2Cibex%2Cjpeg%2Criscv32i%2Cspi&sourceAName=33e29bff117562482f165cfbc499da0b9a7f4840&sourceAID=5983&sourceBName=e535cd04f669046929bc5636cd8cf6b12eef402b&sourceBID=2

ihp-sg13g2

  • Trivial differences in the leakage
image

gf180

  • Trivial differences in the leakage
image

asap7, nangate45, sky130hd, sky130hs

  • No difference

nangate45 case
image

@jhkim-pii

Copy link
Copy Markdown
Contributor

That is a fair point. @jhkim-pii perhaps we should leave sta seeing these cells in dbNetwork and just change ORFS to use -remove_cells to drop them from Verilog.

Hmm.. I don't know how to do this. How can we check the physical-only attribute with Tcl command?

@maliberty

Copy link
Copy Markdown
Member

We know the classes of cells we want to exclude (fill, tap). Those cells are specified in variables that we can use.

@jhkim-pii

Copy link
Copy Markdown
Contributor

Got it.
Discard this approach due to the leakage power error.

This is an alternative ORFS solution (it does not dump physical-only cells by write_verilog).
The-OpenROAD-Project/OpenROAD-flow-scripts#3480

@jhkim-pii jhkim-pii closed this Sep 9, 2025
@openroad-ci openroad-ci deleted the secure-no-write-physical-only branch December 27, 2025 14:20
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.

floorplan: No TAPCELL insertion in asap7/mock-array design w/ the hierarchical flow

4 participants