Skip to content

Conversation

@richardyrh
Copy link
Contributor

This is to support the TSTECH16C SRAM generators. The depth/widths supported is not a linear sequence; also, some combinations of valid depth and valid width is not valid (thus the triple option). This change should not have any impact on existing SRAM generators, since it looks for the JSON keys depth list, width list, or triples.

triples will take precedence, followed by the explicit lists, and then finally the fallback is the current scheme of specifying 3 numbers (start, end, step). If nothing is found, the compilation fails (like it currently does).

Related PRs / Issues:

Type of change:

  • Bug fix
  • New feature
  • Other enhancement

Impact:

  • RTL change
  • Software change (RISC-V software)
  • Build system change
  • Other

Contributor Checklist:

  • Did you set main as the base branch?
  • Is this PR's title suitable for inclusion in the changelog and have you added a changelog:<topic> label?
  • Did you state the type-of-change/impact?
  • Did you delete any extraneous prints/debugging code?
  • Did you mark the PR with a changelog: label?
  • (If applicable) Did you update the conda .conda-lock.yml file if you updated the conda requirements file?
  • (If applicable) Did you add documentation for the feature?
  • (If applicable) Did you add a test demonstrating the PR?
  • (If applicable) Did you mark the PR as Please Backport?

CI Help:
Add the following labels to modify the CI for a set of features.
Generally, a label added only affect subsequent changes to the PR (i.e. new commits, force pushing, closing/reopening).
See ci:* for full list of labels:

  • ci:fpga-deploy - Run FPGA-based E2E testing
  • ci:local-fpga-buildbitstream-deploy - Build local FPGA bitstreams for platforms that are released
  • ci:disable - Disable CI

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for more flexible SRAM depth and width specifications in SRAM generators, specifically targeting TSTECH16C SRAM generators that don't support linear sequences or all combinations of valid depth/width pairs.

  • Extends SRAMGroup to support explicit lists of depths/widths and valid triples (depth, width, port) combinations
  • Adds new JSON parsing logic to handle "depth list", "width list", and "triples" fields with fallback to existing range-based specifications
  • Updates macro compilation logic to use triples when available for generating SRAM macros

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
tools/tapeout/src/main/scala/macros/MacroCompiler.scala Added logic to handle triples for macro generation with fallback to existing depth/width iteration
tools/tapeout/src/main/scala/macrolib/SRAM.scala Extended SRAMGroup case class and JSON parsing to support explicit lists and triples alongside existing range specifications

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

@iansseijelly iansseijelly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Might be worthy of documenting this new API somehow.

@richardyrh richardyrh merged commit ee46501 into main Oct 4, 2025
71 of 72 checks passed
@richardyrh richardyrh deleted the sram-config branch October 4, 2025 00:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants