Skip to content

synth: delete make memory, mem_dump.py and mem.json - #4750

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

oharboe wants to merge 5 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

@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 thread flow/scripts/memories/max_bits.tcl Resolved
Comment thread flow/test/test_memory_max_bits.py Resolved
@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>
@oharboe oharboe changed the title synth: remove obsolete make memory report and clean up code synth: delete make memory, mem_dump.py and mem.json Oct 29, 2026
@openroad-ci

openroad-ci commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

🔍 QoR check

Metrics reflect the PR merge build — i.e. what will land on the target branch.

Commit aab49b2 · Jenkins build #8 · Baseline: build · View build on dashboard

62 design(s) checked — 0 with regression(s), 0 without a comparable baseline.

oharboe and others added 3 commits November 2, 2026 18:20
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
A failure for another reason, a yosys syntax error say, raised an
IndexError that hid yosys's output; assertIn shows it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe
oharboe marked this pull request as ready for review November 2, 2026 17:46
@oharboe
oharboe requested a review from maliberty November 2, 2026 17:47
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.

2 participants