coralnpu: fix transposed FakeRAM geometry and mis-sized DTCM - #237
Open
bugraonal wants to merge 1 commit into
Open
coralnpu: fix transposed FakeRAM geometry and mis-sized DTCM#237bugraonal wants to merge 1 commit into
bugraonal wants to merge 1 commit into
Conversation
The TCM memories were wired up with width and depth swapped, so on asap7 the synthesized core could not hold its own program: only data bits [15:0] of any word were writable and the 9-bit address was truncated to 7. Three separate defects: 1. designs/src/coralnpu/dev/generated/fakeram_asap7.cfg declared width/depth transposed on both macros (depth=128 for both, contradicting the fakeram_<DEPTH>x<WIDTH> naming) with write_granularity 1 where the RTL drives 16 byte strobes. nangate45 and sky130hd were already correct and serve as the reference. The cfg is not where the error originated -- coralnpu's cfg was reconstructed from the already-transposed committed LEF by reconstruct_cfg_from_lef.py -- but it is where it has to be corrected. 2. designs/src/coralnpu/macros.v is shared by all three platforms and hardcoded the transposed geometry, so it was wrong everywhere: it matched the broken asap7 macro and contradicted the correct nangate45/sky130hd ones. The file already contradicted itself, since wmask[15:0] is 16 byte strobes and only makes sense for a 128-bit word. 3. Sram_2048x128 instantiated fakeram_512x128 (copy-paste in fakeram-memories.patch). coralnpu's DTCM is 32 KB = 2048x128, so it was built from a 512-word macro -- four times too small -- while fakeram_2048x128 was generated and shipped but never instantiated. Easy to miss because under the transposition both macros were 128 words deep anyway. asap7 LEF/LIB regenerated with tools/regenerate_sram.sh; the macros now match nangate45/sky130hd exactly (addr 9/11, wd_in 128, wmask 16) and the generated RTL wrappers line up on both sides.
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
2 participants
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.
Fixes 2 issues with coralnpu:
Issue
1originates from two locations:a) The asap7 fakeram config has the width and depth swapped.
b) macros.v
DATA_WIDTHandSIZEparameters swapped.Issue
2originates from the fakeram patch. The patch replaces the original 2048x128 macro with the 512x128 fakeram macro.The PR fixes these issues and has the regenerated lib/lef files.
Note: I found these issues when performing switching based (simulation with VCD output) power evaluations. The contents of the memory would eventually become
X.