Skip to content
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

ICache documentation. #2761

Closed
wants to merge 1 commit into from
Closed

ICache documentation. #2761

wants to merge 1 commit into from

Conversation

sequencer
Copy link
Member

This PR adds documentation to Rocket ICache.

Type of change: documentation

Impact: no functional change

Development Phase: implementation

Release Notes
Adds documentation to Rocket ICache.

@sequencer
Copy link
Member Author

Still have some TODO to resolve, but can start to be reviewed.

@aswaterman
Copy link
Member

Thanks. I don't have time to review this now, but if anyone else who knows the frontend uarch has feedback, please provide it!

@sequencer
Copy link
Member Author

sequencer commented Dec 15, 2020

Thank you. I'll keep rebasing this patch to top, and resolve those todo by reading codes.

@jerryz123
Copy link
Contributor

jerryz123 commented Mar 23, 2022

I think we should come to a consensus on what style of documentation is appropriate for microarchitecture like this. Specifically, who is the target audience, what background knowledge do we assume they possess, and what parts of this code need documenting?

The following are my opinions on this:

I think we should assume some level of familiarity with microarchitecture, as well as reasonable familiarity with Chisel and Scala syntax. I also believe that documentation should be used to explain the design of the uarch, rather than the Chisel implementation.

I think the three main things to document are 1) configuration parameters, 2) IO signals, and 3) microarchitecture. 1) and 2) should be documented where the Parameters class and IO Bundle are defined. 3), in my opinion, should be documented in contiguous paragraphs of scaladoc, rather than inline. After reading the documentation for 1,2,3, someone familiar with Chisel and Scala should be able to read the rest of the implementation without much confusion, with some assistance from inline comments on particularly obscure implementation details.

@jerryz123
Copy link
Contributor

jerryz123 commented Jul 13, 2022

Closing this due to new PR #3001.

@jerryz123 jerryz123 closed this Jul 13, 2022
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.

3 participants