ram: internal commands moved to cpp file #9312

braydenlouie · 2026-01-22T02:07:11Z

PR migrates TCL scripts called in ram.tcl to cpp methods within ram.cpp

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

Signed-off-by: Brayden Louie <braydenl9988@gmail.com>

gemini-code-assist

Code Review

The pull request successfully refactors the RAM generation flow by migrating power delivery network (PDN) generation, pin placement, filler placement, and routing logic from Tcl scripts to C++ methods within the ram::RamGen class. This improves modularity and provides more direct control over these operations. The RamGen constructor has been updated to incorporate necessary dependencies from other OpenROAD modules, and the Tcl interface now correctly invokes these new C++ functions. The review comments suggest improvements for robustness and maintainability, such as adding nullptr checks and refining constructor design. This is a positive step towards a more robust and maintainable codebase.

gemini-code-assist · 2026-01-22T02:11:31Z

src/ram/src/ram.cpp

+  // need parameters for power and ground nets
+  auto power_net = dbNet::create(block_, "VDD");


The block_ member variable is accessed in ramPdngen, ramPinplacer, and ramRouting. This member is initialized within the generate method. While the generate_ram Tcl command ensures generate is called first, if these C++ methods are called directly via their Tcl/Python bindings without prior initialization of block_, it could lead to a nullptr dereference and a crash. Adding a check for block_ being nullptr at the beginning of these methods would make them more robust against out-of-order calls.

Suggested change

// need parameters for power and ground nets

auto power_net = dbNet::create(block_, "VDD");

if (!block_) {

logger_->error(RAM, 1, "Block is not initialized. Call generate_ram_netlist first.");

}

auto power_net = dbNet::create(block_, "VDD");

auto ground_net = dbNet::create(block_, "VSS");

gemini-code-assist · 2026-01-22T02:11:32Z

src/ram/include/ram/ram.h

+         odb::dbDatabase* db,
+         Logger* logger,
+         pdn::PdnGen* pdngen,
+         ppl::IOPlacer* ioPlacer,
+         dpl::Opendp* opendp_,
+         grt::GlobalRouter* global_router_,
+         drt::TritonRoute* detailed_router_);
  ~RamGen() = default;


The RamGen constructor now accepts a large number of pointers as arguments. While these dependencies are necessary for the new functionality, a constructor with many parameters can sometimes reduce readability and make the class harder to instantiate or test. Consider if some of these related dependencies could be grouped into a single helper struct or if some could be retrieved on-demand from the OpenRoad singleton within the RamGen methods, rather than being stored as direct member pointers. This is a design consideration for improved maintainability.

@maliberty what is the preference on this?

github-actions

clang-tidy made some suggestions

github-actions · 2026-01-22T02:15:23Z

src/OpenRoad.cc

  replace_ = new gpl::Replace(db_, sta_, resizer_, global_router_, logger_);
  pdnsim_ = new psm::PDNSim(logger_, db_, sta_, estimate_parasitics_, opendp_);
  pdngen_ = new pdn::PdnGen(db_, logger_);
+  ram_gen_ = new ram::RamGen(getDbNetwork(), db_, logger_, pdngen_, ioPlacer_, opendp_, global_router_, detailed_router_);


warning: no matching constructor for initialization of 'ram::RamGen' [clang-diagnostic-error]

ram_gen_ = new ram::RamGen(getDbNetwork(), db_, logger_, pdngen_, ioPlacer_, opendp_, global_router_, detailed_router_); ^

Additional context

src/ram/include/ram/ram.h:56: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 8 were provided

class RamGen ^

src/ram/include/ram/ram.h:56: candidate constructor (the implicit default constructor) not viable: requires 0 arguments, but 8 were provided

class RamGen ^

github-actions · 2026-01-22T02:15:23Z

src/ram/include/ram/ram.h

