Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces configuration and surgery scripts for the coralnpu design, including a new register file specification (Regfile_rf.regfile) and a Python script (surgery.py) to post-process the generated Verilog by stripping verification layers, rewriting SRAM wrappers, and optionally restructuring the register file. Feedback on the changes highlights a critical indexing bug in the simulation-only memory initialization task within surgery.py, along with recommendations to ensure the output directory is created before writing and to specify UTF-8 encoding when reading and writing files.
|
Re-measured exactly as this PR has it, without the
Period and area hold. Power does not: with the latch-and-AND clock gate the flop build's power nearly doubles (33.0 → 63.3 mW) and the register file's less (35.8 → 45.4 mW), so the power figure depends on how that one clock gate is modelled. The description now carries these numbers, with that caveat. 🤖 Generated with Claude Code |
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 9 with regression(s), 0 without a comparable baseline. ❌ asap7/chi_async_bridge base — 4 failing metric(s)
❌ asap7/ethmac_lvt base — 3 failing metric(s)
❌ asap7/gcd base — 1 failing metric(s)
❌ asap7/swerv_wrapper base — 4 failing metric(s)
❌ gt2n/jpeg base — 1 failing metric(s)
❌ nangate45/bp_be_top base — 1 failing metric(s)
❌ nangate45/tinyRocket base — 1 failing metric(s)
❌ sky130hd/jpeg base — 3 failing metric(s)
❌ sky130hs/gcd base — 2 failing metric(s)
|
asap7/coralnpu and asap7/coralnpu-regfile are in The-OpenROAD-Project/OpenROAD-flow-scripts#4653, its files the same as ORFS patches 0102 and 0103 but for black's line breaks in surgery.py and a space in constraint.sdc. The ladder stops carrying them: 0102 and 0103 go, 0104 keeps only swerv_wrapper-regfile-mux, and coralnpu's rows, its synthesis, surgery and simulation tests leave test/regfile/ladder. designs_by_directory_test checks the swerv mux variant against its base. The design DSL stops exporting .ys files: no design in the series names a SYNTH_POST_HIERARCHY_SCRIPTS script from another package since coralnpu took #4653's form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
asap7/coralnpu and asap7/coralnpu-regfile are in The-OpenROAD-Project/OpenROAD-flow-scripts#4653, its files the same as ORFS patches 0102 and 0103 but for black's line breaks in surgery.py and a space in constraint.sdc. The ladder stops carrying them: 0102 and 0103 go, 0104 keeps only swerv_wrapper-regfile-mux, and coralnpu's rows, its synthesis, surgery and simulation tests leave test/regfile/ladder. designs_by_directory_test checks the swerv mux variant against its base. The design DSL stops exporting .ys files: no design in the series names a SYNTH_POST_HIERARCHY_SCRIPTS script from another package since coralnpu took #4653's form. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
Where the flop baseline's period goes, and why the register file is the lever I took this PR's flop baseline to global route and asked where its reg2reg period goes: OpenROAD 22b7c537, ORFS ef42174 with this PR applied, built with bazel-orfs. Here it reaches 2,031 ps against the 1,900 ps clock. The description reports 1,976 ps; the OpenROAD commit and flow differ. One path family sets the period. All of the worst 200 reg2reg paths start at the register file's read data ( It is logic, not placement. Mean over those 200 paths:
The modules on the path are compact. Measured by the radius holding half of each module's cell area, against a disc at the design's density, the LSU is 0.83× and the register file 1.18×. A perfect placement would recover at most the roughly 550 ps of repeaters and wire. It is a mass, not one path. Removing the worst launching register 40 times ( This is the cycle the design chose. CoralNPU is an in-order three-stage pipeline: fetch, decode/dispatch, execute/writeback, up to 4 instructions per cycle (microarch.md). Operands are registered at the dispatch → execute boundary. Then one execute cycle does the rest: the register file adds the immediate ( Compared with the literature, the following appears to be left as an exercise for the reader:
What silicon exists. Synaptics' Astra SL2610 (12 nm) is the first production implementation of Coral NPU, "a tightly-coupled CPU optimized for Synaptics' PPA requirements", with the scalar core only; the vector and matrix units are still in development. Google offers the design as IP, with VeriSilicon productising it, and states a target of 800 MHz (1,250 ps) at 22 nm. This snapshot is the open reference design that production silicon is derived from, not a taped-out netlist; how Synaptics' implementation handled the points above is not public. What follows for this PR. The period is set by the register file's read and address logic feeding the LSU, in a cycle the design intends to close as written. Of the exercises above, the register file is the one a flow can take on without changing the RTL's behaviour, and building it as a generated array is what the second commit does (1,976 → 1,900 ps in the description). The remaining gap to the design's own target, 1,250 ps at 22 nm against 2,031 ps here on asap7, a faster node, is either implementation (how synthesis maps this one cycle) or this snapshot's configuration (instruction lanes, FPU and vector enabled), which may be heavier than the 800 MHz one. I have not separated the two. Sources: CoralNPU Chisel and docs, Google Developers Blog, EE Times, VeriSilicon, Ibex ID/EX, CV32E40P LSU. |
|
Notes for later: getting the most testing value out of CoralNPU with one timing budget These are notes on what to investigate later, written up here for now so they sit next to the design they're about. Nothing below has been tried in this PR yet: the budget, its value and its effect on WNS are proposals to check, not results. CoralNPU is a valuable addition to ORFS's asap7 set. It's an open RISC-V design with production silicon behind it (Synaptics' Astra SL2610), it's hierarchical, with this PR its SRAMs are generated macros and its register file can be a generated array, and it runs synthesis to global route in about an hour. Few designs exercise memory generation, macro placement and timing closure together at a turnaround a flow developer can iterate on. Where its WNS comes from today. Taken to global route with this PR's flop baseline, its reg2reg period is about 2,031 ps (OpenROAD 22b7c537, ORFS ef42174). That number comes from one family: the register file's read data into the LSU's input queue ( Why that family is special. CoralNPU's pipeline does the register file read, the address add and the LSU enqueue in one execute cycle (microarch.md), with an 8-read, 6-write register file written as flops. The literature resolves this cycle in one of two ways. Classic pipelines register the effective address before the memory side (the EX/MEM latch of Hennessy & Patterson's five-stage pipeline, as in Rocket and CV32E40P). Register files with this many ports are built as a structure of their own, banked or custom arrays (Rixner et al., HPCA 2000), rather than flops behind mux trees. The open RTL leaves that step to the implementer. An implementation for silicon would most likely take one of those routes. Synaptics describes its version as "optimized for Synaptics' PPA requirements"; the details aren't public. The idea: time that family against its own budget. Without touching the RTL, # The register file's read data into the LSU's input queue: one execute
# cycle in the open RTL, a cycle an implementation for silicon resolves
# (an address register, or a register file built as an array). Timed
# against its own budget, so the rest of the design sets this test
# case's WNS; the paths stay timed and repaired, and a regression shows
# as negative slack here.
set rf_read [get_cells core.score/regfile/io_readData_*]
set lsu_q [get_pins core.score/lsu/ctrl.mem_*/D]
if { [llength $rf_read] == 0 || [llength $lsu_q] == 0 } {
error "constraint.sdc: the register-file -> LSU family matched nothing"
}
set_max_delay 2100 -from $rf_read -to $lsu_qWhat it should give, to be confirmed:
Two numbers per run: WNS for the design against the clock, and the register-file family's slack against its budget. Together they would say how the flow does on the design as a whole, and how the one-cycle read-to-LSU path does on its own. With the generated register file. This PR's second commit takes one of the literature's routes for real: To investigate: whether the budget matches the paths it should and only those; what WNS the rest of the design reports with it; whether that number responds to floorplan and placement changes; and what budget value keeps the family met today while still catching regressions. |
|
@maliberty TL;DR, no action needed now:
🤖 Generated with Claude Code |
|
Results: with the EX/MEM register, coralnpu goes from 2,031 ps to 1,701 ps at global route (1,700 ps clock); with the generated register file too, it meets 1,700 ps. The worst path moves to register file → CSR Correction to my earlier analysis: the queue is 🤖 Generated with Claude Code |
|
@maliberty CoralNPU moved upstream + we've learned a bunch of things, once memory stuff + various and sundry other PRs have landed and we've discussed XiangShan, perhaps we talk about the framing for what an update of CoralNPU should look like. |
17ade36 to
32a3062
Compare
…tions OpenROAD #11723 adds the register-file options to generate_ram that AUTO_MEMORIES_REGFILES, in the next commit, asks for. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
…t by generate_ram 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. memories/regfile_ram.py translates each to OpenROAD generate_ram_netlist options (ports, banks, folds, the zero word, reset, read latency, write style and priority, cells), checks it against the module's ports from synthesis (a stray, missing or mis-sized port stops the build), and writes a wrapper in the module's place around the RAM, <m>_ram: an active-high reset inverted for an active-low flop, addresses narrowed or widened. A register file is inlined unless AUTO_MEMORIES_MACRO_PLACE names it: its cells are then placed, sized and buffered with the rest of the design. A named one is a macro to synthesis and macro placement (an abstract LEF and a liberty of its area beside the FakeRAM views, in blackboxes.txt; a parameterized module's instances lose parameters the netlist does not declare). In `mode netlist` it 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 periphery left to global placement and the resizer; then eliminate_dead_logic. `mode macro` keeps it, with pins, a power grid and a timing model from write_timing_model. Where a row holds several words they sit side by side, and a word whose bits would run more than 150 um is folded into more rows: XiangShan's RobEntryFile and RenameBufferFile, dissolved into a parent, did not legalize otherwise (the parent buffers the word's selects inside the full array). A module named in a spec whose memory has an initial value stops the build: generate_ram's storage has none, and the value would be lost without a word. The spec's read_latency, a key shared with structured_gen's specs, is generate_ram's -register_read_address. The RAMs the wrappers instantiate as macros are listed in regfile_cores.txt: they are in blackboxes.txt but no module of the sources, so ORFS's check that every AUTO_MEMORIES module is a black box after read_slang skips them. Needs generate_ram's register-file options, OpenROAD #11723. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
regfile_dissolve.tcl marks the cores of dissolved register files dont_touch at the macro step, before the PDN step defines the global connections, and global_connect skips dont_touch instances (ODB-0383, deliberate in odb). Their VDD and VSS pins stayed on no net through final: 896 of riscv32i's 7,089 instances in test/regfile, 406,272 cells on XiangShan's XSTile parent. The M1 rails touch them, so timing and routing were unaffected; the netlist, LVS and power analysis saw them unpowered. Every dont_touch instance with a supply pin on no net has its dont_touch lifted while the platform's PDN script makes its global connections, and restored after: one connection, no ODB-0383 warning per instance (406,272 on XSTile). A supply pin already on a net is left alone.
…ly mode mode macro wrote a register file's timing model (write_timing_model) from generate_ram's netlist before any clock tree was built, so the macro's clock pin was the whole unbuffered clock net: 8.5 pF for 64 words of 256 bits on asap7, 32 pF for XiangShan's 954-bit Ftq queue, whose clock then arrived 7.7 ns late in its parent and set the parent's whole clock tree. A register file wanted as a macro is a design of its own: run it through the flow, where CTS builds its clock tree, and give the parent its abstract as a macro. mode macro now stops with that advice; a spec without a mode line means mode netlist, which dissolves the register file into its parent after macro placement.
AUTO_MEMORIES replaced a parameterized memory module (coralnpu's Sram #(NUM_ENTRIES, GLOBAL_BASE_ADDR)) by its macro with the instance's parameters still on it: yosys writes `Sram #(.NUM_ENTRIES(...)) ...` into 1_2_yosys.v, and OpenROAD's Verilog reader stops on it (STA-0171, syntax error). The macro is the module at the widths detection sized it, as the inlined register files already are: drop the parameters from the instances of both, through one proc. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
32a3062 to
e6a6657
Compare
flow/designs/src/coralnpu/generate is a Bazel module that writes
CoreMiniAxi.sv from coralnpu's Chisel source at a pinned commit:
cd flow/designs/src/coralnpu/generate
bazelisk run //:update
It elaborates coralnpu's CoreAxi as upstream's CoreMiniAxi build
configures it (128-bit fetch and load/store, float with Zfbfmin), in a
wrapper that ties the test enable te to 0, so synthesis removes the
test-mode reset bypasses and clock-gate overrides, and without the debug
ports a synthesis build does not use. firtool runs without the
verification layers and without register randomization. The dependency
pins and coralnpu's patches to cvfpu are coralnpu's own.
The module is in a directory of its own, so flow/designs/src/coralnpu
holds no Bazel package. CoreMiniAxi.sv is regenerated in the next commit,
with the design changes the new RTL needs.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
CoreMiniAxi.sv is regenerated from coralnpu b50b85a9 by src/coralnpu/generate, and the design is configured for it: - The five TCM banks (ITCM one, DTCM four) are coralnpu's Sram, 512 x 128 with one read-write port and a 16-lane byte mask. sram.memories names its pins (ADDITIONAL_MEMORIES) and AUTO_MEMORIES replaces it with a generated macro, write mask and all. The previous build swapped in USE_ASAP7 fakerams, which have no write mask, so a masked write wrote all 16 bytes. - Regfile and FRegfile keep their words as separate registers, with no array for yosys to infer. surgery.py (SYNTH_VERILOG_SURGERY) moves them into Regfile_rf and FRegfile_rf, which AUTO_MEMORIES_REGFILES has generate_ram build as standard cells, every word with its asynchronous reset: Regfile 32 x 32 with x0 reading zero, nine reads and six writes; FRegfile 32 x 32, four reads and two writes. - The clock is 1,900 ps in the platform's picoseconds (it was 10.00), with the platform's set_max_delay IO budgets; the asynchronous reset is not timed. The kept hierarchy is coralnpu's units by name, with OPENROAD_HIERARCHICAL=1. - The divider's 128 x 83 SRT table is a ROM yosys maps to logic. SYNTH_MEMORY_MAX_BITS counts ROMs today and stops synthesis on it; the design builds once a ROM is no longer counted, a separate change. surgery.py recognises each register file token for token and stops the build on anything else. Each was simulated before and after the surgery, and generate_ram's netlists against the behavioural modules the surgery writes, on the same random inputs, every output compared every cycle, with a negative control for each that is caught. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
e6a6657 to
818b99e
Compare
A SYNTH_KEEP_MODULES name that matched no module was a yosys warning and nothing else: synthesis went on with that module flattened. A misspelled name, or a module the RTL has renamed, changed the result without a word. That is how asap7/coralnpu's keep list went stale in The-OpenROAD-Project#4653: the current RTL renamed DispatchV1, LsuV1 and DBus2AxiV1 to DispatchV2, LsuV3 and DBus2AxiV2, and a full flow ran with three of its ten units flattened before anything noticed. Each name now has to select a module, by either of the two patterns already used (the name itself, or slang's uniquified name$...); synthesis stops otherwise, naming it. On asap7/riscv32i a misspelled name stops synthesis and the real one passes; asap7/chi_async_bridge, whose kept modules have slang's uniquified names, passes unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
coralnpu on asap7 at its current RTL, generated from coralnpu's source, with its SRAMs and both register files generated and the function unchanged.
Draft: the series may still be rewritten before it is marked ready for review.
Built through final at a 1,900 ps clock in about 90 minutes, hermetically with bazel-orfs on this PR's OpenROAD (The-OpenROAD-Project/bazel-orfs#1415), with a baseline for scale:
The baseline is ballpark numbers only, not a controlled comparison. It is master's asap7/coralnpu built through global route with the same tools as this PR, not with master's OpenROAD and yosys. Its 10 ps clock sets every repair step chasing an impossible target, which likely accounts for much of its CTS and global-route time: at global route,
repair_timingalone takes 605 s. This PR's design is the more congested of the two at global route.The worst path at the end runs from the debug module's request queue through the CSRs, the float core and the load/store unit to the retirement buffer. The generated register files' write enables close with 36 ps to spare.
Depends on #4744, not in this PR: the FPU divider's SRT bound table is a 128 × 83 ROM (10,624 bits) that yosys maps to logic.
SYNTH_MEMORY_MAX_BITScounts ROMs today and stops synthesis on it. The limit is there so that a read/write memory, which belongs in a macro or a mock, does not go undetected into flip-flops; a ROM holds constants and should go through the normal flow. Raising the limit to admit the ROM would let read/write memories of that size through too, so ROMs stop being counted instead: #4744. The build above carries that change.#4745 makes a
SYNTH_KEEP_MODULESname that is not in the design stop synthesis: this design's keep list went stale against the renamed RTL unnoticed.Commits:
tools/OpenROAD: bump to a commit with generate_ram's register-file options (OpenROAD #11723).AUTO_MEMORIES_REGFILES: register files listed by spec are built by generate_ram from placed standard cells. Each is inlined in the RTL module's place, or macro-placed and dissolved into its cells after macro placement.mode macrostops;mode netlistis the only mode.Sram #(NUM_ENTRIES)) lose their parameters when it becomes a macro. Before this, the netlist wrote them out and OpenROAD's Verilog reader stopped on it.src/coralnpu/generate, that writesCoreMiniAxi.svfrom coralnpu's Chisel source at a pinned commit (bazelisk run //:update).AUTO_MEMORIES. The earlierUSE_ASAP7fakerams had no write mask, so a masked write wrote all 16 bytes.surgery.pymovesRegfile's andFRegfile's words into modules of their own.generate_rambuilds both as standard cells, every word with its asynchronous reset.How the function was checked:
surgery.pyrecognises each register file token for token and stops the build on anything else. Each register file was simulated before and after the surgery on the same random inputs, every output compared every cycle. The run was 200,000 cycles, with resets and with addresses biased to collide; there were no mismatches. Negative controls were caught within 20 cycles: a write port's mask dropped, and the write priority swapped.🤖 Generated with Claude Code