ram: added multi-port configuration support by braydenlouie · Pull Request #10258 · The-OpenROAD-Project/OpenROAD

braydenlouie · 2026-04-25T00:07:28Z

Summary

Added multi-port support to the RAM generator, allowing for RAM configurations with varying read, write, and read-write ports. Includes a refactoring of decoder creation, changes to decoder output buffer location, and updates to behavioral Verilog generation.

Type of Change

New feature
Breaking change
Refactoring

Impact

The generate_ram command now takes -rw_ports, -r_ports, and -w_ports instead of -read_ports. Existing TCL scripts using -read_ports 1 will need to be updated to 1-rw_ports1 1. The generated RAM netlist now supports 1rw, 1rw1r, 1r1w, and 2r1w configurations with independent address buses and select logic per port

Verification

I have verified that the local build succeeds (./etc/Build.sh).
I have run the relevant tests and they pass.
My code follows the repository's formatting guidelines.
I have signed my commits (DCO).

Related Issues

#9392

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

gemini-code-assist

Code Review

This pull request refactors the RAM generator to support multiple port types (Read-Only, Write-Only, and Read-Write) by modularizing the decoder, buffer, and inverter logic into column-based layouts. The changes include significant updates to the generation flow and the addition of validation logic in the TCL interface. Review feedback identified a logic error in the decoder layer calculation for non-power-of-2 word counts and potential division-by-zero vulnerabilities in the column placement logic when the number of address inputs is zero.

gemini-code-assist · 2026-04-25T00:10:27Z

+  int layers = std::log2(num_words) - 1;

-  for (int i = 0; i < layers; ++i) {
-    auto input_net = makeNet(prefix, fmt::format("layer_in{}", i));
-    // sets up first AND gate, closest to slice's select + write enable gate
-    if (i == 0 && i == layers - 1) {
-      makeInst(word_cell.get(),
-               prefix,
-               fmt::format("and_layer{}", i),
-               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
-                {and2_ports_[{PortRoleType::DataIn, 1}], addr_nets[i + 1]},
-                {and2_ports_[{PortRoleType::DataOut, 0}], decoder_out_net}});
-      prev_net = input_net;
-    } else if (i == 0) {
-      makeInst(word_cell.get(),
-               prefix,
-               fmt::format("and_layer{}", i),
-               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
-                {and2_ports_[{PortRoleType::DataIn, 1}], input_net},
-                {and2_ports_[{PortRoleType::DataOut, 0}], decoder_out_net}});
-      prev_net = input_net;
-    } else if (i == layers - 1) {  // last AND gate layer
-      makeInst(word_cell.get(),
-               prefix,
-               fmt::format("and_layer{}", i),
-               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
-                {and2_ports_[{PortRoleType::DataIn, 1}], addr_nets[i + 1]},
-                {and2_ports_[{PortRoleType::DataOut, 0}], prev_net}});
-      prev_net = input_net;
-    } else {  // middle AND gate layers
+  for (int word = 0; word < num_words; ++word) {
+    auto word_cell = std::make_unique<Cell>();
+    dbNet* prev_net = decoder_output_nets[word];
+    for (int i = 0; i < layers; ++i) {
+      dbNet* input_net = nullptr;
+      if (i == layers - 1) {
+        input_net = addr_nets[word][i + 1];
+      } else {
+        input_net = makeNet(prefix, fmt::format("layer_in{}_word{}", i, word));
+      }
      makeInst(word_cell.get(),
               prefix,
-               fmt::format("and_layer{}", i),
+               fmt::format("and_layer{}_word{}", i, word),
               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
+               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[word][i]},
                {and2_ports_[{PortRoleType::DataIn, 1}], input_net},
                {and2_ports_[{PortRoleType::DataOut, 0}], prev_net}});
      prev_net = input_net;
    }


The calculation of layers using std::log2(num_words) - 1 is incorrect for non-power-of-2 word counts because of integer truncation. For example, if num_words is 7, std::log2(7) is approximately 2.8, which truncates to 2, resulting in layers = 1. However, 7 words require 3 address bits and thus 2 AND gate layers.

Additionally, the current implementation fails to handle the case where num_words is 2 (num_inputs is 1). In this case, layers becomes 0, the loop does not execute, and the decoder_output_nets are left unconnected. A buffer should be used to connect the address bit directly to the decoder output in this scenario. Ensure that the buffer insertion follows the rule that a buffer should not be inserted if there is more than one driver.