-  RamGen(sta::dbNetwork* network, odb::dbDatabase* db, utl::Logger* logger);
+  RamGen(sta::dbNetwork* network,
+         odb::dbDatabase* db,
+         Logger* logger,


warning: unknown type name 'Logger' [clang-diagnostic-error]

Logger* logger, ^

github-actions · 2026-01-22T02:15:23Z

src/ram/include/ram/ram.h

-  RamGen(sta::dbNetwork* network, odb::dbDatabase* db, utl::Logger* logger);
+  RamGen(sta::dbNetwork* network,
+         odb::dbDatabase* db,
+         Logger* logger,


warning: unknown type name 'Logger'; did you mean 'utl::Logger'? [clang-diagnostic-error]

Suggested change

Logger* logger,

utl::Logger* logger,

Additional context

src/ram/include/ram/ram.h:26: 'utl::Logger' declared here

class Logger; ^

github-actions · 2026-01-22T02:15:24Z

src/ram/src/ram.cpp

+  block_->addGlobalConnect(nullptr, ".*", power_pin, power_net, true);
+  block_->addGlobalConnect(nullptr, ".*", ground_pin, ground_net, true);
+
+  block_->globalConnect();


warning: no matching member function for call to 'globalConnect' [clang-diagnostic-error]

block_->globalConnect(); ^

Additional context

src/odb/include/odb/db.h:751: candidate function not viable: requires 2 arguments, but 0 were provided

int globalConnect(bool force, bool verbose); ^

src/odb/include/odb/db.h:752: candidate function not viable: requires 3 arguments, but 0 were provided

int globalConnect(dbGlobalConnect* gc, bool force, bool verbose); ^

rovinski · 2026-01-22T03:39:32Z

Mostly looks good, please fix the build errors and clang-tidy then I can review.

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

github-actions

clang-tidy made some suggestions

github-actions · 2026-01-23T18:58:18Z

src/ram/src/ram.cpp

+{
+  vector<odb::dbMaster*> filler_masters;
+  for (const std::string& cell : filler_cells) {
+    filler_masters.push_back(db_->findMaster(cell.c_str()));


warning: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation]

src/ram/src/ram.cpp:530:

- for (const std::string& cell : filler_cells) { + filler_masters.reserve(filler_cells.size()); + for (const std::string& cell : filler_cells) {

rovinski · 2026-01-25T23:01:48Z

Looks like clang-format, clang-tidy, and the ram8x8 test are all failing. Please fix the first 2 and investigate the last one. There appears to be an unrelated bazel failure so you can ignore that.

braydenlouie · 2026-01-25T23:28:35Z

@rovinski I'm working on getting the clang-format and clang-tidy fixed as of now. I've been running into build issues with boost::stacktrace_basic when I tried to pull the new globalConnect, that's why last two commits have just been merges. I'm in the process of rebuilding right now, but if I run into the same issue again, I'll comment again.

maliberty · 2026-01-26T03:31:23Z

Note that we upgraded boost versions so you might need to re-run the dependency installer.

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

github-actions · 2026-01-28T01:18:28Z

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

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

github-actions · 2026-01-28T01:41:36Z

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

rovinski

LGTM. Go ahead and mark as ready for review.

braydenlouie added 12 commits December 12, 2025 11:13

ram: beginning of moving tcl scripts to cpp file

e7f828e

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

Merge branch 'The-OpenROAD-Project:master' into internal-tools

4610012

ram: added hard-coded pdngen to cpp src file

0295b71

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: edit to intermediate files and cpp file structure

07df53c

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: added hard-coded pin placer

058e9b3

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: finished hard-coded filler and routing, drt still has issues

64c8039

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: hard-coded cpp internal tool calls working

5b128b9

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: changed parameters for tool calls, no formatting

8b5efb1

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: formatting changes to files

2f26351

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: formatting and added thread count to routing

517b589

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: small tweaks to pdngen method

d24fb2b

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

Merge branch 'master' into internal-tools

f949e10

Signed-off-by: Brayden Louie <braydenl9988@gmail.com>

gemini-code-assist bot reviewed Jan 22, 2026

View reviewed changes

github-actions bot reviewed Jan 22, 2026

View reviewed changes

braydenlouie added 2 commits January 23, 2026 13:52

ram: fixed merge conflicts

607b2d6

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: fixed formatting of OpenRoad.cc

7efbf9c

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

github-actions bot reviewed Jan 23, 2026

View reviewed changes

braydenlouie added 2 commits January 24, 2026 16:57

Merge branch 'The-OpenROAD-Project:master' into internal-tools

a8cd77b

Merge branch 'The-OpenROAD-Project:master' into internal-tools

0cd2f05

ram: fixed formatting, updated BUILD and ok file

91af054

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

ram: fixed BUILD file formatting

687cea8

Signed-off-by: braydenl9988 <braydenl9988@gmail.com>

rovinski approved these changes Jan 28, 2026

View reviewed changes

rovinski requested a review from maliberty January 28, 2026 03:35

braydenlouie marked this pull request as ready for review January 28, 2026 03:50

		// need parameters for power and ground nets
		auto power_net = dbNet::create(block_, "VDD");

ram: internal commands moved to cpp file #9312

Are you sure you want to change the base?

ram: internal commands moved to cpp file #9312

Conversation

braydenlouie commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

rovinski Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

rovinski Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 22, 2026

Choose a reason for hiding this comment

Uh oh!

rovinski commented Jan 22, 2026

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

rovinski commented Jan 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

braydenlouie commented Jan 25, 2026

Uh oh!

maliberty commented Jan 26, 2026

Uh oh!

github-actions bot commented Jan 28, 2026

Uh oh!

github-actions bot commented Jan 28, 2026

Uh oh!

rovinski left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

braydenlouie commented Jan 22, 2026 •

edited

Loading

rovinski commented Jan 25, 2026 •

edited

Loading