Skip to content

asap7/coralnpu: current RTL from source, SRAMs and register files generated - #4653

Closed
oharboe wants to merge 5 commits into
The-OpenROAD-Project:masterfrom
oharboe:coralnpu-showcase
Closed

oharboe wants to merge 5 commits into
The-OpenROAD-Project:masterfrom
oharboe:coralnpu-showcase

Conversation

@oharboe

@oharboe oharboe commented Oct 8, 2026 •

Copy link
Copy Markdown
Collaborator

coralnpu on asap7 at its current RTL, generated from coralnpu's source, with its SRAMs and both register files generated and the function unchanged.

It uses three changes, now on master, that wire up more of generate_ram's experimental features in the flow:

This PR is the design alone.

Built through final at a 1,900 ps clock in about 90 minutes, hermetically with bazel-orfs on OpenROAD 89e24308 (The-OpenROAD-Project/bazel-orfs#1415), with a baseline for scale:

asap7/coralnpu baseline: master's design this PR
clock target 10 ps 1,900 ps
achieved period at global route 1,917 ps 1,894 ps (+6.2 ps)
achieved period, final not run 2,008 ps (−108 ps), 0 DRC
standard cells / macros at global route 19,287 / 13,872 µm² (fakerams) 19,411 / 13,965 µm²
global route congestion iterations 1 of 30 12 of 30
floorplan 3.7 min 1.5 min
place 9.4 min 9.3 min
CTS 13.9 min 2.7 min
global route 19.3 min 9.1 min
detailed route not run 61.5 min

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_timing alone 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.

#4744 (a ROM is not counted against SYNTH_MEMORY_MAX_BITS; the FPU divider's 128 × 83 SRT table is one) and #4745 (a SYNTH_KEEP_MODULES name not in the design stops synthesis) are merged.

The numbers above were measured on OpenROAD 89e24308 with those three changes applied as patches, before they were split out and merged; they have not been rerun on master.

Commits:

  • src/coralnpu: a Bazel module, src/coralnpu/generate, that writes CoreMiniAxi.sv from coralnpu's Chisel source at a pinned commit (bazelisk run //:update).
    • It configures the core as coralnpu's own CoreMiniAxi build does.
    • The test enable is tied to 0.
    • There are no verification layers, randomization or debug ports.
  • asap7/coralnpu: coralnpu's current RTL (b50b85a9), its memories generated.
    • The five TCM banks become masked SRAM macros through AUTO_MEMORIES. The earlier USE_ASAP7 fakerams had no write mask, so a masked write wrote all 16 bytes.
    • surgery.py moves Regfile's and FRegfile's words into modules of their own. generate_ram builds both as standard cells, every word with its asynchronous reset.
    • The clock is in picoseconds; it was 10.00 on a ps platform.
    • The kept hierarchy is coralnpu's units by name, as the current RTL names them.

How the function was checked:

  • Surgery: surgery.py recognises 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.
  • generate_ram's netlists: simulated against the behavioural modules the surgery writes, in the same way, using cell models from the asap7 liberty files. There were no mismatches. Negative controls were caught: one write port's data taken from another, and the write priority swapped.

🤖 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 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.

Comment thread flow/designs/asap7/coralnpu/surgery.py Resolved
Comment thread flow/designs/asap7/coralnpu/surgery.py Resolved
Comment thread flow/designs/asap7/coralnpu/surgery.py Resolved
Comment thread flow/designs/asap7/coralnpu/surgery.py Resolved
@oharboe

oharboe commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator Author

Re-measured exactly as this PR has it, without the ClockGate mapping onto ASAP7's ICG that the first numbers used:

coralnpu, global route flops generated register file
min period 1,976 ps 1,900 ps (−3.9 %, meets the target)
std-cell area 16,563 µm² 15,512 µm² (−6.3 %)
power 63.3 mW 45.4 mW (−28 %)
stage time 3,994 s 1,112 s

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

@openroad-ci

openroad-ci commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

🔍 QoR check

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

Commit 953a6ab · Jenkins build #11 · Baseline: build · View build on dashboard

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

❌ asap7/cva6 base — 2 failing metric(s)
Metric target base delta limit band
cts__timing__setup__tns -1393.820 -276.256 -404.539% -458.256 PeriodPadding 20.000%
finish__timing__setup__tns -6939.740 -5521.570 -25.684% -6625.884 PeriodPadding 20.000%
❌ asap7/ibex base — 2 failing metric(s)
Metric target base delta limit band
finish__timing__setup__tns -1131.880 -704.768 -60.603% -884.768 PeriodPadding 20.000%
globalroute__timing__setup__tns -5389.390 -3861.920 -39.552% -4634.304 PeriodPadding 20.000%
❌ sky130hd/ibex base — 1 failing metric(s)
Metric target base delta limit band
cts__timing__setup__tns -21.382 -8.157 -162.127% -9.789 PeriodPadding 20.000%
❌ sky130hs/jpeg base — 3 failing metric(s)
Metric target base delta limit band
cts__timing__setup__tns -4.482 -0.270 -1561.146% -0.970 PeriodPadding 20.000%
finish__timing__setup__tns -7.146 -2.671 -167.558% -3.371 PeriodPadding 20.000%
globalroute__timing__setup__tns -22.099 -14.239 -55.200% -17.087 PeriodPadding 20.000%

oharboe added a commit to The-OpenROAD-Project/bazel-orfs that referenced this pull request Oct 9, 2026
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>
oharboe added a commit to The-OpenROAD-Project/bazel-orfs that referenced this pull request Oct 9, 2026
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>
@oharboe

oharboe commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator Author

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 (core.score/regfile/io_readData_*) and end in the LSU's input queue (core.score/lsu/ctrl.mem_*). 810 endpoints sit between 2,000 and 2,099 ps, with the rest of the design mostly at 1,200–1,500 ps.

It is logic, not placement. Mean over those 200 paths:

ps
logic 1,392
repeaters (about 31 per path) 481
wire 69
clock skew 26

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 (set_false_path -from the register, as a measurement in a thrown-away session, never in the SDC) moves the period only from 2,031 to 1,993 ps.

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 (busAddr = rdata + imm, priority-encoded across lanes, Regfile.scala), the LSU builds its µop from it, and the µop is enqueued in the LSU's input buffer (lsu.md, "Ingestion"). The documented load latency is 2+ cycles.

Compared with the literature, the following appears to be left as an exercise for the reader:

  1. The address register. The classic five-stage pipeline, and Rocket and CV32E40P after it, registers the effective address after address generation, before any memory-side logic: the EX/MEM latch (Hennessy & Patterson). CoralNPU puts address generation, µop construction and the enqueue in one cycle, much as Ibex does with its combined ID/EX stage. That buys a cycle of load latency and leaves this cycle's depth to the implementation.
  2. The multiported register file. The integer register file has 8 read and 6 write ports and is written as flops behind mux trees. The literature treats register files with this many ports as a structure of their own, because delay, area and energy grow with port count: banked, replicated or custom arrays (Rixner et al., "Register Organization for Media Processing", HPCA 2000; Zyuban & Kogge, "The Energy Complexity of Register Files", ISLPED 1998). Generating it as an array is that exercise, and it is this PR's second commit.
  3. Issue width in the critical cycle. Bypass and select logic grow with issue width and become the cycle-limiting paths of wide machines; the literature's answer is to partition them (Palacharla, Jouppi & Smith, "Complexity-Effective Superscalar Processors", ISCA 1997). Here the address cycle carries the 4-lane priority encoding of the bus port. The LSU's input buffer is built as a multi-enqueue buffer for all instruction lanes (CircularBufferMulti(…, p.instructionLanes, …)), while the dispatch documentation says memory instructions dispatch at most one per cycle (dispatch.md). Whether synthesis removes the enqueue lanes that never carry data is not measured here.
  4. Retiming. Retiming (Leiserson & Saxe, "Retiming Synchronous Circuitry", Algorithmica 1991) moves existing registers; it cannot add a stage. The RTL has no empty pipeline stages for it to fill. The register file's output registers hold their value through a feedback mux, which blocks moving them. The LSU's one mention of retiming is a designer's TODO for a manual change (// TODO: if timing violation, retime this). Retiming in synthesis has nothing to work with on this path.

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.

@oharboe

oharboe commented Oct 10, 2026

Copy link
Copy Markdown
Collaborator Author

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 (core.score/regfile/io_readData_* → core.score/lsu/ctrl.mem_*), 810 endpoints within 100 ps of the worst. Below it, about 1,200–1,500 ps of paths across fetch, dispatch, the FPU and the bus bridges. Whether floorplanning, macro placement, global placement and timing repair show there is one of the things to investigate.

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, constraint.sdc could stand in for that implementation step, timing these paths against a budget of their own, just above where they are today, while the rest of the design is timed against the clock. A sketch, untested:

# 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_q

What it should give, to be confirmed:

  • An idiomatic WNS. The reported number would be the one the design has once that cycle is resolved, as an implementation for silicon would resolve it. Floorplanning, macro placement, global placement and repair would then move it, instead of sitting under one family.
  • Nothing unoptimised or unwatched. Unlike an exception that removes paths from timing, set_max_delay is an optimisation target. The paths stay in placement and repair against their budget, a regression shows as negative slack, and an improvement (a generated register file, a better-mapped address add) shows as positive slack.
  • No change to the RTL. The snapshot stays byte-identical to upstream; the budget lives in the SDC beside the clock.
  • A rename fails loudly. The guard turns a moved instance name into an error instead of a silently empty constraint. It relies on this PR's kept modules (Regfile, LsuV1) and OPENROAD_HIERARCHICAL=1.
  • Repair effort goes where it can help. This PR's description notes how much of the flop build's CTS and global route goes into this one family.

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: coralnpu-regfile builds the register file as an array. Its slack against the budget would then measure what the array buys, while the design's WNS keeps tracking everything else. coralnpu and coralnpu-regfile side by side could make a compact regression set: memory generation, register-file generation, macro placement and timing closure, in one fast design.

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.

@oharboe

oharboe commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

@maliberty TL;DR, no action needed now:

  • The coralnpu snapshot is stale. CoreMiniAxi.sv (firtool 1.114, added 2026-08-14) records no upstream commit or recipe. Its worst timing path, register-file read to the LSU's input queue, is one upstream has since reworked in Chisel (43d2030 and an LSU timing series, targeting 800 MHz in 12 nm). Problems measured on this snapshot may not exist upstream.
  • In lieu of a fresh upstream snapshot, I'm testing surgery that resolves this ghost WNS outlier the textbook way: an EX/MEM address register on the LSU's inputs, so the rest of the design sets the WNS. Results to follow here.
  • coralnpu isn't in CI. It has no rules-base.json, so nothing depends on it today.
  • Upstream takes outside fixes slowly. It imports outside PRs, and 15 of 37 have landed. Several RTL fixes have waited months with no reply.
  • This PR stays a draft until various and sundry AUTO_MEMORIES PRs land. It carries data for a later decision on coralnpu in ORFS: the baseline fixes, register file vs flops, and where the period goes.
  • One option for later: regenerate the Verilog from a pinned coralnpu commit and check it in, so testers need neither Bazel nor Chisel.

🤖 Generated with Claude Code

@oharboe

oharboe commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator Author

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 minstret. Each EX/MEM design now has its own clock just under what it reaches. Details in the description and the new README.

Correction to my earlier analysis: the queue is FifoX, not CircularBufferMulti, and the path enters it through dispatch's lane issue, not the address.

🤖 Generated with Claude Code

Comment thread flow/designs/asap7/coralnpu/config.mk Resolved
@oharboe

oharboe commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

@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.

@oharboe oharboe closed this Oct 18, 2026
@oharboe oharboe reopened this Oct 26, 2026
@oharboe oharboe changed the title asap7/coralnpu: register file generated, the snapshot unchanged asap7/coralnpu: SRAMs and register files generated, the function unchanged Oct 26, 2026
@oharboe oharboe changed the title asap7/coralnpu: SRAMs and register files generated, the function unchanged [DO NOT REVIEW] asap7/coralnpu: SRAMs and register files generated, the function unchanged Oct 26, 2026
@oharboe oharboe changed the title [DO NOT REVIEW] asap7/coralnpu: SRAMs and register files generated, the function unchanged asap7/coralnpu: current RTL from source, SRAMs and register files generated Oct 27, 2026
@oharboe oharboe changed the title asap7/coralnpu: current RTL from source, SRAMs and register files generated [DO NOT REVIEW] asap7/coralnpu: current RTL from source, SRAMs and register files generated Oct 27, 2026
oharboe added a commit to oharboe/OpenROAD-flow-scripts that referenced this pull request Oct 27, 2026
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>
maliberty pushed a commit that referenced this pull request Nov 2, 2026
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 (#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>
@maliberty

Copy link
Copy Markdown
Member

what is your plan with this DNR PR?

@oharboe

oharboe commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

what is your plan with this DNR PR?

I was going to mark it as ready for review when everything all needed PRs in OpenROAD and ORFS are merged. I have so many other PRs open that I didn't think to check if we are there at this point.

This has been a very useful smaller than XiangShan design for me to iterate on when working on XiangShan, so I wanted to offer it to ORFS community in case it proved helpful.

@oharboe

oharboe commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Split out three changes, independent of each other, that wire up more of generate_ram's experimental features in the flow:

#4653 is now coralnpu alone, rebased on master, and stays a draft until those merge; then the numbers are rerun on master and it goes up for review. The OpenROAD it needs is already in master's tools/OpenROAD, and #4744/#4745 are merged.

oharboe and others added 2 commits November 3, 2026 09:56
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.

coralnpu's ClockGate.sv and Sram.v carry an `ifdef branch per foundry
PDK, naming cells and macros of proprietary PDKs. resources.py removes
those branches before firtool sees the files, so CoreMiniAxi.sv keeps
the FPGA and generic branches as upstream wrote them; a file with no
such branch to scrub stops the build. The scrubber assembles the names
from fragments so that it does not itself trip ORFS's security scan,
which still runs on every file, the generated Verilog included.

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; a
  ROM is not counted against SYNTH_MEMORY_MAX_BITS.

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.

Needs AUTO_MEMORIES_REGFILES (The-OpenROAD-Project#4781), the supply pins of dont_touch
instances connected (The-OpenROAD-Project#4782) and a parameterized module as a macro
(The-OpenROAD-Project#4783).

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>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
@oharboe oharboe changed the title [DO NOT REVIEW] asap7/coralnpu: current RTL from source, SRAMs and register files generated asap7/coralnpu: current RTL from source, SRAMs and register files generated Nov 6, 2026
@oharboe
oharboe marked this pull request as ready for review November 6, 2026 15:42
@oharboe
oharboe requested a review from maliberty November 6, 2026 15:43
@maliberty

Copy link
Copy Markdown
Member

Review drafted by Claude (AI) at maliberty's request, focused on how coralnpu is integrated from upstream rather than on the design itself.

The weakest part of this PR isn't the design. It's how the RTL is pulled from upstream: the PR keeps its own copy of coralnpu's Chisel build, and nothing records or checks what that copy produces.

Context

  • The pin is out of date. The PR pins upstream commit b50b85a9 (2026-10-26). Upstream main is 45 commits ahead.
  • Upstream now has its own ORFS flow. On 2026-10-28 it added synthesis/rvv_core_mini/ (ASAP7 config.mk, SDC, emit_rtl.sh) and synthesis/BUILDING_CORALNPU_WITH_OPENROAD.md. That flow builds RvvCoreMiniAxi, the vector variant, not ORFS's CoreMiniAxi. It calls upstream's own Bazel emit target, does not check in the generated .sv, and still uses fakerams. ORFS and upstream now maintain two separate asap7 coralnpu setups that don't know about each other. Before merging, it's worth deciding which upstream top ORFS should track, and whether ORFS should use upstream's emit target rather than its own.

Must fix

  1. No SOURCE.md or LICENSES/. Other designs built from upstream RTL (src/chi_async_bridge, src/picorv32) have both. CoreMiniAxi.sv combines:

    • coralnpu's own code (Apache-2.0, Google)
    • cvfpu, common_cells and fpu_div_sqrt_mvp (Solderpad SHL-0.51, ETH Zurich)
    • rocket-chip's AsyncQueue, compiled into the Chisel output so no license header survives (Apache-2.0)

    It should get src/coralnpu/SOURCE.md (top, upstream commits, toolchain, what was changed) and src/coralnpu/LICENSES/. Master already lacks these; this PR is the natural place to add them.

  2. The output is not the RTL upstream's own build produces. The description says it configures the core "as coralnpu's own CoreMiniAxi build does", but the build itself is different:

    • Toolchain: Chisel 7.2.0 / firtool 1.133.0, where upstream uses Chisel 7.0.0-RC1 / firtool 1.114.0 (upstream MODULE.bazel, rules/repo_defs.bzl).
    • Build files: coralnpu.BUILD.bazel replaces all of upstream's BUILD files (patch_cmds deletes them) with its own source globs and exclude lists.
    • Other changes: ScmInfo is stubbed, te is tied to 0, and the debug ports are dropped.

    Each of these may be justified, but none is written down outside the Bazel comments. Every upstream bump means re-checking the globs and excludes by hand. Upstream has a bzlmod MODULE.bazel and its own emit targets. Using upstream's core_mini_axi_cc_library emit target directly, plus a small post-processing step, would be much less to maintain.

  3. surgery.py matches firtool's output text exactly. It looks for firtool-generated names such as _wdata_k_value_n_T, data_N, _GEN_0 and valid_0_N. Two consequences:

    • The README says moving to a newer coralnpu means changing the commit and running //:update. In practice almost any bump (new firtool, Chisel, or a Regfile.scala change, which upstream has already reworked per this thread) will hit fail() and need surgery.py rewritten. It does fail loudly, which is good, but the README should say this.
    • The 200k-cycle equivalence testbenches and the negative controls described in the PR are not checked in. The next person to regenerate can't re-verify the surgery. Check them in, at least as a script under generate/.

    Alternative: do the extraction in Chisel. The PR already applies patches to cvfpu through http_archive(patches=...). A Scala patch that puts Regfile's storage behind a BlackBox/module boundary would survive firtool upgrades, and reviewers could read it as a diff against upstream.

  4. resources.py hides foundry names from the security scan. It builds them from fragments so the scrubber itself doesn't trip the scan. The intent is fine, but code built to get past the scanner will be hard to defend. The scrubber doesn't need foundry names at all: keep only the branches it knows are safe (`else and FPGA_XILINX) and drop every other `ifdef/`elsif branch. That removes the obfuscation and the (?!180) special case, and still fails safe when upstream adds a new PDK branch.

Should fix

  1. Builds can't be reproduced exactly. generate/.gitignore excludes MODULE.bazel.lock (upstream commits its own), and generate/ has no .bazelversion. Because generate/ is its own Bazel root, bazelisk probably won't pick up ORFS's top-level .bazelversion (worth checking). Commit the lockfile and add a .bazelversion.
  2. Nothing checks that the checked-in file matches the generator. update.sh just copies the output over. The generated file doesn't name its source either: its only header line is // Generated by CIRCT firtool-1.133.0. Options:
    • prepend a header (upstream commit, generate/ recipe, toolchain versions) in update.sh;
    • add a diff_test in generate/BUILD.bazel so bazelisk test //... there shows drift.
  3. sram.memories leaves out Sram's rvalid output. rvalid is a registered copy of enable. Please confirm that AUTO_MEMORIES keeps it, rather than leaving it dangling or dropping it. Also confirm the generated macro holds read data between reads the way upstream's behavioural model does: there, rdata = mem[raddr] and raddr only updates on enable & ~write.
  4. Repo-name parsing in resources.py is fragile. It extracts repo names with external/[^/]*?([a-z_]+)/ from bzlmod's canonical names (+http_archive+coralnpu). That works today, but would break on a repo name containing a digit or on a change to canonical naming. Use the Bazel labels it already has, or an explicit mapping.

Minor

  • The branch contains three merges of master; rebasing before un-drafting would give a cleaner history.
  • Each upstream bump replaces about 1.8 MB of checked-in SV; this one is +26k/−19k lines. That's acceptable so testers don't need Bazel or Chisel, but it's another reason to stamp the header.
  • The design isn't in CI (no rules file), so nothing catches it if it breaks. Point 6 matters more because of that.

Bottom line: the Bazel generator is a real improvement over master's unrecorded snapshot. Before merging, it needs a SOURCE.md and LICENSES/, an allowlist-based scrubber, and the surgery verification checked in. It should also settle the toolchain mismatch: use upstream's versions, or record why it doesn't. Separately, upstream's new asap7 flow is a good reason to talk to them about whether ORFS should share or follow it.

🤖 Generated with Claude Code

@oharboe

oharboe commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Upstream now has its own ORFS flow … worth deciding which upstream top ORFS should track, and whether ORFS should use upstream's emit target rather than its own.

How coralnpu upstream builds today (main 605865628):

  • Not on the BCR. The module is coralnpu_hw, and the BCR has no entry for it.
  • No bazel-orfs, ORFS or OpenROAD dependency in its MODULE.bazel.
  • Bazel only emits the RTL, on the host. synthesis/rvv_core_mini/emit_rtl.sh builds //hdl/chisel/src/coralnpu:rvv_core_mini_axi_cc_library_emit_verilog and copies out RvvCoreMiniAxi.sv.
  • Place and route is plain ORFS make in Docker. Per synthesis/BUILDING_CORALNPU_WITH_OPENROAD.md, openroad/orfs:latest has no Bazel, so the .sv is generated first. run_full_flow.sh then runs make do-1_synth … do-finish inside the container, with SRAMs as fakeram_512x128/fakeram_2048x128.

The next commits on this PR follow upstream:

  • Top: RvvCoreMiniAxi, the variant upstream's own asap7 flow builds.
  • Emit: generate/ uses upstream's own emit target from its own module, at its pinned commit. The replacement BUILD files go. Upstream's firtool/Chisel versions come with the module.
  • Pin: upstream main as of 2026-11-06.
  • Differences, recorded in SOURCE.md:
    • Visibility patch: the emit targets are private, so a one-line patch makes them visible.
    • Generated memories: the SRAMs and register files become generated macros (AUTO_MEMORIES, generate_ram) instead of fakerams.
    • Register-file surgery: the register files are moved into modules of their own by generate/surgery.py.

Where it differs from upstream's flow: Bazel all the way through.

  • In generate/: bazelisk test //... re-runs the surgery check (simulation before/after, with two negative controls that must fail), and a diff_test checks that the checked-in .sv matches what the module produces.
  • The checked-in, post-surgery .sv keeps the design buildable with plain make and no Bazel or Chisel, as today. It carries a header naming the upstream commit and toolchain.
  • The flow also builds under bazel-orfs, as the other ORFS designs do.

Build framework: how coralnpu runs its own flow, whether Docker and make, bazel-orfs or something else, is coralnpu's choice. A pull request to coralnpu that moves its flow onto bazel-orfs would be a framework choice made for them from outside. If they want it, they will pull on that rope themselves. This PR consumes what they publish, the way they publish it.

The other points of the review are answered with those commits.

@oharboe

oharboe commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as moot. coralnpu upstream's own OpenROAD work is on RvvCoreMiniAxi, the vector variant (synthesis/rvv_core_mini/), not on CoreMiniAxi. ORFS will follow that in a new pull request: RvvCoreMiniAxi, built from upstream's own emit target, with its memories built by generate_ram wherever possible.

@oharboe oharboe closed this Nov 6, 2026
@oharboe

oharboe commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Correction to the closing comment above: there won't be a new ORFS pull request for RvvCoreMiniAxi either. coralnpu upstream has moved on to other work, and ORFS will not build a flow for its core that upstream itself is not pursuing.

I asked upstream whether it wants its OpenROAD flow built with Bazel, bazel-orfs and generate_ram: google-coral/coralnpu#151. Any work on coralnpu here waits for their answer.

An earlier comment said the emit targets need a one-line visibility patch. That was wrong: the package that emits RvvCoreMiniAxi.sv is public.

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.

3 participants