Skip to content

Commit

Permalink
fix(LoadQueueUncache): exhaust the various cases of flush (OpenXiangS…
Browse files Browse the repository at this point in the history
…han#4300)

**Bug trigger point:**

The flush occurs during the `s_wait` phase. The entry has already passed
the flush trigger condition of `io.uncache.resp.fire`, leading to no
flush. As a result, `needFlushReg` remains in the register until the
next new entry's `io.uncache.resp.fire`, at which point the normal entry
is flushed, causing the program to stuck.

**Bug analysis:** The granularity of flush handling is too coarse.

In the original calculation:
```
val flush = (needFlush && uncacheState === s_idle) || (io.uncache.resp.fire && needFlushReg)
```
Flush is only handled in two states: `s_idle` and non-`s_idle`. This
distinction makes the handling of the other three non-`s_idle` states
very coarse. In fact, for the remaining three states, there needs to be
corresponding feedback based on when `needFlush` is generated and when
`NeedFlushReg` is delayed in the register.
1. In the `s_req` state, before the uncache request is sent, the flush
can be performed in time, using `needFlush` to prevent the request from
being sent.
2. If the request has been sent and the state reaches `s_resp`, to avoid
mismatch between the uncache request and response, the flush can be only
performed after receiving the uncache response, i.e., use `needFlush ||
needFlushReg` to flush when `io.uncache.resp.fire`.
3. If a flush occurs during the `s_wait` state, it can also prevent a
write-back and use `needFlush` to flush in time.

**Bug Fix:** 

For better code readability, the `uncacheState` state machine update is
used here to update the `wire` `flush`. Where `flush` refers to
executing the flush, `needFlush` refers to the signal that triggers the
flush, and `needFlushReg` refers to the flush signal stored for delayed
processing flush.
  • Loading branch information
Maxpicca-Li authored Feb 24, 2025
1 parent 1eb8dd2 commit afa1262
Showing 1 changed file with 16 additions and 8 deletions.
24 changes: 16 additions & 8 deletions src/main/scala/xiangshan/mem/lsqueue/LoadQueueUncache.scala
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class UncacheEntry(entryIndex: Int)(implicit p: Parameters) extends XSModule
*/
val needFlushReg = RegInit(false.B)
val needFlush = req_valid && req.uop.robIdx.needFlush(io.redirect)
val flush = (needFlush && uncacheState===s_idle) || (io.uncache.resp.fire && needFlushReg)
val flush = WireInit(false.B)
when(flush){
needFlushReg := false.B
}.elsewhen(needFlush){
Expand Down Expand Up @@ -123,27 +123,35 @@ class UncacheEntry(entryIndex: Int)(implicit p: Parameters) extends XSModule
)
switch (uncacheState) {
is (s_idle) {
when (canSendReq) {
when (needFlush) {
uncacheState := s_idle
flush := true.B
}.elsewhen (canSendReq) {
uncacheState := s_req
}
}
is (s_req) {
when (io.uncache.req.fire) {
when(needFlush){
uncacheState := s_idle
flush := true.B
}.elsewhen(io.uncache.req.fire) {
uncacheState := s_resp
}
}
is (s_resp) {
when (io.uncache.resp.fire) {
when (needFlushReg) {
when (needFlush || needFlushReg) {
uncacheState := s_idle
flush := true.B
}.otherwise{
uncacheState := s_wait
}
}
}
is (s_wait) {
when (writeback) {
when (needFlush || writeback) {
uncacheState := s_idle
flush := true.B
}
}
}
Expand All @@ -157,7 +165,7 @@ class UncacheEntry(entryIndex: Int)(implicit p: Parameters) extends XSModule
io.slaveId.bits := slaveId

/* uncahce req */
io.uncache.req.valid := uncacheState === s_req
io.uncache.req.valid := uncacheState === s_req && !needFlush
io.uncache.req.bits := DontCare
io.uncache.req.bits.cmd := MemoryOpConstants.M_XRD
io.uncache.req.bits.data := DontCare
Expand Down Expand Up @@ -202,7 +210,7 @@ class UncacheEntry(entryIndex: Int)(implicit p: Parameters) extends XSModule
io.ncOut.bits := DontCare

when(req.nc){
io.ncOut.valid := (uncacheState === s_wait)
io.ncOut.valid := (uncacheState === s_wait) && !needFlush
io.ncOut.bits := DontCare
io.ncOut.bits.uop := selUop
io.ncOut.bits.uop.lqIdx := req.uop.lqIdx
Expand All @@ -217,7 +225,7 @@ class UncacheEntry(entryIndex: Int)(implicit p: Parameters) extends XSModule
io.ncOut.bits.is128bit := req.is128bit
io.ncOut.bits.vecActive := req.vecActive
}.otherwise{
io.mmioOut.valid := (uncacheState === s_wait)
io.mmioOut.valid := (uncacheState === s_wait) && !needFlush
io.mmioOut.bits := DontCare
io.mmioOut.bits.uop := selUop
io.mmioOut.bits.uop.lqIdx := req.uop.lqIdx
Expand Down

0 comments on commit afa1262

Please sign in to comment.