chisel icon indicating copy to clipboard operation
chisel copied to clipboard

Implement module prefix API

Open jared-barocsi opened this issue 4 years ago • 1 comments

Contributor Checklist

  • [x] Did you add Scaladoc to every public function/method?
  • [x] Did you add at least one test demonstrating the PR?
  • [x] Did you delete any extraneous printlns/debugging code?
  • [x] Did you specify the type of improvement?
  • [ ] Did you add appropriate documentation in docs/src?
  • [x] Did you state the API impact?
  • [x] Did you specify the code generation impact?
  • [x] Did you request a desired merge strategy?
  • [x] Did you add text to be included in the Release Notes for this change?

Type of Improvement

  • new feature/API

API Impact

Adds new API

Backend Code Generation Impact

No impact

Desired Merge Strategy

Squash and merge

Release Notes

Implement Module Prefix API

  • Adds a new object, withModulePrefix(prefix), that prefixes all instantiated modules within its scope with the prefix argument
  • Can be nested either directly in itself, or within modules created in its scope

Reviewer Checklist (only modified by reviewer)

  • [x] Did you add the appropriate labels?
  • [x] Did you mark the proper milestone (Bug fix: 3.3.x, [small] API extension: 3.4.x, API modification or big change: 3.5.0)?
  • [x] Did you review?
  • [x] Did you check whether all relevant Contributor checkboxes have been checked?
  • [ ] Did you mark as Please Merge?

jared-barocsi avatar Oct 07 '21 22:10 jared-barocsi

Discussed this again with @jackkoenig . The main limitation of this PR as it is is that we lose the "what is the prefix (ad-hoc namespace)" and "what is the leaf name" distinction. I'd suggest we do one of (in increasing order of complexity / design thought required):

  1. make some convention that names can be . - separated into .fir, and lowered later to concatenate these names (minor firrtl spec expansion). Then make this PR use . to separate the prefixes
  2. Add a first class concept of "prefix" to firrtl spec and pass the prefix in directly
  3. Add a first class concept of "namespace" to firrtl spec and stop using prefix as ad-hoc namespace

mwachs5 avatar Jun 21 '22 15:06 mwachs5