Repository navigation
asap7/cva6: the RAMs as upstream has them, made macros - #4777
Merged
maliberty merged 4 commits intoNov 6, 2026
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request replaces hardcoded fakeram7 models in the CVA6 design with behavioral SRAM models and integrates them using AUTO_MEMORIES and a Python-based surgery script (surgery.py). The feedback suggests explicitly specifying UTF-8 encoding when opening files in the surgery script to prevent potential UnicodeDecodeError issues on systems with different default encodings.
Member
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 1 with regression(s), 0 without a comparable baseline. ❌ asap7/cva6 base — 2 failing metric(s)
|
vvbandeira
pushed a commit
that referenced
this pull request
Nov 4, 2026
flow/scripts/memories/gen_memories.py runs tools/FakeRAM2.0/run.py,
found relative to itself. The image copies flow/ and tools/AutoTuner but
not tools/FakeRAM2.0, so every AUTO_MEMORIES flow run from the image
stops at the memories step:
/usr/bin/python3: can't open file
'/OpenROAD-flow-scripts/tools/FakeRAM2.0/run.py': [Errno 2] No such
file or directory
No design in CI sets AUTO_MEMORIES today, so CI on master does not show
it; a PR that adds one fails there (asap7/cva6 in #4777).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
oharboe
marked this pull request as ready for review
November 6, 2026 06:32
asap7/cva6 had rewritten hpdcache_sram_wbyteenable to instantiate a fakeram7_128x64 macro and dropped its byte enables: the RAM holds both ways of a set in one 64-bit word, so every refill and store hit wrote the whole word and overwrote the other way's data. hpdcache_sram.sv and hpdcache_sram_wbyteenable.sv are upstream cva6's again (3a389af, cv-hpdcache 04de808), unmodified, with upstream's behavioural hpdcache_sram_1rw and hpdcache_sram_wbyteenable_1rw. AUTO_MEMORIES makes each a macro, cva6.memories naming its pins and byte enables: the data RAM (128 x 64, 8 byte enables) and the directory (64 x 28). Fixes The-OpenROAD-Project#4769 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
asap7/cva6 had rewritten sram_cache to instantiate fakeram7_64x256 and fakeram7_64x25 in place of its techno cut, dropping the I-cache RAMs' byte enables and replacing tc_sram's read behaviour with a fakeram's. sram_cache.sv is upstream cva6's again, unmodified. Upstream fills its techno cut, tc_sram_wrapper_cache_techno, with pulp's generic tc_sram; SYNTH_VERILOG_SURGERY (surgery.py) puts a single-port SRAM with byte enables there in its place, for AUTO_MEMORIES to make a macro of: the I-cache data (64 x 256, 32 byte enables) and tag (64 x 25, 4 byte enables, the last of them 1 bit, which needs mask_lane_width). The fakeram views leave the config. The Verilog is in surgery/, next to the script: the module as upstream has it (the one in VERILOG_FILES must match it token for token, or the build stops), its replacement, and the two SRAMs. The script reads and writes UTF-8 whatever the locale. tc_sram's read data follows the last read address through idle cycles and writes; an SRAM holds while idle. cva6_icache uses read data only in a cycle after a read, or after idle cycles with no write since that read, so the two return the same data whenever it is used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PFLaCzFtFbnWE4nPTDwpC Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
The README said "with some changes for the RAMs". The RAMs are now upstream's; what still differs are tie-offs for outputs or struct fields cv32a65x leaves undriven, listed by file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
oharboe
force-pushed
the
cva6-upstream-rams
branch
from
November 6, 2026 11:16
12f6fb8 to
1ea7bac
Compare
CI's CTS stage stops in the Kepler LEC check: kepler-formal cannot load the netlist, "cva6_icache_data_sram cannot be found in SNL", the macro AUTO_MEMORIES makes of the I-cache data RAM. A new kepler-formal is in the works; LEC_CHECK is 0 for cva6 until the Kepler team rewires it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018PFLaCzFtFbnWE4nPTDwpC Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
oharboe
force-pushed
the
cva6-upstream-rams
branch
from
November 6, 2026 11:19
1ea7bac to
afac85c
Compare
oharboe
commented
Nov 6, 2026
| $(PLATFORM_DIR)/lib/NLDM/fakeram7_128x64.lib \ | ||
| $(PLATFORM_DIR)/lib/NLDM/fakeram7_64x28.lib \ | ||
| $(PLATFORM_DIR)/lib/NLDM/fakeram7_64x25.lib | ||
| # Kepler LEC off for now: kepler-formal stops on the macros AUTO_MEMORIES |
Collaborator
Author
Contributor
There was a problem hiding this comment.
Some update. The issue was not with the tool but with missing memory files that were added with this change and need to be given to the formal check. Added the fix to #4329
Collaborator
Author
|
@maliberty cva6 is now set up quite differently, and correctly, so I see no sense in looking at the QoR "regression". This is a new baseline. |
maliberty
approved these changes
Nov 6, 2026
Collaborator
Author
|
😌 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before: asap7/cva6 rewrites cva6's RAM wrappers onto fakeram7 macros, and the D-cache data RAM loses its byte enables, so every refill and store hit overwrites the other way's word (#4769).
Now: the RAM RTL is upstream cva6's (3a389af), unmodified, and AUTO_MEMORIES makes each RAM a macro.
cva6 on asap7 after placement: die 151.5 µm × 151.5 µm, 8 RAM macros from AUTO_MEMORIES.
Changes:
hpdcache_sram.sv,hpdcache_sram_wbyteenable.svand the behaviouralhpdcache_sram_1rw.sv/hpdcache_sram_wbyteenable_1rw.svas upstream has them;cva6.memoriesnames their pins and byte enables for AUTO_MEMORIES. Fixes cva6: D-cache data RAM drops its byte enables, so every cache write corrupts the other way #4769.sram_cache.svas upstream has it;surgery.py(SYNTH_VERILOG_SURGERY) puts a single-port SRAM with byte enables behind its techno cut in place of pulp'stc_sram, and AUTO_MEMORIES makes the data and tag RAMs macros. The Verilog is insurgery/: the techno cut as upstream has it (the build stops if upstream's differs), its replacement, and the two SRAMs.config.mk: AUTO_MEMORIES on,LEC_CHECK = 0(below).Each macro's function is checked against the RTL's RAM by LEC, with controls that must fail. All 8 RAM instances, full size, run in cva6's Verilator testharness against Spike: the two-way test and CoreMark pass, RVFI traces are byte-identical to upstream's RAMs, and the controls fail.
Kepler LEC is off for cva6: kepler-formal cannot load the netlist after CTS (
cva6_icache_data_sram cannot be found in SNL), the macro AUTO_MEMORIES makes of the I-cache data RAM. A new kepler-formal is in the works; cva6 setsLEC_CHECK = 0until the Kepler team rewires it.🤖 Generated with Claude Code
https://claude.ai/code/session_018PFLaCzFtFbnWE4nPTDwpC