[Hardware][Chisel] Fix TensorStoreNarrowVME high-half stutter / AXI W-stability violation on mid-burst WREADY de-assert

[dreamqin68]

Summary

TensorStoreNarrowVME advances its sub-block select register tag back to 0 on tag === (numMemBlock - 1).U without gating that wrap on the write handshake io.vme_wr.data.fire. When the AXI slave legally de-asserts WREADY at the high-half beat of a 128-bit output element, the engine rolls tag back to the low half while the high-half beat is still unaccepted. This has two consequences:

1. It changes WDATA while WVALID is held high and the beat has not been accepted — an AXI4 write-data channel stability violation (per the AMBA AXI handshake-process rule, ARM IHI 0022 §A3.2.1: once a source asserts VALID it must keep VALID asserted and hold the payload — WDATA/WSTRB/WLAST — stable until the READY handshake completes).
2. It re-presents and re-accepts the low half in the slot that owed the high half, injecting a duplicate beat that shifts the accepted write stream by one 64-bit beat, so the beat the AW burst committed to its last slot is displaced off the end. When the burst is packed with real data that displaced beat is a real block (silent data loss); the test below proves the beat-sequence shift itself but does not exhibit a real-data drop (see the scope note).

The bug only fires when WREADY de-asserts at the high-half (last sub-block) beat; a stall at the low-half beat is benign. It is contingent on the AXI slave / interconnect de-asserting WREADY mid-burst, which AXI4 explicitly permits, so designs whose memory subsystem happens to hold WREADY high across a burst never observe it — but it is a latent correctness bug, not a protocol-legal behavior.

Scope of the "corruption" claim: the test below proves the module-level effect — the accepted-beat sequence shifts by one 64-bit beat and re-accepts the low half — which is a genuine AXI-protocol violation on the write channel. The end-to-end numerical impact on a stored tensor (which DRAM bytes end up wrong, and whether a tail block is dropped) additionally depends on the AW-committed burst length and the memory subsystem's tail handling, and is not reproduced here. So "corruption" here means a proven write-stream desynchronization, not a demonstrated end-to-end wrong-value workload.

Root cause

In hardware/chisel/src/main/scala/core/TensorStoreNarrowVME.scala:

when(state === sWriteCmd || tag === (numMemBlock - 1).U) {   // <-- wrap ungated by fire
  tag := 0.U
}.elsewhen(io.vme_wr.data.fire) {
  tag := tag + 1.U
}

io.vme_wr.data.bits.data := mdata(tag) and io.vme_wr.data.valid := state === sWriteData. On a WREADY-low cycle the FSM stays in sWriteData (its transition out is guarded by io.vme_wr.data.ready), so WVALID stays high while tag wraps underneath it.

The inconsistency is visible inside the same block: the sibling pointers for the same write stream are already handshake-gated —

}.elsewhen(io.vme_wr.data.fire && tag === (numMemBlock - 1).U) { set := set + 1.U }
...
}.elsewhen(io.vme_wr.data.fire && set === (tensorLength - 1).U && tag === (numMemBlock - 1).U) { raddr_cur := raddr_cur + 1.U }

Only tag's wrap is left ungated.

The fix

Gate both the wrap and the increment on io.vme_wr.data.fire, matching how set and raddr_cur already advance:

when(state === sWriteCmd) {
  tag := 0.U
}.elsewhen(io.vme_wr.data.fire) {
  tag := Mux(tag === (numMemBlock - 1).U, 0.U, tag + 1.U)
}

On entry (sWriteCmd) tag resets to 0; on each accepted beat it wraps if at the last sub-block, else increments. During a WREADY stall there is no fire, so tag holds and the high-half beat stays presented and stable.

Why this is behavior-preserving off the stall path

The one place the FSM leaves sWriteData between sub-block groups is the sReadMem re-read cycle. A maintainer will reasonably ask what tag does across the sWriteData -> sReadMem -> sWriteData round-trip, since the original code reset tag unconditionally at the wrap moment while this patch folds the reset into the fire branch. Tracing the FSM (sWriteData block and is(sReadMem){ state := sWriteData }):

• sReadMem is entered only on io.vme_wr.data.ready (= fire, since WVALID is high in sWriteData) with tag === (numMemBlock - 1).U and xcnt =/= xlen. So the high-half beat has already been accepted before the FSM moves to sReadMem.
• On that accepting cycle the patched code takes the .elsewhen(io.vme_wr.data.fire) branch and computes Mux(tag === last, 0.U, ...) = 0.U — identical to the original unconditional reset for the fire case.
• On the sReadMem cycle itself, state === sWriteCmd is false and io.vme_wr.data.valid (state === sWriteData) is false, so fire is false; neither branch executes and tag holds 0.
• Back in sWriteData, tag = 0 selects the low half of the next element.