int num_inputs = std::ceil(std::log2(num_words)); int layers = num_inputs - 1; for (int word = 0; word < num_words; ++word) { auto word_cell = std::make_unique<Cell>(); dbNet* prev_net = decoder_output_nets[word]; if (layers == 0) { makeInst(word_cell.get(), prefix, fmt::format("buf_word{}", word), buffer_cell_, {{buffer_ports_[{PortRoleType::DataIn, 0}], addr_nets[word][0]}, {buffer_ports_[{PortRoleType::DataOut, 0}], prev_net}}); } else { for (int i = 0; i < layers; ++i) { dbNet* input_net = nullptr; if (i == layers - 1) { input_net = addr_nets[word][i + 1]; } else { input_net = makeNet(prefix, fmt::format("layer_in{}_word{}", i, word)); } makeInst(word_cell.get(), prefix, fmt::format("and_layer{}_word{}", i, word), and2_cell_, {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[word][i]}, {and2_ports_[{PortRoleType::DataIn, 1}], input_net}, {and2_ports_[{PortRoleType::DataOut, 0}], prev_net}}); prev_net = input_net; } } }

References

A buffer should not be inserted if there is more than one driver.

gemini-code-assist · 2026-04-25T00:10:27Z

+
+  // number of inverters per bit group (one per port in this column)
+  // fillers evenly space each bit group within the column
+  int fillers_per_group = (num_words / num_inputs) - ports_in_col;


Potential division by zero if num_inputs is 0. While num_words is expected to be at least 2, adding a defensive check or ensuring num_inputs is at least 1 would make the code more robust.

gemini-code-assist · 2026-04-25T00:10:27Z


-  ram_grid_.addLayout(std::move(cell_inv_layout));
+  // determining how many inverters can fit in one column
+  int ports_per_col = num_words / num_inputs;


Potential division by zero if num_inputs is 0. This can happen if num_words is 1. Consider adding a check to ensure num_inputs > 0 before performing the division.

github-actions

clang-tidy made some suggestions

github-actions · 2026-04-25T00:12:33Z

-                                    const std::vector<odb::dbNet*>& selects,
-                                    const std::vector<odb::dbNet*>& addr_nets);
+  void makeDecoderColumn(const std::string& prefix,
+                         const int num_words,


warning: parameter 'num_words' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int num_words,

int num_words,

github-actions · 2026-04-25T00:12:34Z

+                         const std::vector<std::vector<odb::dbNet*>>& addr_nets,
+                         const std::vector<odb::dbNet*>& decoder_output_nets);
+  void makeBufferColumn(const std::string& prefix,
+                        const int num_words,


warning: parameter 'num_words' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int num_words,

int num_words,

github-actions · 2026-04-25T00:12:34Z

+                        const std::vector<odb::dbNet*>& select_nets);
+
+  void makeSelectColumn(const std::string& prefix,
+                        const int num_words,


warning: parameter 'num_words' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int num_words,

int num_words,

github-actions · 2026-04-25T00:12:34Z

+                        const std::vector<odb::dbNet*>& write_select_nets);
+
+  std::unique_ptr<Layout> makeInverterColumn(
+      const int num_words,


warning: parameter 'num_words' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int num_words,

int num_words,

github-actions · 2026-04-25T00:12:34Z

+
+  std::unique_ptr<Layout> makeInverterColumn(
+      const int num_words,
+      const int num_inputs,


warning: parameter 'num_inputs' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int num_inputs,

int num_inputs,

github-actions · 2026-04-25T00:12:34Z

+      const int num_words,
+      const int num_inputs,
+      const int start_port,
+      const int end_port,


warning: parameter 'end_port' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int end_port,

int end_port,

github-actions · 2026-04-25T00:12:34Z

                                      int read_ports);

+  std::vector<odb::dbNet*> makeDecoderOutputNets(const std::string& prefix,
+                                                 const int num_words);


warning: parameter 'num_words' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int num_words);

int num_words);

github-actions · 2026-04-25T00:12:34Z

