🐛 [lane_sequencer] Masked/indexed memory fixes (#455, #459, #462)#468
Open
saurav-fermions wants to merge 4 commits into
Open
🐛 [lane_sequencer] Masked/indexed memory fixes (#455, #459, #462)#468saurav-fermions wants to merge 4 commits into
saurav-fermions wants to merge 4 commits into
Conversation
The index-address operand request feeding the address generator already carries the index element count in its vl field, but it passed scale_vl=1, so the operand requester recomputed the fetch length as (vl << vtype.vsew) >> eew. For indexed ops vtype.vsew is the data SEW and eew is the index EEW, so this is identity only when SEW==EEW. Otherwise: * EEW > SEW: under-fetch -> addrgen waits forever -> hang * EEW < SEW: over-fetch -> operand valid stuck high -> next op corrupts Set scale_vl=0 on both the load (VLXE) and store (VSXE) SlideAddrGenA requests so the already-correct index element count is used as-is. Verified with a Spike-vs-Ara differential (vlxe_sew_eew, vluxei16 + vsew=e8, vl=64, 4 lanes, VLEN=4096): * before: Ara hangs; Spike completes * after: Ara matches Spike (out[i] = mem[idx[i]]) Refs pulp-platform#455
For masked `vslideup`, the MASKU only consumes mask bits whose index is above the slide stride. The lane sequencer tries to skip the whole mask rows below the stride by advancing the MaskM operand's `vstart`, but the expression vstart += ((stride >> NrLanes*ELEN) << NrLanes*ELEN) / 8 shifts by the *value* NrLanes*ELEN (256) rather than its log2, so the term is always 0 and no rows are skipped. The MASKU then reads mask bits 0..stride-1 (typically inactive) instead of the active window, so the slid-in elements are never written. The MaskM operand is fetched with eew=EW64, so the operand_requester adds `vstart` directly to the VRF row address. One row holds NrLanes*ELEN packed mask bits, so the correct skip is `stride / (NrLanes*ELEN)` rows. Verified against Spike: masked `vslideup.vx` by 256 (e8, vl=300) now writes elements 256..299 from the source (Ara previously left them at the destination background value). Refs pulp-platform#459
The MaskM operand request for masked loads and stores set the mask's `vstart` to `vfu_operation_d.vstart` (= architectural vstart / NrLanes). The mask is bit-packed (NrLanes*ELEN bits per VRF row) and fetched with eew=EW64, so the operand requester adds `vstart` directly to the VRF row address (vstart >> (EW64-EW64)). The element-scaled value over-counts: for any architectural vstart in [NrLanes, NrLanes*ELEN) it skips a whole mask row that still holds the active mask bits, so the MASKU delivers the wrong (typically inactive) bits and active elements are dropped. This is most visible through segment memory ops: the segment sequencer breaks them into single-element micro-ops with vstart = element index, so every masked segment load/store with an active element index >= NrLanes silently failed to load/store that element. Use the bit-packed mask-row index `vstart / (NrLanes*ELEN)` instead. The data path already handles vstart correctly (unmasked vstart ops work), and the VLDU/VSTU pick the correct in-row bit once the right row is fetched. Verified against Spike: - masked vle16/vse16 with vstart=4 now load/store element 4; - masked vlseg2e16.v (vl=6) now applies all mask bits incl. lanes 4..5. Refs pulp-platform#462
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
lane_sequencercorrectness fixes, each atomic and accompanied by aSpike-vs-Ara differential reproducer.
vslideup.vxwith large stride uses the wrong mask window #459 — Skip the correct number of whole mask rows for large-stridevslideup, fixing the MASKU reading mask bits0..stride-1instead of the active window above the stride.vlseg2e16.vignores active mask bit 5 and leaves lane 5 undisturbed #462 — Use the bit-packed mask-row index for the masked load/store mask operand, fixing masked memory ops withvstart >= NrLanes(notably segment-load micro-ops) that dropped active elements.Verification
Reproducers
vlxe_sew_eew,vslideup_mask, andvle_vstart_mask/vse_vstart_mask2/vlseg_maskare checked against the Spike golden model andpass on the Verilated Ara model at 2/4/8/16 lanes.
Checklist
Fixes #455
Fixes #459
Fixes #462
Developed with SAIGE, Fermions' autonomous RTL/EDA debugging agent; root-caused, tested, and signed off by the submitter (@saurav-fermions).