So on the no-stall path the patched tag sequence is bit-for-bit identical to the original; the only behavioral change is under back-pressure (high-half beat not yet fired), where tag now holds instead of wrapping early. Empirically the no-stall accepted-beat sequence (the four real beats LOW0, HIGH0, LOW1, HIGH1 followed by two ZERO pad beats), which crosses the sReadMem round-trip between elements 0 and 1, is identical before and after the patch.

Verification

This PR adds a chiseltest regression test, hardware/chisel/src/test/scala/unittest/TensorStoreNarrowVMETest.scala, that reproduces the defect directly on the Chisel module — treadle backend, no Verilator or external tooling, runs under sbt test:

sbt "testOnly unittest.TensorStoreNarrowVMEWReadyStallTest"

It stores two 128-bit elements, holds WREADY low for three cycles on the high-half beat, and asserts (P1) WDATA is stable while WVALID && !WREADY and (P2) the low half is not accepted twice. On the pre-fix RTL it fails both properties; with this fix it passes.

Concretely, P2 pins the accepted-beat sequence under the stall to the following (this is the contrast the assertion encodes, not console output — chiseltest reports only pass/fail). LOWk/HIGHk are element k's two 64-bit halves — bytes 0x10..0x1f for element 0, 0x40..0x4f for element 1 — and ZERO is a burst pad beat read past the two loaded elements:

pre-fix  STALL: LOW0, LOW0, HIGH0, LOW1, HIGH1, ZERO   (WDATA changed under WVALID && !WREADY; low half accepted twice)
post-fix STALL: LOW0, HIGH0, LOW1, HIGH1, ZERO, ZERO   (identical to the no-stall control)

Pre-fix, LOW0 is duplicated and every later beat shifts one slot right: HIGH1 is still accepted, and what falls off the burst tail here is a ZERO pad beat, so this stimulus does not drop real data (the two pad beats absorb the shift; a burst packed with real data would drop a real block — see the scope note).

numMemBlock = 2 here is derived, not a literal: (tensorWidth = blockOut = 16) * (tensorElemBits = outBits = 8) / (memBlockBits = memParams.dataBits = 64) = 2 (TensorUtil.scala:62, store tensorType = "out") — a 128-bit element streamed as two 64-bit sub-blocks {low, high}. The fix gates the wrap on fire and is correct for any numMemBlock.

Precedent (same defect class, load side)

The same class of defect — a DRAM-transfer pointer advancing while the data handshake is de-asserted — was fixed once before on the load path, in 164f3e0d ("LoadUop FSM bug fix", 2020): "The LoadUop FSM incorrectly advances the address of the next uop to read from DRAM when the DRAM data valid bit is deasserted", fixed by folding the address advance into the handshake-guarded section — the same shape as this store-side fix. The store-side tag wrap was not covered then and is still ungated.

---

Related / out of scope (not part of this PR)

• Legacy monolithic store on the separate master branch. The master branch (not the default main) still ships the pre-refactor monolithic TensorStore.scala (256 lines), whose tag wrap at lines 182-186 is the identical ungated when(state === sWriteCmd || tag === (numMemBlock - 1).U) { tag := 0.U }.elsewhen(io.vme_wr.data.fire()) { tag := tag + 1.U }. On main, TensorStore.scala is only a 62-line dispatcher with no tag logic (the datapath was split into TensorStoreNarrowVME / TensorStoreWideVME), so a main grep of TensorStore.scala will not show the bug — it lives in TensorStoreNarrowVME.scala, which this PR fixes. If master is still maintained, the same one-line handshake-gating applies there.
• TensorLoadSimple.scala dead-code look-alike. It has a structurally similar ungated tag wrap on the read side (state === sIdle || state === sReadCmd || tag === (numMemBlock-1).U -> tag := 0), but it is dead code: TensorLoad.scala selects it only under val forceSimpleTensorLoad = false, a hardcoded flag, so no shipped config instantiates it. Not touched by this PR.
• Live load path is unaffected (by inspection). The load path (TensorLoadNarrowVME / TensorLoadWideVME) has no ungated sub-block-select register analogous to the store's tag: the only tag there is the VME transaction id (io.vmeData.bits.tag / io.vmeCmd.bits.tag), not a wrapping sub-block counter, and every scratchpad write is gated on the read-data handshake — vmeDataFirePipe = vmeDataValidPipe & vmeDataReadyPipe (TensorLoadWideVME.scala:100) with the SRAM write enable driven by ShiftRegister(vmeDataFirePipe, ...) (:229). So the ungated-pointer class here cannot arise on the read side; this is verifiable by reading the code, no witness required.