🐛 [vlsu/sequencer] Indexed-mem & OoO-retire fixes (#434,#456,#457)#471
Open
saurav-fermions wants to merge 4 commits into
Open
🐛 [vlsu/sequencer] Indexed-mem & OoO-retire fixes (#434,#456,#457)#471saurav-fermions wants to merge 4 commits into
saurav-fermions wants to merge 4 commits into
Conversation
An indexed memory element whose effective address is misaligned to its EEW must raise a load/store address-misaligned exception (RVV spec) and report the faulting effective address in [ms]tval. The addrgen instead reported `riscv::ILLEGAL_INSTR` with tval=0, so software saw the wrong mcause and lost the faulting address. Set the exception cause to LD_ADDR_MISALIGNED / ST_ADDR_MISALIGNED based on the access direction and forward the faulting virtual address (idx_final_vaddr_q) as tval. The cause/tval already propagate to CVA6 via ara_resp.exception. The load/store still has to be drained, so widen the addrgen_illegal_load_o/store_o kill condition to also fire on the misaligned cause (it is really a "kill faulted op" signal for the VLDU/ VSTU drain, not specific to illegal instructions). Verified against Spike with a trap probe: unmasked vluxei32.v with a base+1 index now traps with mcause=4 (LD_ADDR_MISALIGNED) and mtval=faulting address (Ara previously reported mcause=2, mtval=0). Refs pulp-platform#457
A masked indexed load/store must not raise a misalignment exception for an element that is masked off: per the RVV spec, masked-off elements generate neither exceptions nor architectural accesses. Ara trapped because the address generator checked is_addr_error() per indexed element without any visibility into the per-element mask. Route the mask (peek-only; the load/store units still own the handshake) into the addrgen. For a misaligned indexed element whose mask bit is 0, suppress the exception, align the address down to the element width, and emit a normal beat. The VLDU/VSTU already zero the byte strobes of masked-off elements, so the element is dropped exactly like any other masked-off element and the element accounting stays in sync. The mask is resolved in the addrgen only when the whole vector fits within a single mask chunk (vl <= elements-per-chunk); otherwise the conservative trapping behaviour is kept, so there is no regression on multi-chunk vectors. Verified against the Spike golden model (vlxe_masked_misalign, e16/e32/e64): masked-off misaligned element no longer traps and keeps its old value, while an active misaligned element still traps with LD_ADDR_MISALIGNED (cause=4). Refs pulp-platform#456
…O retire
Ara issues in order but retires out of order. The main sequencer's read table
recorded only the *last* instruction reading each vector register, so a writer
built a WAR hazard against just that last reader. A slow earlier reader could
then be overtaken: once the last (fast) reader retired, the writer was allowed
to overwrite the register while the slow reader had not finished reading it,
so the slow reader observed the new value.
Example (the writer to v2 only waited for the fast vadd, not the slow vdivu):
vdivu.vv v6, v4, v2 ; slow, reads v2
vadd.vv v8, v2, v2 ; fast, reads v2 -> overwrote the read-table entry
vadd.vv v2, v12, v12 ; writes v2 -> WAR only vs the fast vadd (bug)
Replace the "last reader id" per register with a per-register bitmask of every
in-flight reader (one bit per instruction id). The WAR build then ORs the whole
reader mask into the writer's hazards, so the writer waits for all pending
readers to retire. This is the maintainer's preferred fix (issue pulp-platform#434): it only
affects writers (no reader-reader serialization), so it does not slow down
read-heavy code such as matmul. The mask decays with vinsn_running, like the
previous valid bit. Writes still track only the last writer (serialized by WAW).
Verified against the Spike golden model with the rar_retire_hazard reproducer
(256-element e32/m2 divide + clobber): unfixed Ara returned 168/256 wrong
elements (sum=1040), the fixed model matches Spike (sum=2048, bad=0).
Regressions: dotproduct (reductions) and fmatmul both still pass.
Refs pulp-platform#434
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three correctness fixes in the address generator / VLSU and the main sequencer,
each atomic and accompanied by a Spike-vs-Ara differential reproducer. Includes
the headline out-of-order-retire hazard fix (#434).
addrgen) — Report a misaligned indexed access asLD/ST_ADDR_MISALIGNEDwith the faulting effective address, instead ofILLEGAL_INSTRwithtval=0.addrgen/vlsu) — Do not raise a misalignment exception for a masked-off indexed element. The addrgen peeks the mask and, when the vector fits in a single mask chunk, aligns the masked-off element's address down and emits a normal beat (the load/store unit's byte strobes already drop it). Resolved per element-lane to stay correct at any lane count.ara_sequencer) — Track all in-flight readers of each vector register (per-register bitmask) instead of only the last one, so a writer builds a WAR hazard against every pending reader. Prevents a later writer from overtaking a slow earlier reader under Ara's out-of-order retirement.Verification
Reproducers
vlxe_misalign_cause,vlxe_masked_misalign(e16/e32/e64), andrar_retire_hazardare checked against the Spike golden model and pass at2/4/8/16 lanes. For #434 the unfixed model was built to confirm the reproducer
fails (
bad=168/256) and the fixed model matches Spike (bad=0); the fix ismeasured cycle-for-cycle performance-neutral on imatmul.
Checklist
Fixes #434
Fixes #456
Fixes #457
Developed with SAIGE, Fermions' autonomous RTL/EDA debugging agent; root-caused, tested, and signed off by the submitter (@saurav-fermions).