Repository navigation
Conversation
There was a problem hiding this comment.
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.
54139ee to
aa48664
Compare
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>
aa48664 to
88c0590
Compare
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 0 with regression(s), 0 without a comparable baseline. |
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>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
make memoryprinted the inferred-memory table of the last synthesis frommem.json. It is obsolete: nothing in ORFS documents or calls it, and itsmem_hierarchical.jsonbranch has read a file nothing writes since hierarchical synthesis moved to estimates (d07d6d9). What is in use is theSYNTH_MEMORY_MAX_BITScheck, which stops a doomed synthesis early.That check needed
mem.json, a JSON dump of the whole design written on every synthesis, only so thatmem_dump.pycould find the memories in it.synth.tclalready loops over the same memories in Tcl forSYNTH_MOCK_LARGE_MEMORIES. The check now runs in that loop,synth_memory_max_bitsinflow/scripts/memories/max_bits.tcl, andmem.json,mem_dump.pyandmake memorygo.What changes
Memory <path> has dimensions <rows> x <width> = <bits>, with, a ROMfor a ROM. Before, the line was written only withSYNTH_MOCK_LARGE_MEMORIES=1. That line andShrunk memory <path> from <rows> rows to 1are unchanged.json -o mem.jsonsorted it: a yosys backend sorts the design by name before it writes, and the passes after it depend on that order.write_json /dev/nullkeeps the sort without the file; dropping it changes results and would be its own change.mem.jsonis no longer written toRESULTS_DIR, andmake memoryis gone.Tests
flow/test/test_memory_max_bits.pyruns 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_collecton every synthesis, not only withSYNTH_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