Documentation to TLBroadcast #2706
Conversation
| val addressMask = AddressDecoder(flatAddresses.map(Seq(_)), flatAddresses.map(_.mask).reduce(_|_)) | ||
| /** This identify all the caches for the probe walker. */ | ||
| val caches: Seq[IdRange] = clients.filter(_.supports.probe).map(_.sourceId) | ||
| /** Use start bit in IdRange to locate which client is target. */ |
There was a problem hiding this comment.
I think it is the lowest id value in the IdRange, which isn't the same as a bit index within a UInt
| val d_trackerOH = VecInit(trackers.map { t => t.need_d && t.source === d_normal.bits.source }).asUInt holdUnless d_first | ||
|
|
||
| /** if d is valid, and not drop. */ |
There was a problem hiding this comment.
Good comments don't repeat the code or explain it. They clarify its intent. The next line of code literally says !out.d.valid || !d_drop so I don't think this comment adds any clarity.
| (trackers zip UIntToOH(in.e.bits.sink).asBools) foreach { case (tracker, select) => | ||
| tracker.e_last := select && in.e.fire() | ||
| } | ||
|
|
||
| // Depending on the high source bits, we might transform D | ||
| /* Client D Channel: | ||
| * Forward or transform manger D channel based on the payload of the source. |
There was a problem hiding this comment.
you should find/replace this change, i see more
| // ProbeAck => decrement tracker, drop | ||
| // ProbeAckData => decrement tracker, send out A as PutFull(DROP) | ||
| // ReleaseData => send out A as PutFull(TRANSFORM) | ||
| // Release => send out D as ReleaseAck |
There was a problem hiding this comment.
Did this information get lost? Or is it replicated elsewhere now?
There was a problem hiding this comment.
In has been moved to Top of this file under
Behavior of [[TLBroadcast]]:
Actually original comments was wrong or not accurate, since ProbeAckData won't decrement tracker(MSHR)
Real behavior should be:
- ClientA -Acquire-> BH
- BH -Probe-> ClientB
- ClientB -ProbeAckData-> BH
- BH -Put-> Manger
- Manger -AccessAckData-> BH
decrement by Acquire should happen in 5. rather than 3.
| val cacheOH = UInt(caches.W) | ||
| } | ||
|
|
||
| /** Tracker to track trans */ |
There was a problem hiding this comment.
I would say something here about how some people might call this a Miss Status Handling Register (MSHR)
There was a problem hiding this comment.
Yes, I finally understand it after documenting to MSHR of InclusiveCache, I found they have a similar behavior.
| val in_a_first: Bool = Input(Bool()) | ||
| /** incoming A channel from clients. */ | ||
| val in_a: DecoupledIO[TLBundleA] = Flipped(Decoupled(new TLBundleA(edgeIn.bundle))) | ||
| /** output A channel to mangers. */ |
There was a problem hiding this comment.
I'd say "inwards" and "outwards" here to match what we wrote for the diplomacy node docs
| count := 1.U | ||
| } | ||
|
|
||
| /* cache [[cacheOH]] by [[io.clearOH]] */ |
There was a problem hiding this comment.
Again I think this repeats the code without clarifying its intent. (I think its intent is to clear some of the pending bits identifying which caches still need to be probed).
Related issue: None
Type of change: Documentation
Impact: no functional change
Development Phase: implementation