Repository navigation
Conversation
|
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size. |
A register file, many ports over few words, is not an SRAM: FakeRAM's single-port model does not describe it, and synthesised to flops its mux trees are placed by a placer that does not know they are an array. AUTO_MEMORIES_REGFILES lists spec files, one per RTL module. gen_memories.py runs OpenROAD's generate_regfile on each: it checks the spec against the module's ports and writes the cells (<m>.v), an abstract whose pins sit where their connections land in the array (<m>.lef) and a timing model (<m>.lib, <m>_pre_layout.lib) beside the FakeRAM views. The module joins blackboxes.txt, so synthesis and macro placement see a macro, and memories.json records it with kind `regfile` (the others get `fakeram` or `flops`). In `mode netlist` the macro dissolves into its cells at the end of macro placement (regfile_dissolve.tcl): the core FIRM where the placer put the macro, flipped as it was, row parity kept; the address decode left to global placement and the resizer; the core's own cells and nets dont_touch except the flops (CTS rewires them) and the cells on the core's boundary (repair_design buffers their nets); then eliminate_dead_logic, as at synthesis. `mode macro` keeps the macro. designs/asap7/regfile is the smallest example: one 16x8 2R2W file in a parent of flops, seconds to global route. Moving parts into OpenROAD, the dissolve in particular, is left for later: faster, unit-testable, maintained with the database it edits. Needs OpenROAD's generate_regfile: The-OpenROAD-Project/OpenROAD#11669 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds register file generation functionality (generate_regfile) to the RAM tool, introducing core generation logic, structural Verilog output, LEF/Liberty view generation, and RTL port checking. The build system, documentation, and tests are updated to support this feature. The review feedback identifies a missing header in views.cpp and several instances of potential undefined behavior where char values are passed directly to std::isspace, std::isdigit, and std::isalnum without being cast to unsigned char first.
A register file, many ports over few words, is not an SRAM: FakeRAM's single-port model does not describe it, and synthesised to flops its mux trees are placed by a placer that does not know they are an array. AUTO_MEMORIES_REGFILES lists spec files, one per RTL module. gen_memories.py runs OpenROAD's generate_regfile on each: it checks the spec against the module's ports and writes the cells (<m>.v), an abstract whose pins sit where their connections land in the array (<m>.lef) and a timing model (<m>.lib, <m>_pre_layout.lib) beside the FakeRAM views. The module joins blackboxes.txt, so synthesis and macro placement see a macro, and memories.json records it with kind `regfile` (the others get `fakeram` or `flops`). In `mode netlist` the macro dissolves into its cells at the end of macro placement (regfile_dissolve.tcl): the core FIRM where the placer put the macro, flipped as it was, row parity kept; the address decode left to global placement and the resizer; the core's own cells and nets dont_touch except the flops (CTS rewires them) and the cells on the core's boundary (repair_design buffers their nets); then eliminate_dead_logic, as at synthesis. `mode macro` keeps the macro. designs/asap7/regfile is the smallest example: one 16x8 2R2W file in a parent of flops, seconds to global route. Moving parts into OpenROAD, the dissolve in particular, is left for later: faster, unit-testable, maintained with the database it edits. Needs OpenROAD's generate_regfile: The-OpenROAD-Project/OpenROAD#11669 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
15f7218 to
6e815a8
Compare
A register file, many ports over few words, is not an SRAM: FakeRAM's single-port model does not describe it, and synthesised to flops its mux trees are placed by a placer that does not know they are an array. AUTO_MEMORIES_REGFILES lists spec files, one per RTL module. gen_memories.py runs OpenROAD's generate_regfile on each: it checks the spec against the module's ports and writes the cells (<m>.v), an abstract whose pins sit where their connections land in the array (<m>.lef) and a timing model (<m>.lib, <m>_pre_layout.lib) beside the FakeRAM views. The module joins blackboxes.txt, so synthesis and macro placement see a macro, and memories.json records it with kind `regfile` (the others get `fakeram` or `flops`). In `mode netlist` the macro dissolves into its cells at the end of macro placement (regfile_dissolve.tcl): the core FIRM where the placer put the macro, flipped as it was, row parity kept; the address decode left to global placement and the resizer; the core's own cells and nets dont_touch except the flops (CTS rewires them) and the cells on the core's boundary (repair_design buffers their nets); then eliminate_dead_logic, as at synthesis. `mode macro` keeps the macro. designs/asap7/regfile is the smallest example: one 16x8 2R2W file in a parent of flops, seconds to global route. Moving parts into OpenROAD, the dissolve in particular, is left for later: faster, unit-testable, maintained with the database it edits. Needs OpenROAD's generate_regfile: The-OpenROAD-Project/OpenROAD#11669 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
…cells
generate_regfile builds a register file with many read and write ports
over few words as an array of placed standard cells, from a spec that
names the module, its words and bits, its read and write ports by the
RTL's own port names, and the cells to use:
generate_regfile -spec rf.regfile -check_ports rf.v \
-verilog rf_cells.v -def rf.def -lef rf.lef -liberty rf.lib
The array is laid out as a bitcell array is: word rows by bit columns,
each tile a flop, its write select and an AND per read port; the read
word lines are decoded in a header column, the read bit lines are OR
trees in a footer row. Every cell is placed by construction. Banks,
bit folds, a registered read and a word that reads zero are spec
options.
It writes the cells as structural Verilog, their placement as DEF, an
abstract LEF and a timing model (.lib, and a _pre_layout.lib). In
`mode netlist` the address decode is left unplaced for the parent to
place and size, and the abstract's pins sit where their connections
land in the array, so a macro placer that flips the block sees where
its wires go. -check_ports refuses a spec whose ports are not the
module's.
A flow can use it as a macro, or place the macro and then replace it
with the cells, as ORFS's AUTO_MEMORIES_REGFILES does.
Tests check what the generated file computes, not how it is laid out:
case analysis on its inputs and storage, and OpenSTA's constant
propagation through its cells (src/ram/test/regfile_checks.tcl).
generate_regfile_asap7 (an 8 x 4, two-bank, 2R2W file, word 0 reading
zero): every read port reads every word, a write loads only the word it
addresses and every other word holds, the abstract and the timing model
name every port, and a spec whose ports are not the RTL module's is
refused. generate_regfile_clock_gate_asap7: a write opens the clock gate
of the word it addresses, and only that one.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
`async_reset <port> low|high` clears every stored word to zero asynchronously, as an RTL register file written `always @(posedge clk or negedge rst_n)` does: ibex's, cva6's, swerv's and coralnpu's all do. The storage flops are `cell flop_r` (an async reset/set flop, asap7 DFFASRHQNx1) with the set tied off by `cell tie_hi`; one tie, and for an active-high reset one inverter, per word in its header column, so no reset or tie net fans out past one word's bits. `reset` keeps its meaning, a port the array ignores. Read-address registers (read_latency) are not reset. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com> Tests: generate_regfile_reset_asap7 and generate_regfile_reset_high_asap7, one per polarity: every storage flop is the reset flop, its reset is asserted exactly while the reset port is active, its set is tied off, and the file reads its words once reset is released. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ports -check_ports read only firtool's headers: `module m(` with numeric widths, and it skipped any port declaration it could not read. ibex's register file is `module ibex_register_file_ff #(parameter ...) (` with `[DataWidth-1:0]` widths, so the check could not find it, and a skipped port is a check that passes without checking. The check now reads a `#(...)` parameter list, evaluates widths in its parameters' integer defaults (+ - * / and parentheses), multiplies packed dimensions, and stops on a declaration it cannot read. Comments are stripped by a linear scan from the header on, not std::regex, which recurses per character on megabyte netlists. `unused <port>...` names one-bit inputs the module has and the array ignores (ibex's test_en_i and, with DummyInstructions off, dummy_instr_id_i): the block gets the port, connected to nothing. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com> Test: generate_regfile_sv_ports_asap7: a parameterized module header with an unused input is accepted; the same module with a wider parameter is refused, naming the port and both widths; a typed declaration is refused, quoted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two write ports writing one word in one cycle had their data ORed: right for an RTL that never does it (swerv asserts so), wrong for one that does. cva6's superscalar commit can retire two instructions writing one register, and its register file lets the later commit port win. `write_priority last` masks each write select by the later ports' (an inverter, an AND and an OR per port per word, in the word's header); the clock gate still opens on any write. `or` stays the default. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com> Test: generate_regfile_write_priority_asap7: both ports writing one word load the later port's data; writes to different words load their own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6e815a8 to
96dd5ba
Compare
Port gained bank_addr and bank_data for read_banked, and the
positional Port{addr, data, en} initializers in ReadSpec now trip
-Wmissing-field-initializers under -Werror in the bazel build.
Designated initializers name the fields and leave the bank vectors
empty, so a new field does not break them again.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
The Bazel build's clang fails with -Wmissing-designated-field-initializers under -Werror when a designated initializer leaves out fields of Port (en, bank_addr, bank_data). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
There was a problem hiding this comment.
I ran this but found many instances are unplaced
There was a problem hiding this comment.
Thanks for running it. Those are intended. In mode netlist (which this test uses) the generator fixes the core, the storage flops and read bitlines, and leaves the periphery (address inverters, decode, hold) unplaced for the parent's placer and resizer, which alone know the drivers and loads outside the array: 96 of 320 instances in this 8×4 test. mode macro places everything. I've added a commit so it says so (RAM-0051), the README describes what each mode places, and tests check both modes.
Why offer the choice at all: it's complicated, and no one shape wins. riscv32i, a few minutes per run (one run each):
| riscv32i | period vs flops | std-cell area | power |
|---|---|---|---|
| flops (today) | 907 ps | 1,289 µm² | 31.1 mW |
| inlined (every cell placed by global placement) | +1.6 % | −24 % | −23 % |
mode netlist |
+4.9 % | −22 % | −22 % |
mode macro |
does not place (MPL-0003: edge pins need channels the floorplan doesn't have) |
On a XiangShan tile, with 1.3 M register-file cells, the order flips: inlining is 7.7 % slower than placed arrays, with congestion up from 21.5 to 29.2 %. Placing the arrays makes global route somewhat faster there, but today it makes the floorplan stage slower, so build time is a cost to weigh too.
So I expect this to end up a per-design, per-project choice, decided by QoR, build time and how much design-space exploration the project does. These modes are the options to explore that with.
Until this is all in OpenROAD and ORFS, running it yourself is of course tedious. This PR is one of several needed before there's an out-of-the-box ORFS + OpenROAD flow that people who know placement better than I do can run with fast turnaround and give direction on.
There was a problem hiding this comment.
I don't think this is a good strategy. The general placer will not pack things tightly in the way a tool like this can.
There was a problem hiding this comment.
If you'd like this option removed, I'll remove it. But some data suggests it's worth keeping for now: it lets us study the differences, and on small designs leaving placement to global placement can come out ahead.
On packing you're right: on riscv32i the generator's cells fill 70 % of the array's box. On a XiangShan tile, inlining the register files (every cell placed by global placement) was 7.7 % slower than placed arrays, with global-route congestion up from 21.5 % to 29.2 %.
On a small, dense core, though, what decides it is the macro boundary rather than packing. riscv32i at global route, 950 ps clock, one run each (the flow is deterministic):
| riscv32i | min period | core µm² | std cells µm² |
|---|---|---|---|
| flip-flops | 954 | 4012 | 1284 |
| register file as a macro, 62 % (riscv32i's own) | MPL-0003, does not place | – | – |
| register file as a macro, 55 % (smallest that places) | 959 | 4644 | 980 |
| register file inlined, 62 % | 955 | 3544 | 973 |
At the same period, inlining fits a 24 % smaller core. Placed as a macro, the array doesn't place at all at the design's own utilisation.
This is one small design, so I don't know yet where the crossover lies between this and XiangShan. Keeping the choice is what lets us find out. Every configuration we tried, including the ones that lost: https://github.com/The-OpenROAD-Project/bazel-orfs/blob/regfile/auto-memories/test/regfile/campaign/results.md, with a summary in https://github.com/The-OpenROAD-Project/bazel-orfs/blob/main/ideas/regfile-campaign.md.
There was a problem hiding this comment.
Register file as macro takes more area than flip-flops or inlined? That seems dubious and needs more explanation.
There was a problem hiding this comment.
You're right, it was dubious. I measured where the area goes, and it is not the macro as such: the abstract the generator writes is loose.
riscv32i, 62 %, current head, at floorplan:
| core µm² | register file as sized by the floorplan | |
|---|---|---|
| inlined | 3544 | its cells, ~520 µm² |
macro, then dissolved (mode netlist) |
4319 | the abstract, 996 µm² |
The abstract is 53.5 × 18.6 µm, but the cells in it total 478 µm² (48 %). The rest is a decode column that mode netlist leaves to the parent anyway (225 µm²), row ends and service columns (198 µm²), and tile slack (95 µm²). initialize_floorplan -utilization charges the whole box, and that accounts for 96 % of the larger core. Halos and row cuts contribute almost nothing (13 µm²). At global route, both shapes reach the same period (954 ps at a 950 ps clock; flops 951).
So the dense packing you describe isn't happening yet: the generator pads its own outline. I'll tighten the abstract to what is placed and size the floorplan by cell area.
On whether macro placement makes sense here: on a design the size of riscv32i, inlining is the shape to use, and the numbers above show why. The macro shapes are for large designs. I'm testing them on XiangShan in bazel-orfs as part of that campaign, where the ORFS side also needs work: RTL MP placing banks of memories sensibly. These options get compared there.
…ts it In mode netlist the generator fixes the core (storage flops and read bitlines) and leaves the periphery (address inverters, decode, hold) unplaced for the parent's placer and resizer, which alone know the drivers and loads outside the array. On its own, such a block shows those cells unplaced and said nothing about why. RAM-0051 now reports how many it left and how many it fixed, the README says what each mode places, and the tests check it: generate_regfile_asap7 (mode netlist) that every storage flop is fixed and the periphery left unplaced, and generate_regfile_modes_asap7 that mode macro places every instance. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
RAM-0051 is generate_regfile's "needs -spec." error in ram.tcl, so the mode netlist info message that reports the unplaced periphery takes the next free id. Fixes the messages_txt duplicate-id check. Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
A register file, many ports over few words, is not an SRAM: FakeRAM's single-port model does not describe it, and synthesised to flops its mux trees are placed by a placer that does not know they are an array. AUTO_MEMORIES_REGFILES lists spec files, one per RTL module. gen_memories.py runs OpenROAD's generate_regfile on each: it checks the spec against the module's ports and writes the cells (<m>.v), an abstract whose pins sit where their connections land in the array (<m>.lef) and a timing model (<m>.lib, <m>_pre_layout.lib) beside the FakeRAM views. The module joins blackboxes.txt with the area its .lib states, so synthesis and macro placement see a macro that SYNTH_MINIMUM_KEEP_SIZE can cost, and memories.json records it with kind `regfile` (the others get `fakeram` or `flops`). In `mode netlist` the macro dissolves into its cells at the end of macro placement (regfile_dissolve.tcl): the core FIRM where the placer put the macro, flipped as it was, row parity kept; the address decode left to global placement and the resizer; the core's own cells and nets dont_touch except the flops (CTS rewires them) and the cells on the core's boundary (repair_design buffers their nets); then eliminate_dead_logic, as at synthesis. `mode macro` keeps the macro. designs/asap7/regfile is the smallest example: one 16x8 2R2W file in a parent of flops, seconds to global route. Moving parts into OpenROAD, the dissolve in particular, is left for later: faster, unit-testable, maintained with the database it edits. Needs OpenROAD's generate_regfile: The-OpenROAD-Project/OpenROAD#11669 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@maliberty ready for review. The red pr-merge is on origin/master, not this PR: the only failure is |
…ells AUTO_MEMORIES_REGFILES lists register-file specs; OpenROAD's generate_regfile builds each as placed standard cells during canonicalization. In mode netlist a file is a macro through synthesis and macro placement and then dissolves into its cells. AUTO_MEMORIES_MACRO_PLACE names the files placed as macros; the others are inlined in synthesis. Carried: - OpenROAD 0010: The-OpenROAD-Project/OpenROAD#11669, its net diff at its head, as one patch. - ORFS 0089, 0091, 0099: the three commits of The-OpenROAD-Project/OpenROAD-flow-scripts#4651 as rebuilt on #4650 (blackboxes.txt lines carry each register file's area), without its example designs (asap7/regfile, asap7/riscv32i-regfile, src/regfile). They apply after 0090 (#4650, #1205). 0090 loses the hunks it carried for the retired 0071 (its register files' DEF die area); the rest of it is unchanged. STRUCTURED_MEMORIES and STRUCTURED_PLACEMENT (ORFS patches 0071, 0078, 0082, 0084) are retired. A flow that still sets either one fails and the error names the replacement: at load time in arguments or sources, and at analysis in user_arguments or on a stage rule. test/structured_netlist, test/planned_parent, the XiangShan flows, tools/structured_gen/test and tools/macro_select move to the new variables. test/regfile is the flow's own fixture: the dissolve, every register clocked (macro and inlined), global route, simulation against the RTL, and synthesis with SYNTH_MINIMUM_KEEP_SIZE=1. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
This PR confuses me as it seems to completely duplicate the ram generator functionality but in a less portable way. The ram generator IS a register file generator. The current ram generator code already supports most of the options here. The only thing missing for multi-port support is to instantiate multiple decoders. Having a reset for an RF is bad design and is not supported for performance reasons. This would imply that an architecture is reading from a register before writing to it first, which makes no sense. Even if this was really required, there should be a software routine or FSM to write zeros to each register on startup. Further, this PR appears to only support a FF-based regfile and not a latch-based regfile, which is adding area and power overhead for no real gain. |
|
Thanks, Austin. Point taken: this belongs in the ram generator, not beside it. That's what I'm grinding on now. TL;DR
Planned PRs, and the use case behind each
On reset. It's opt-in, and nothing changes for designs that don't ask for it. Before treating it as a mistake I looked at why these designs reset:
I agree a software routine or FSM can zero the registers. Supporting these designs as they are written lets them use the generator without changing their RTL. Only the reset flop and its per-bit value are inside the core; polarity and synchronous reset stay in the surrounding logic. On latches. |
|
Closing in favour of the generate_ram options above; the single-concern PRs will link back here. |
That's a real doozy and I will just say that any RAM with that many ports is tricky. I think pretty much any RAM generator will struggle to give you reasonable performance, even if the density is high. The current ram generator methodology will definitely not work for that many ports - you will surely run into routing congestion issues. I think a synthesized RF is the only valid option there. Otherwise, you're going to be reading a bunch of research papers and have to resort to custom logic to implement something reasonable.
I think that usually such functionality should belong in RTL and not in the generated RAM. But I guess I would be open to comparing the two implementations.
This one again is extremely tough and adds a ridiculous amount of overhead. I'm not sure if any such implementation would be compatible with the way the ram module currently works. You would probably have to add an OR onto each word select line to OR with a reset signal, you would have to mux every data input with a 0, and you would have to also OR the clock for the ICG with the reset signal - in a glitchless way. That's at least off the top of my head. I sincerely think that this is a low performance technique that we should not encourage users to do.
Not sure what this means
I don't think there is much point in having a user select the microarchitectural details. The tool should be able to automatically try strategies and select the best one depending on user constraints.
If only the ECC registers are reset then only the ECC registers should be reset. You do not want to pay the overhead of resetting every bit of every register. In practice this means the ECC FFs should be synthesized and the normal ram bits can be part of the macro.
This doesn't make sense. Architectural registers that always read 0 should simply be synthesized to tielo cells. I don't know of any architecture with a zero reg that allows writes to it. There is no reset here - just use tielo cells.
What does this mean, swerv is not a PDK |
|
Continuing in #11723: #11723 (comment) |
3 participants
generate_regfile: a register-file generator insrc/ram, built on the RAM generator. It writes a multi-port register file as a structured standard-cell array, in place of the flip-flops synthesis would make.On ORFS's asap7 cores, at global route, against the same design with flip-flops:
The integration test is The-OpenROAD-Project/bazel-orfs#1187: these OpenROAD changes and the ORFS changes that use them (
AUTO_MEMORIES_REGFILES), carried as patches, run on ORFS's own designs. Its TL;DR: The-OpenROAD-Project/bazel-orfs#1187 (comment)One commit per concern:
ram: generate_regfile: the generator. Word rows by bit columns, a write mux or an integrated clock gate per word, read select and bitline trees. It checks its ports against the RTL module it replaces.async_reset <port> low|high: storage that resets to zero, as an RTLalways @(posedge clk or negedge rst_n)register file does (ibex, swerv, cva6). The storage flop is an async reset/set flop with its set tied off, one tie per word.#(...)parameter list, widths written in parameters, packed dimensions. A declaration it cannot read stops it rather than being skipped.unused <port>names inputs the array ignores (ibex'stest_en_i).write_priority last: the later write port wins when two write one word in one cycle, as cva6's commit ports need. The default (or) is unchanged.ORFS uses it in The-OpenROAD-Project/OpenROAD-flow-scripts#4651 (
AUTO_MEMORIES_REGFILES).Tests check what the generated register file computes rather than golden files: case analysis on its inputs and storage, and OpenSTA's constant propagation through its cells (
src/ram/test/regfile_checks.tcl). They cover every read of every word, writes and holding, the clock gate per word, both reset polarities, the SystemVerilog port check and its refusals, and write priority. Each commit passes its own tests.🤖 Generated with Claude Code