mbff: expose visibility of some idempotent helper functions#10548
mbff: expose visibility of some idempotent helper functions#10548mikesinouye wants to merge 6 commits into
Conversation
Signed-off-by: Mike Inouye <mikeinouye@google.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the MBFF class in src/gpl/src/mbff.h by explicitly adding public: and private: access specifiers to organize its member functions. Specifically, the clock pin functions are now marked as public, while the (MB)FF utility functions are marked as private. There are no review comments, and I have no feedback to provide.
|
I prefer not to have interleaved public and private sections, as in https://google.github.io/styleguide/cppguide.html#Declaration_Order Just move the method declarations to the existing public segment |
Signed-off-by: Mike Inouye <mikeinouye@google.com>
Done. |
|
This is ok but at the same time I wonder if these shouldn't move to dbNetwork as they have little to do with mbff clustering so it seems strange to require an MBFF object to call them. What do you think of moving them? |
|
In our usage we already have the MBFF object but I agree with your general point. My only concern would be that I haven't validated their usage outside of the mbff context. If you want me to move them I can. |
|
If you don't mind I think it would be cleaner to move them. |
Signed-off-by: Mike Inouye <mikeinouye@google.com>
Signed-off-by: Mike Inouye <mikeinouye@google.com>
|
I moved them and getLibertyScanX() functions from dbSta -> dbNetwork as the new helpers need them but dbNetwork is a dependency of dbSta. Updated callsites to use dbNetwork object in a few different locations. |
| sta::LibertyPort* getLibertyScanEnable(const LibertyCell* lib_cell); | ||
| sta::LibertyPort* getLibertyScanIn(const LibertyCell* lib_cell); | ||
| sta::LibertyPort* getLibertyScanOut(const LibertyCell* lib_cell); |
There was a problem hiding this comment.
Why did these become member functions? It seems unrelated to the mbff change.
There was a problem hiding this comment.
I moved the getLibertyScanX() functions from dbSta -> dbNetwork as the mbff helpers need them but dbNetwork is a dependency of dbSta. The moved mbff functions cannot include the dbSta functions or this will result in a dep loop. To avoid this I moved them to dbNetwork.
Signed-off-by: Mike Inouye <mikeinouye@google.com>
2 participants
Summary
These functions are useful for MBFF utilities in wrapper code that use OpenROAD as a library.
Verification
./etc/Build.sh).