+                                                 const int num_words);
+
+  std::vector<odb::dbNet*> makeSelectNets(const std::string& prefix,
+                                          const int num_words,


warning: parameter 'num_words' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

const int num_words,

int num_words,

github-actions · 2026-04-25T00:12:34Z

+  // fillers evenly space each bit group within the column
+  int fillers_per_group = (num_words / num_inputs) - ports_in_col;
+  // ensures we don't go negative for small rams
+  fillers_per_group = std::max(0, fillers_per_group);


warning: no header providing "std::max" is directly included [misc-include-cleaner]

src/ram/src/ram.cpp:5:

- #include <array> + #include <algorithm> + #include <array>

github-actions · 2026-04-25T00:12:34Z

+  // building out inverter columns
+  for (int col = 0; col < inv_col_count; ++col) {
+    int start_port = col * ports_per_col;
+    int end_port = std::min(start_port + ports_per_col, total_ports);


warning: no header providing "std::min" is directly included [misc-include-cleaner]

int end_port = std::min(start_port + ports_per_col, total_ports); ^

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

github-actions · 2026-04-25T00:36:22Z

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

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

github-actions · 2026-04-25T01:48:08Z

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

braydenlouie · 2026-04-25T16:37:25Z

@rovinski PR is ready for review. CI is failing because the defok and lefok files need to be updated, but I'm waiting to address any changes from reviews first before updating that.

rovinski · 2026-04-27T08:09:30Z

+  ReadOnly,
+  WriteOnly,


Can just be Read / Write

rovinski · 2026-04-27T08:12:41Z

+    if (i == 0 && rw_ports > 0) {
+      addr_name = "addr_rw";
+    } else if (rw_ports > 0) {
+      addr_name = fmt::format("addr_r{}", i - 1);


should this be addr_r or addr_rw?

rovinski · 2026-04-27T08:14:39Z

@gemini-code-assist /review

gemini-code-assist

Code Review

This pull request refactors the RAM generator to support multiple port types (read-only, write-only, and read-write) by updating the generation logic, interfaces, and behavioral Verilog output. Key changes include the introduction of RamPortType and the modularization of column generation for decoders, buffers, and inverters. Review feedback highlights a logic failure in the decoder for small RAM sizes (1-2 words), an incorrect port indexing condition in the generate function, and a suggestion to use a more precise column count calculation for layout predictability.

gemini-code-assist · 2026-04-27T08:16:53Z

+  int layers = std::ceil(std::log2(num_words)) - 1;

-  for (int i = 0; i < layers; ++i) {
-    auto input_net = makeNet(prefix, fmt::format("layer_in{}", i));
-    // sets up first AND gate, closest to slice's select + write enable gate
-    if (i == 0 && i == layers - 1) {
-      makeInst(word_cell.get(),
-               prefix,
-               fmt::format("and_layer{}", i),
-               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
-                {and2_ports_[{PortRoleType::DataIn, 1}], addr_nets[i + 1]},
-                {and2_ports_[{PortRoleType::DataOut, 0}], decoder_out_net}});
-      prev_net = input_net;
-    } else if (i == 0) {
-      makeInst(word_cell.get(),
-               prefix,
-               fmt::format("and_layer{}", i),
-               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
-                {and2_ports_[{PortRoleType::DataIn, 1}], input_net},
-                {and2_ports_[{PortRoleType::DataOut, 0}], decoder_out_net}});
-      prev_net = input_net;
-    } else if (i == layers - 1) {  // last AND gate layer
-      makeInst(word_cell.get(),
-               prefix,
-               fmt::format("and_layer{}", i),
-               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
-                {and2_ports_[{PortRoleType::DataIn, 1}], addr_nets[i + 1]},
-                {and2_ports_[{PortRoleType::DataOut, 0}], prev_net}});
-      prev_net = input_net;
-    } else {  // middle AND gate layers
+  for (int word = 0; word < num_words; ++word) {
+    auto word_cell = std::make_unique<Cell>();
+    dbNet* prev_net = decoder_output_nets[word];
+    for (int i = 0; i < layers; ++i) {
+      dbNet* input_net = nullptr;
+      if (i == layers - 1) {
+        input_net = addr_nets[word][i + 1];
+      } else {
+        input_net = makeNet(prefix, fmt::format("layer_in{}_word{}", i, word));
+      }
      makeInst(word_cell.get(),
               prefix,
-               fmt::format("and_layer{}", i),
+               fmt::format("and_layer{}_word{}", i, word),
               and2_cell_,
-               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[i]},
+               {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[word][i]},
                {and2_ports_[{PortRoleType::DataIn, 1}], input_net},
                {and2_ports_[{PortRoleType::DataOut, 0}], prev_net}});
      prev_net = input_net;
    }
+    decoder_layout->addCell(std::move(word_cell));
  }


