module-name-prefix FIRRTL pass

[kenzhang82]

Contributor Checklist

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

Type of Improvement

API Impact

Backend Code Generation Impact

Desired Merge Strategy

Release Notes

Reviewer Checklist (only modified by reviewer)

• [ ] Did you add the appropriate labels?
• [ ] Did you mark the proper milestone (1.2.x, 1.3.0, 1.4.0) ?
• [ ] Did you review?
• [ ] Did you check whether all relevant Contributor checkboxes have been checked?
• [ ] Did you mark as Please Merge?

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: ekiwi
:x: kenzhang82
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[kenzhang82]

In case anyone is still wondering if there is a solution, here is transform that Chipyard folks are using for tapeout https://github.com/ucb-bar/barstools/blob/master/tapeout/src/main/scala/barstools/tapeout/transforms/AddSuffixToModuleNames.scala

I think we should ask Chipyard folks to upstream this. I am closing this PR for now.

[sequencer]

Sorry for the late replay. AFAIK, SiFive and UCB has their own transform for this feature, I personally think this can be upstreamed, if you are interested in making this feature upstreamed, I'm willing to help:)

[kenzhang82]

The link I posted above is pretty much the same feature as in this PR, I think it would be better for the maintainers of FIRRTL and the barstool repo to upstream this (it would be much smoother process I reckon).

[kenzhang82]

Giving a second thought I think we should take the initiative to upstream this, I do have another similar feature request for the memlib pass to be upstreamed too, I will put them together.

[seldridge]

Also, I'd like to get something upstreamed here! This is a pretty fundamental thing. Thanks for hacking on this. :+1:

[ekiwi]

One question that I think needs to be answered here is: What is the desired behavior for the toplevel (aka main) module?

• prefix toplevel module
• do not prefix toplevel module
• make this an option

[kenzhang82]

Thanks for the advice and comments @seldridge @ekiwi, I will amend the PR accordingly and let you guys know when they are good for review.

[ekiwi]

@kenzhang82 Could you please run sbt scalafmtAll and commit + push any changes? The CI is complaining that the code isn't properly formatted.

[kenzhang82]

Yes sure @ekiwi

[kenzhang82]

Yes sure @ekiwi

And done, can you check if it is roughly ok now? Sorry I don't have time yet to amend the PR, do you think we should merge this in or wait for me to amend it further as per the comments above?

[ekiwi]

And done, can you check if it is roughly ok now? Sorry I don't have time yet to amend the PR, do you think we should merge this in or wait for me to amend it further as per the comments above?

I will try to get @seldridge or @jackkoenig to look at this. Since I am not really a user of this API, I am not sure what the requirements are.