-
Notifications
You must be signed in to change notification settings - Fork 614
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
Fix incorrect comment in ScalaDoc #1756
Conversation
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.
I tested it on scastie, seems right now
@@ -127,7 +127,7 @@ object RegNext { | |||
* val x = Wire(UInt()) | |||
* val y = Wire(UInt(8.W)) | |||
* val r1 = RegInit(x) // width will be inferred | |||
* val r2 = RegInit(y) // width is set to 8 | |||
* val r2 = RegInit(y) // width will be inferred |
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.
Isn't it set to 8 eventually? What's the mechanism?
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.
inference in FIRRTL
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.
Can we add it as a comment? It may not be obvious that it is inferred, especially if the example code doesn't assign anything else to r2 that could change its width.
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.
@edwardcwang Here's the scastie demo. I am not sure of the mechanism, but it confirms, I believe, the correctness of Jack's change
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.
Read the whole ScalaDoc, that’s sort of the point of this whole example: https://www.chisel-lang.org/api/3.4.1/chisel3/RegInit$.html
It’s just that that particular comment is wrong and contradicts the correct info above.
(cherry picked from commit d0db0b8)
(cherry picked from commit d0db0b8)
(cherry picked from commit d0db0b8)
(cherry picked from commit d0db0b8) Co-authored-by: Jack Koenig <[email protected]>
(cherry picked from commit d0db0b8) Co-authored-by: Jack Koenig <[email protected]>
(cherry picked from commit d0db0b8) Co-authored-by: Jack Koenig <[email protected]>
Contributor Checklist
docs/src
?Type of Improvement
API Impact
No impact
Backend Code Generation Impact
No impact
Desired Merge Strategy
Release Notes
Fix incorrect example in ScalaDoc for width behavior of RegInit
Reviewer Checklist (only modified by reviewer)
Please Merge
?