The decoder logic fails for RAMs with 1 or 2 words. When num_words is 2, layers becomes 0, and the loop that creates AND gates is skipped, leaving decoder_output_nets floating. For 1 word, layers is -1. In these cases, the decoder output should be driven either by the address bit (for 2 words) or tied high (for 1 word). Since the layout expects a cell in this column, a buffer should be used to drive the output net from the appropriate source. Note that per repository guidelines, a buffer should not be inserted if there is more than one driver.

int layers = std::ceil(std::log2(num_words)) - 1; for (int word = 0; word < num_words; ++word) { auto word_cell = std::make_unique<Cell>(); dbNet* prev_net = decoder_output_nets[word]; if (layers > 0) { for (int i = 0; i < layers; ++i) { dbNet* input_net = nullptr; if (i == layers - 1) { input_net = addr_nets[word][i + 1]; } else { input_net = makeNet(prefix, fmt::format("layer_in{}_word{}", i, word)); } makeInst(word_cell.get(), prefix, fmt::format("and_layer{}_word{}", i, word), and2_cell_, {{and2_ports_[{PortRoleType::DataIn, 0}], addr_nets[word][i]}, {and2_ports_[{PortRoleType::DataIn, 1}], input_net}, {and2_ports_[{PortRoleType::DataOut, 0}], prev_net}}); prev_net = input_net; } } else if (num_words == 2) { makeInst(word_cell.get(), prefix, fmt::format("sel_buf_{}", word), buffer_cell_, {{buffer_ports_[{PortRoleType::DataIn, 0}], addr_nets[word][0]}, {buffer_ports_[{PortRoleType::DataOut, 0}], prev_net}}); } decoder_layout->addCell(std::move(word_cell)); }

References

A buffer should not be inserted if there is more than one driver.

@rovinski Do you want to keep support for 1-word RAMs? It may just need a special exception case.

As we discussed in the meeting, 1-word RAMs don't make sense. You don't have to go out of the way to support 2-word either. But there should be a clear error message in these cases about what the minimum supported word size is. This check should happen very early in the code.

gemini-code-assist · 2026-04-27T08:16:53Z

