Skip to content

Conversation

@akashlevy
Copy link
Contributor

@akashlevy akashlevy commented Nov 10, 2025

I discovered a bug when one of our tests failed on is_buffer property check on SKY130. It's clearly a buffer but is marked as a non-buffer because of the wells in SKY130. I've included a minimal example in the tests/ directory.

This PR adds a PortDirection enum value for wells (nwell, pwell, deepnwell, deeppwell) and ignores them in a couple places where they should be ignored (I just searched for isPowerGround() and evaluated whether wells should be ignored at those places).

Let me know if any changes are needed. Thanks!

…sBuffer (+ other functions) to accommodate wells
@akashlevy akashlevy changed the title Add support for "well" direction type (nwell, pwell, etc.), and fix isBuffer (+ other functions) to accommodate wells Add support for "well" direction type (nwell, pwell, etc.), and fix isBuffer (+ other functions) Nov 10, 2025
@akashlevy akashlevy changed the title Add support for "well" direction type (nwell, pwell, etc.), and fix isBuffer (+ other functions) Add support for "well" direction type (nwell, pwell, etc.) and fix isBuffer (+ other functions) Nov 10, 2025
akashlevy added a commit to Silimate/OpenSTA that referenced this pull request Nov 10, 2025
@jjcherry56
Copy link
Collaborator

Well I guess we can argue about the need for a well port direction over using LibertyPort::isPwrGnd, which means pg_pin in liberty speak and is probably what you really mean for excluding wells.

@akashlevy
Copy link
Contributor Author

The difference between isPowerGround and isPwrGnd was confusing me. I didn't notice isPwrGnd before, that seems to obviate the need for distinguishing wells.

Probably the best solution is to simply replace most instances of isPowerGround (which matches only power and ground) with isPwrGnd (which matches any valid pg pin). Shall I amend this to do that?

@akashlevy
Copy link
Contributor Author

(Also isPwrGnd could be renamed to is isPGPin or something like that for more clarity)

@maliberty
Copy link
Contributor

(Also isPwrGnd could be renamed to is isPGPin or something like that for more clarity)

I can't say I find that clearer and we are in the process of integrating with the current name.

@parallaxsw
Copy link
Owner

parallaxsw commented Nov 15, 2025 via email

@akashlevy akashlevy changed the title Add support for "well" direction type (nwell, pwell, etc.) and fix isBuffer (+ other functions) Fix isBuffer Nov 16, 2025
@akashlevy
Copy link
Contributor Author

Yeah, let's minimize changes; I don't care to argue for well as a port direction or isPwrGnd naming.

Let's just fix the issue at hand, which is isBuffer. One-line fix with simple test case demonstrating the issue

akashlevy added a commit to Silimate/OpenSTA that referenced this pull request Nov 16, 2025
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.

4 participants