Skip to content

synth: remove obsolete make memory report and clean up code - #4750

Draft
oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:retire-mem-dump
Draft

oharboe wants to merge 2 commits into
The-OpenROAD-Project:masterfrom
oharboe:retire-mem-dump

Conversation

@oharboe

@oharboe oharboe commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

make memory printed the inferred-memory table of the last synthesis from mem.json. It is obsolete: nothing in ORFS documents or calls it, and its mem_hierarchical.json branch has read a file nothing writes since hierarchical synthesis moved to estimates (d07d6d9). What is in use is the SYNTH_MEMORY_MAX_BITS check, which stops a doomed synthesis early.

That check needed mem.json, a JSON dump of the whole design written on every synthesis, only so that mem_dump.py could find the memories in it. synth.tcl already loops over the same memories in Tcl for SYNTH_MOCK_LARGE_MEMORIES. The check now runs in that loop, synth_memory_max_bits in flow/scripts/memories/max_bits.tcl, and mem.json, mem_dump.py and make memory go.

What changes

  • Every synthesis logs each inferred memory, Memory <path> has dimensions <rows> x <width> = <bits>, with , a ROM for a ROM. Before, the line was written only with SYNTH_MOCK_LARGE_MEMORIES=1. That line and Shrunk memory <path> from <rows> rows to 1 are unchanged.
  • A read/write memory over the limit stops synthesis with an error that names each one, its size and the limit. The old error printed the limit where the size belonged.
  • A ROM is not counted, as in synth: a ROM is not counted against SYNTH_MEMORY_MAX_BITS #4744.
  • The design is still sorted where json -o mem.json sorted it: a yosys backend sorts the design by name before it writes, and the passes after it depend on that order. write_json /dev/null keeps the sort without the file; dropping it changes results and would be its own change.
  • Breaking: mem.json is no longer written to RESULTS_DIR, and make memory is gone.
  • No longer reported: the list of source files, the instance paths of each memory, and the summary from mem_dump: add summary of inferred memory usage #3855.

Tests

flow/test/test_memory_max_bits.py runs the proc in yosys on a RAM and a ROM twice its size: a RAM over the limit stops synthesis and is named, a ROM over the limit passes and is logged as a ROM, and a mock shrinks the RAM instead of stopping. Counting ROMs again fails two of the three tests.

The loop now runs memory_collect on every synthesis, not only with SYNTH_MOCK_LARGE_MEMORIES=1. CI's QoR check shows whether that moves any design.

Builds on #4744 and is a draft until it merges.

🤖 Generated with Claude Code

SYNTH_MEMORY_MAX_BITS is the AUTO_MEMORIES policy's tripwire: a
read/write memory larger than it belongs in a macro (AUTO_MEMORIES,
fakeram) or a mock, and must not go undetected into flip-flops.
mem_dump.py counted ROMs too. A ROM, a $mem_v2 with no write port,
holds constants that synthesis maps to logic like any other logic; it
is not storage a RAM would replace, and it goes through the normal
flow. Admitting a large ROM meant raising the limit, which let
read/write memories of that size through as well.

coralnpu's FPU divider has one (The-OpenROAD-Project#4653): its SRT bound table, 128 x 83 =
10,624 bits, which synthesises on its own in about 3 s to about 700
cells on asap7. ROMs are rare, so they get no limit of their own: they
are not counted. The table still lists every memory, a ROM marked as
one, and variables.yaml says so under SYNTH_MEMORY_MAX_BITS.

test_mem_dump.py checks that a read/write memory over the limit stops
synthesis and that a ROM of the same size does not.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request replaces the Python-based memory size reporting with an early-failing Tcl check during synthesis, ensuring that ROMs (read-only memories) are excluded from the SYNTH_MEMORY_MAX_BITS limit. It also updates the documentation and adds a comprehensive test suite. The review feedback is highly constructive, pointing out a potential crash in the Tcl script if path parsing fails, and suggesting an assertion in the test suite to prevent an uninformative IndexError when tests fail unexpectedly. Both recommendations should be addressed to improve robustness.

Comment on lines +15 to +17
set index [string first "/" $path]
set module [string range $path 0 [expr { $index - 1 }]]
set instance [string range $path [expr { $index + 1 }] end]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If select -list returns any unexpected output (such as warnings, info messages, or empty lines), the path may not contain a / character. In such cases, string first will return -1, which causes string range to extract invalid module and instance names, leading to a hard crash in rtlil::get_param. Adding a defensive check to skip paths without a / ensures robustness against unexpected output.

    set index [string first "/" $path]
    if { $index == -1 } {
      continue
    }
    set module [string range $path 0 [expr { $index - 1 }]]
    set instance [string range $path [expr { $index + 1 }] end]

def test_ram_over_limit_stops(self):
returncode, output = self.run_check(2048)
self.assertNotEqual(returncode, 0, output)
error = output.split("over SYNTH_MEMORY_MAX_BITS=2048")[1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If the test fails for an unexpected reason (e.g., Yosys syntax error or file not found), the output will not contain the string "over SYNTH_MEMORY_MAX_BITS=2048". Attempting to split and access index 1 will then raise an IndexError: list index out of range, masking the actual failure output. Adding an assertion to verify the presence of the error message before splitting ensures a clear and helpful test failure message.

Suggested change
error = output.split("over SYNTH_MEMORY_MAX_BITS=2048")[1]
self.assertIn("over SYNTH_MEMORY_MAX_BITS=2048", output)
error = output.split("over SYNTH_MEMORY_MAX_BITS=2048")[1]

@oharboe
oharboe force-pushed the retire-mem-dump branch 2 times, most recently from 54139ee to aa48664 Compare October 27, 2026 20:25
make memory printed the inferred-memory table of the last synthesis
from mem.json. Nothing in ORFS documents or calls it, and its
mem_hierarchical.json branch has read a file nothing writes since
hierarchical synthesis moved to estimates (d07d6d9). What is in use
is the SYNTH_MEMORY_MAX_BITS check, which stops a doomed synthesis
early.

That check needed mem.json, a JSON dump of the whole design written on
every synthesis, only so that mem_dump.py could find the memories in
it. synth.tcl already loops over the same memories in Tcl for
SYNTH_MOCK_LARGE_MEMORIES. The check now runs in that loop,
synth_memory_max_bits in flow/scripts/memories/max_bits.tcl, and
mem.json, mem_dump.py and make memory go.

Every synthesis now logs each inferred memory with its dimensions, a
ROM marked as one; the log lines are the ones the mock already wrote.
A read/write memory over the limit stops synthesis with an error that
names each one, its size and the limit. A ROM is not counted.

The design is still sorted where json -o mem.json sorted it: a yosys
backend sorts the design by name before it writes, and the passes after
it depend on that order. write_json /dev/null keeps the sort without the
file; dropping it changes results and would be its own change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
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.

1 participant