-
Notifications
You must be signed in to change notification settings - Fork 586
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make SRAMInterface
parameters publicly available
#3826
Conversation
numReadwritePorts: Int, | ||
masked: Boolean = false) | ||
val memSize: BigInt, | ||
val tpe: T, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This actually can't be public in this way because making an argument to a class a val
makes it a public field of the class in a way that is indistinguishable with public vals in the body. Since this is of type Data
, it will become a field of the Bundle.
Instead, you can keep tpe
not a val
but add a public def below that just returns it, something like def dataType: T = tpe
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I'm actually thinking of putting this into a case class so the whole interface doesn't have to get passed around all the time (per what Megan said). So that should avoid the Data
issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok nvm, switching to your suggested getter because of what Andrew said
using case classes has the long-term issue of dead fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that "API Modification" means breaking something, just making stuff public is a new "Feature"
(cherry picked from commit 8f0196f)
(cherry picked from commit 8f0196f)
(cherry picked from commit 8f0196f) Co-authored-by: Deborah Soung <debs@sifive.com>
(cherry picked from commit 8f0196f) Co-authored-by: Deborah Soung <debs@sifive.com>
Contributor Checklist
docs/src
?Type of Improvement
Desired Merge Strategy
Release Notes
memSize
,dataType
,numReadPorts
,numWritePorts
,numReadwritePorts
,masked
parameters are now visible forSRAMInterface
.Reviewer Checklist (only modified by reviewer)
3.6.x
,5.x
, or6.x
depending on impact, API modification or big change:7.0
)?Enable auto-merge (squash)
, clean up the commit message, and label withPlease Merge
.Create a merge commit
.