ram: auto-detect power and ground pin names#10580
Conversation
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.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 removes the explicit -power_pin and -ground_pin arguments from the generate_ram command, instead automatically detecting power and ground pin names from the cell library and storing them in member sets. Feedback was provided regarding a potential state leakage issue where these member sets accumulate pin names across multiple runs in the same session, suggesting to clear them at the end of ramPdngen.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| for (const auto& pin_name : power_pin_names_) { | ||
| block_->addGlobalConnect(nullptr, ".*", pin_name.c_str(), power_net, true); | ||
| } | ||
|
|
||
| for (const auto& pin_name : ground_pin_names_) { | ||
| block_->addGlobalConnect(nullptr, ".*", pin_name.c_str(), ground_net, true); | ||
| } | ||
|
|
||
| block_->globalConnect(false, false); |
There was a problem hiding this comment.
Since RamGen is a persistent object in the OpenROAD session (retrieved via ord::getRamGen()), the member variables power_pin_names_ and ground_pin_names_ will accumulate pin names across multiple runs of generate_ram in the same session. This can lead to incorrect global connections or errors if different libraries or designs are processed sequentially.
To prevent this state leakage, clear both sets at the end of ramPdngen once the global connections have been established.
| for (const auto& pin_name : power_pin_names_) { | |
| block_->addGlobalConnect(nullptr, ".*", pin_name.c_str(), power_net, true); | |
| } | |
| for (const auto& pin_name : ground_pin_names_) { | |
| block_->addGlobalConnect(nullptr, ".*", pin_name.c_str(), ground_net, true); | |
| } | |
| block_->globalConnect(false, false); | |
| for (const auto& pin_name : power_pin_names_) { | |
| block_->addGlobalConnect(nullptr, ".*", pin_name.c_str(), power_net, true); | |
| } | |
| for (const auto& pin_name : ground_pin_names_) { | |
| block_->addGlobalConnect(nullptr, ".*", pin_name.c_str(), ground_net, true); | |
| } | |
| block_->globalConnect(false, false); | |
| power_pin_names_.clear(); | |
| ground_pin_names_.clear(); |
There was a problem hiding this comment.
It's unnecessary because you would not be creating more than one RAM per session. OpenROAD doesn't support operating on more than one design and likely won't in the future.
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
| for (auto& [role, name] : pin_map) { | ||
| if (role.type == PortRoleType::Power) { | ||
| power_pin_names_.insert(name); | ||
| } else if (role.type == PortRoleType::Ground) { | ||
| ground_pin_names_.insert(name); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Does this guarantee that only one primary power is used? What if there are two different power pins found? We would want to error in that case.
There was a problem hiding this comment.
I have added a error check in baf136e, ready for review
…cross cells, remove unnecessary clear function for pin sets Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
Signed-off-by: Thinh Nguyen <nguyenthinh19011@gmail.com>
2 participants
Summary
Automatically detect power and ground pin names instead of requiring the user to manually specify using
-power_pinand-ground_pin, then globally connect them. The RAM generator will only collect primary power/grounds. README.md and tcl files are updated to reflect this new changeType of Change
Impact
User no longer need to manually specify power and ground names when using the tool
Verification
./etc/Build.sh).Related Issues
Fixes #10562
@rovinski