Repository navigation
FakeRAM2.0: size the macro height from its pins, not a 3.5 um floor - #4608
Merged
maliberty merged 2 commits intoOct 4, 2026
Merged
Conversation
Process.get_macro_dimensions() floored every macro's height at 3.5 um. The floor came in with the ORFS asap7 backend patch (a632406) as a bare literal: no comment, nothing in the commit message, and no PDK value it corresponds to. It sits in the shared Process class, so it applied to every FakeRAM2.0 config, not only the asap7 backend. Removed so that whatever depended on it fails loudly. A forced 4x25 memory, as in gen_memories_test.py and tinyRocket's tag_array, now fails with "not enough tracks (num pins: 55, available tracks: 27)". The next commit fixes that from the pin count. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Signal pins are stacked one per track up the macro's left edge, inset by y_offset at both ends. A shallow memory with many pins came out shorter than that and set_pin_pitches() raised "not enough tracks". The macro is now at least 2 * y_offset + num_pins * pin_pitch tall, computed from values the process config already has. This is what the dropped 3.5 um floor was papering over for one shape. A forced 4x25 asap7 RAM (55 pins) is now 2.8 um tall, not 4.2. Shapes the floor did not cover also generate now: 16x64 and 16x128 asap7 RAMs passed the idiomatic gate but failed with "not enough tracks" even with the floor. Memories whose bitcells already fit their pins are unchanged. set_pin_pitches() rounds before flooring the track count, so a height that fits its pins exactly does not lose a track to float error (46.99999 -> 46). With a 1 nm snap this failed 185 of 14448 swept configurations, and none after. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the memory height calculation logic to ensure that shallow memories with many pins are made tall enough to accommodate all signal pins. It removes a hardcoded minimum height constraint from the process dimensions, rounds the track availability calculation to prevent floating-point precision issues, and adds comprehensive unit tests to verify these behaviors. There are no review comments, and I have no additional feedback to provide.
Member
🔍 QoR checkMetrics reflect the PR merge build — i.e. what will land on the target branch. Commit 62 design(s) checked — 0 with regression(s), 0 without a comparable baseline. |
maliberty
reviewed
Oct 3, 2026
maliberty
approved these changes
Oct 4, 2026
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.
TL;DR: FakeRAM2.0 sizes a macro's minimum height from its signal pins, replacing an unexplained 3.5 µm floor. This answers the review question "Where did 3.5 come from?".
Git history doesn't explain the 3.5: it arrived as a bare literal in a632406. Commit 1 removes it, and a forced 4×25 memory (tinyRocket's
tag_array,gen_memories_test.py) then fails loudly with "not enough tracks (num pins: 55, available tracks: 27)". Commit 2 computes the minimum from the pins, which stack one per track up the left edge:Testing: new
memory_test.pycases fail without the fix. A sweep of 14,448 process/memory configs has no failures.🤖 Generated with Claude Code