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

[tracegen] Add tracegen support for the BOOM L1D #362

Merged
merged 3 commits into from
Jan 24, 2020
Merged

Conversation

jerryz123
Copy link
Contributor

Depends on #358 's BOOM bump

@jerryz123 jerryz123 requested a review from zhemao December 11, 2019 06:37
@jerryz123 jerryz123 removed their assignment Dec 13, 2019
@jerryz123 jerryz123 force-pushed the boom-tracegen branch 4 times, most recently from f6ebd08 to ed1728d Compare December 14, 2019 23:23
@jerryz123
Copy link
Contributor Author

I split up the shim logic into its own module, and split up the mixins for instantiating the BOOM tracegen.

@jerryz123 jerryz123 requested a review from zhemao December 26, 2019 23:30
colinschmidt added a commit that referenced this pull request Jan 18, 2020
colinschmidt added a commit that referenced this pull request Jan 21, 2020
Copy link
Contributor

@zhemao zhemao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's some weirdness with queue handling here. Otherwise, I'm satisfied.


when (io.tracegen.req.fire()) {
rob_tail := WrapInc(rob_tail, rob_sz)
rob_bsy(rob_tail) := true.B
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't rob_head and rob_tail backwards here? Entries are placed into a circular buffer at the head and taken out at the tail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follow's BOOM's ROB naming scheme.

io.tracegen.req.ready := (!rob_bsy(rob_tail) &&
!rob_wait_till_empty &&
(ready_for_amo || !(isAMO(io.tracegen.req.bits.cmd) || io.tracegen.req.bits.cmd === M_XLR || io.tracegen.req.bits.cmd === M_XSC)) &&
(WrapInc(rob_tail, rob_sz) =/= rob_head) &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This always leaves one entry in the ROB empty. Take a look at Chisel's queue implementation to see how you can detect full queues properly.

@zhemao
Copy link
Contributor

zhemao commented Jan 22, 2020

Take a look at this part of the Chisel codebase to see how to detect queue empty/full correctly.

https://github.com/freechipsproject/chisel3/blob/master/src/main/scala/chisel3/util/Decoupled.scala#L207

Copy link
Contributor

@zhemao zhemao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll relent on the queue handling for now.

@jerryz123 jerryz123 merged commit 05f17f5 into dev Jan 24, 2020
@colinschmidt colinschmidt deleted the boom-tracegen branch January 27, 2020 18:54
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