+          fmt::format("rw_sel_p{}", p), num_words, RamPortType::ReadWrite);
+      read_select_nets[r_sel_idx++] = write_select_nets[w_sel_idx];
+      ++w_sel_idx;
+    } else if (p < w_ports) {


The condition p < w_ports is incorrect here. Based on the port ordering established earlier (RW ports, then W ports, then R ports), this should check if the current port index p falls within the range of write-capable ports (RW + W).

Suggested change

} else if (p < w_ports) {

} else if (p < rw_ports + w_ports) {

gemini-code-assist · 2026-04-27T08:16:53Z

-  ram_grid_.setNumLayouts(col_cell_count + 1);
+  // Each read/write port needs a buffer col, decoder col, and inv col
+  int total_ports = rw_ports + r_ports + w_ports;
+  int col_cell_count = slices_per_word * (mask_size + 1) + (total_ports * 3);


The calculation of col_cell_count is an upper-bound estimate that may lead to empty columns at the end of the RAM layout. While Grid handles dynamic resizing, pre-allocating the exact number of columns is preferred for layout predictability. The exact count is slices_per_word * (mask_size + 1) + 2 * total_ports + inv_col_count.

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

github-actions · 2026-04-27T18:42:37Z

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

braydenlouie · 2026-04-27T18:44:11Z

@gemini-code-assist /review

gemini-code-assist

Code Review

This pull request refactors the RAM generator to support multiple port types, including read-write, read-only, and write-only ports. It introduces a more modular column-based layout approach with specific methods for decoder, buffer, and inverter columns, and updates the behavioral Verilog generation to reflect these changes. The review feedback identifies a bug in the Verilog port naming logic, points out dead code in the new column generation functions, and suggests improvements for type casting and error message formatting consistency.

gemini-code-assist · 2026-04-27T18:47:06Z

+    if (i == 0 && rw_ports > 0) {
+      addr_name = "addr_rw";
+    } else if (rw_ports > 0) {
+      addr_name = fmt::format("addr_rw{}", i - 1);


The address name for read-only ports is incorrectly generated as addr_rw{i-1} when a read-write port is present. Based on the declarations in lines 1258-1261, it should be addr_r{i-1}. This will cause simulation or synthesis errors due to mismatched port names.

Suggested change

addr_name = fmt::format("addr_rw{}", i - 1);

addr_name = fmt::format("addr_r{}", i - 1);

gemini-code-assist · 2026-04-27T18:47:06Z

+void RamGen::makeSelectColumn(const std::string& prefix,
+                              const int num_words,
+                              RamPortType port_type,
+                              dbNet* write_enable,
+                              const std::vector<dbNet*>& decoder_output_nets,
+                              const std::vector<dbNet*>& read_select_nets,
+                              const std::vector<dbNet*>& write_select_nets)
+{
+  auto select_layout = std::make_unique<Layout>(odb::vertical);
+  if (port_type == RamPortType::Read) {
+    for (int word = 0; word < num_words; ++word) {
+      auto sel_cell = std::make_unique<Cell>();
+      makeInst(
+          sel_cell.get(),
+          prefix,
+          fmt::format("select_inv_{}", word),
+          inv_cell_,
+          {{inv_ports_[{PortRoleType::DataIn, 0}], decoder_output_nets[word]},
+           {inv_ports_[{PortRoleType::DataOut, 0}], read_select_nets[word]}});
+      select_layout->addCell(std::move(sel_cell));
+    }
+  } else if (port_type == RamPortType::Write) {
+    for (int word = 0; word < num_words; ++word) {
+      auto sel_cell = std::make_unique<Cell>();
+      makeInst(
+          sel_cell.get(),
+          prefix,
+          fmt::format("we_and{}", word),
+          and2_cell_,
+          {{and2_ports_[{PortRoleType::DataIn, 0}], decoder_output_nets[word]},
+           {and2_ports_[{PortRoleType::DataIn, 1}], write_enable},
+           {and2_ports_[{PortRoleType::DataOut, 0}], write_select_nets[word]}});
+      select_layout->addCell(std::move(sel_cell));
+    }
+
+  } else if (port_type == RamPortType::ReadWrite) {
+    for (int word = 0; word < num_words; ++word) {
+      auto sel_cell = std::make_unique<Cell>();
+      makeInst(
+          sel_cell.get(),
+          prefix,
+          fmt::format("we_and_{}", word),
+          and2_cell_,
+          {{and2_ports_[{PortRoleType::DataIn, 0}], decoder_output_nets[word]},
+           {and2_ports_[{PortRoleType::DataIn, 1}], write_enable},
+           {and2_ports_[{PortRoleType::DataOut, 0}], write_select_nets[word]}});
+
+      makeInst(
+          sel_cell.get(),
+          prefix,
+          fmt::format("select_inv_{}", word),
+          inv_cell_,
+          {{inv_ports_[{PortRoleType::DataIn, 0}], decoder_output_nets[word]},
+           {inv_ports_[{PortRoleType::DataOut, 0}], read_select_nets[word]}});
+      select_layout->addCell(std::move(sel_cell));
+    }
+  }
+  ram_grid_.addLayout(std::move(select_layout));
+}


The function makeSelectColumn is defined but never called within the generate method or elsewhere in the class. This appears to be dead code or an incomplete refactoring of the select logic that is currently still handled inside makeSlice.

gemini-code-assist · 2026-04-27T18:47:06Z

+  const int num_inputs = std::ceil(std::log2(num_words));
+  const int ports_per_col
+      = (num_inputs > 0) ? (num_words / num_inputs) : total_ports;
+  const int inv_col_count = std::ceil((float) total_ports / ports_per_col);


Avoid using C-style casts in C++ code. Using static_cast<float> is preferred for better type safety and clarity.

Suggested change

const int inv_col_count = std::ceil((float) total_ports / ports_per_col);

const int inv_col_count = std::ceil(static_cast<float>(total_ports) / ports_per_col);

gemini-code-assist · 2026-04-27T18:47:06Z

+  }
+
+  if { $rw_ports + $r_ports < 1 } {
+    utl::error RAM:33 "Must specify at least one read output port"


The error ID RAM:33 contains an extra colon, which is inconsistent with the standard RAM <number> format used for other error messages in this file.

utl::error RAM 33 "Must specify at least one read output port"

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

github-actions · 2026-04-27T19:03:34Z

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

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

github-actions · 2026-04-27T22:37:06Z

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

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

rovinski · 2026-06-01T03:04:51Z

Do you know why the unrelated .ok files are changing? Adding more ports shouldn't change the default 1rw memories.

braydenlouie · 2026-06-01T03:08:40Z

The changes for addressing pin naming convention required changes to the on file since the logs have a line where it lists out the constrained region for those pins. It was the thing causing the failures on the CI tests.

braydenlouie · 2026-06-01T03:10:44Z

For the latch and mux tests, the ok files had to be changed since the order in which the cells were being placed had to be redone.

rovinski · 2026-06-01T03:12:27Z

Sorry I guess I mean more of the .lefok/.defok files. Is that also only because of the name changes?

rovinski · 2026-06-01T03:18:51Z

For the latch and mux tests, the ok files had to be changed since the order in which the cells were being placed had to be redone.

Interesting that e.g. 8x8_latch_sky130 shrunk by 3um?

-  SIZE 95.68 BY 27.2 ;
+  SIZE 92 BY 27.2 ;

braydenlouie · 2026-06-01T18:54:09Z

Unsure about the latch one specifically, since even when I run on the upstream version of the RAM tool, the generated RAM shrunk by 3 um.

For the mux tests, the positions of the AND and INV were altered, otherwise, there were congestion issues with using col/mux with multi-port. That change caused the placement of the AND and INV gates used for col/mux to be placed in different areas for the make_8x8_mux4 and make_8x8_mux2 tests. The net and pin connections remain the same.

The changes to the remainder of the tests are all pin names and the order in which routing metals appear, since I had changed it to a different naming convention initially, updated all the ok files, and then changed the convention back based on the review changes.

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

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

rovinski · 2026-06-02T02:55:53Z

Unsure about the latch one specifically, since even when I run on the upstream version of the RAM tool, the generated RAM shrunk by 3 um.

I'm confused. Is this an issue of Bazel vs. CMake again? If the upstream test doesn't match the CI then something should be failing?

For the mux tests, the positions of the AND and INV were altered

Altered how so? Is this intentional, and if so why? I think the main thing is that adding a new feature should not change the output of previously working configurations, unless there is a good reason to.

otherwise, there were congestion issues with using col/mux with multi-port. That change caused the placement of the AND and INV gates used for col/mux to be placed in different areas for the make_8x8_mux4 and make_8x8_mux2 tests. The net and pin connections remain the same.

It seems to me that the old placement should be restored then if the new one doesn't work?
Although do you really mean "col/mux with multi-port" because the existing tests are single port.

The changes to the remainder of the tests are all pin names and the order in which routing metals appear, since I had changed it to a different naming convention initially, updated all the ok files, and then changed the convention back based on the review changes.

ok

braydenlouie · 2026-06-02T04:34:26Z

I'm confused. Is this an issue of Bazel vs. CMake again? If the upstream test doesn't match the CI then something should be failing?

It's an issue with cell selection. I haven't made any changes to that part of the codebase, but if you take a look at the defok and lefok that are upstream and running the test locally, the latches used are different. I've attached a screenshot as a side by side. The left is the upstream defok and lefok files and the right is when I run the make_8x8_latch_sky130 test.

rovinski · 2026-06-02T04:45:16Z

I see, ok that's odd. I would create an issue that the cell selection is non-deterministic so that it can be fixed later. I think the short term solution is to manually specify the latch cell so that there is no ambiguity. Thanks for investigating.

braydenlouie · 2026-06-02T04:50:10Z

Altered how so? Is this intentional, and if so why? I think the main thing is that adding a new feature should not change the output of previously working configurations, unless there is a good reason to.

It seems to me that the old placement should be restored then if the new one doesn't work?
Although do you really mean "col/mux with multi-port" because the existing tests are single port.

I didn't add any unit tests for column mux support with multi-port, but I did some local testing to check compatibility, There were issues with congestion with the old layout of the gates. The new rearrangement of the gates was just to ease congestion by moving them closer to where the address pins were being placed by the pin placer.

rovinski · 2026-06-02T04:59:38Z

No worries, don't worry about supporting col/mux>1 + multi-port yet. Let's just get the basic support merged and then it can be worked on later.

rovinski · 2026-06-02T05:05:12Z

Once you fix the latch selection, I think this can be approved.

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

rovinski · 2026-06-10T03:08:35Z

@braydenlouie any updates?

braydenlouie · 2026-06-12T08:04:46Z

Sorry for the lack of updates, got caught up in work. I added the manual specification for the latch in the tcl script. I haven't reversed any of the changes related to col/mux AND cell placement to accommodate multi-port support. If those are still wanted, I can work on it this weekend when I have more time.

rovinski · 2026-06-12T21:14:14Z

@braydenlouie I'd just like to get this PR clean so that it can be merged. Anything else can be done later.

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

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

braydenlouie · 2026-06-16T06:08:03Z

@rovinski PR is ready for review

rovinski

@maliberty unrelated CI failure

eder-matheus · 2026-06-16T20:10:08Z

@rovinski @braydenlouie I've restarted the CI pipeline that's failing. If it still failing, maybe a merge with latest master could help.

eder-matheus · 2026-06-17T16:49:58Z

The errors in the CI persist even after restarting the pipeline. Could you try merging with master again?

rovinski · 2026-06-17T20:29:01Z

Looks good to go

braydenlouie added 9 commits April 18, 2026 20:24

ram: start of multi-port support

187dfe5

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

ram: makeDecoder simplification

1277845

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

ram: new multi-port methods

fdeb379

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

ram: initial multi-port complete

23c4652

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

ram: code cleanup and tcl file fixes

97b2c92

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

ram: refactoring read/write convention

5da1189

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

ram: behavioral verilog multiport support

445cc74

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

ram: reworked inverter placement

bfad700

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

ram: comment and method cleanup

db17a32

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

github-actions Bot added the size/L label Apr 25, 2026

gemini-code-assist Bot reviewed Apr 25, 2026

View reviewed changes

github-actions Bot reviewed Apr 25, 2026

View reviewed changes

ram: gemini and clang-tidy changes

b1d862c

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

ram: fixed logger error numbers

c5dad29

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

rovinski mentioned this pull request Apr 25, 2026

ram: Add proper support for RAMs with 1 or more r, w, and rw ports #10259

Open

braydenlouie marked this pull request as ready for review April 25, 2026 16:35

rovinski reviewed Apr 27, 2026

View reviewed changes

gemini-code-assist Bot reviewed Apr 27, 2026

View reviewed changes

ram: pr review changes without 1-word support

817492d

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

gemini-code-assist Bot reviewed Apr 27, 2026

View reviewed changes

ram: gemini review changes

8b0e38e

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

ram: added error for 1-word RAM cases

67d3a35

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

ram: updated ng45 7x7 ok file

52390c6

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

braydenlouie added 2 commits June 1, 2026 20:10

Merge branch 'master' into multi-port

3a3375f

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

ram: message id adjustments

350d38f

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

ram: specified latch storage cell

26f832d

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

braydenlouie added 2 commits June 13, 2026 19:31

Merge branch 'master' into multi-port

7db1511

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

ram: updated ng45 lefok file for VDD pin

e30647d

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

rovinski approved these changes Jun 16, 2026

View reviewed changes

Merge branch 'The-OpenROAD-Project:master' into multi-port

9d2f47a

rovinski requested a review from maliberty June 17, 2026 20:28

eder-matheus approved these changes Jun 17, 2026

View reviewed changes

eder-matheus merged commit 613ccd5 into The-OpenROAD-Project:master Jun 17, 2026
16 checks passed

	} else if (p < w_ports) {
	} else if (p < rw_ports + w_ports) {

	addr_name = fmt::format("addr_rw{}", i - 1);
	addr_name = fmt::format("addr_r{}", i - 1);

	const int inv_col_count = std::ceil((float) total_ports / ports_per_col);
	const int inv_col_count = std::ceil(static_cast<float>(total_ports) / ports_per_col);

Conversation

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

Uh oh!

Summary

Type of Change

Impact

Verification

Related Issues

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Apr 25, 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 Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Apr 25, 2026

Uh oh!

github-actions Bot commented Apr 25, 2026

Uh oh!

braydenlouie commented Apr 25, 2026

Uh oh!

rovinski Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

rovinski Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

rovinski commented Apr 27, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

braydenlouie Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

rovinski Apr 28, 2026

braydenlouie commented Apr 25, 2026 •

edited

Loading

braydenlouie commented Jun 1, 2026 •

edited

Loading

braydenlouie commented Jun 2, 2026 •

edited

Loading