🐛 [masku] Mask-unit correctness & deadlock fixes (#446 #448 #450)#469
Open
saurav-fermions wants to merge 6 commits into
Open
🐛 [masku] Mask-unit correctness & deadlock fixes (#446 #448 #450)#469saurav-fermions wants to merge 6 commits into
saurav-fermions wants to merge 6 commits into
Conversation
vid.v does not consume an ALU operand, but the lanes can still drive ALU data into the mask-unit operand path. With the operand never acknowledged, the producing lane stalls indefinitely and the mask unit deadlocks. Acknowledge the ALU operand unconditionally while the issued instruction is VID so the operand path drains. Refs pulp-platform#448
Self-checking reproducer for the mask-unit ALU-operand drain bug (pulp-platform#448): vid.v followed by a mask compare and a masked merge. Verified on the Verilated model (4 lanes, VLEN=4096): * with the masku_operands.sv fix: PASS (masked merge writes 9.0 to elem 2) * without the fix: FAIL (merge dropped, elem 2 stays 7) The drain bug manifests here as a wrong masked-merge result; under different timing it can also deadlock. Refs pulp-platform#448
…-platform#448) The vid.v hang reported in pulp-platform#437 (widening multiply followed by vid.v at LMUL=4) shares the mask-unit ALU-operand-drain path fixed for pulp-platform#448. Confirmed on the Verilated model (4 lanes, VLEN=4096): * without the masku_operands.sv pulp-platform#448 fix: hangs (testbench timeout) * with the fix: completes (vid_m4=1) Refs pulp-platform#437 pulp-platform#448
vcpop.m and vfirst.m fed the full NrLanes*DataWidth mask word to the
popcount/lzc units without masking tail bits beyond vl. With vl < VLMAX
the agnostic tail bits were counted/scanned, so vcpop returned the slice
width instead of vl and vfirst could report a 1 past the vector end.
Zero operand bits at index >= (issue_cnt_q + in_ready_cnt_q*delta_elm_q),
which reconstructs the valid element count for the current word.
Verified with a Spike-vs-Ara differential (vcpop_vl app, 4 lanes,
VLEN=4096):
* before: Ara vcpop = {16,16,16,16,16}, Spike = {1,2,4,8,16} (diverge)
* after: Ara vcpop = {1,2,4,8,16} == Spike (match)
Refs pulp-platform#446
vcompress drives two completion markers through per-element FIFOs: vcompress_last_idx (index FIFO -> masku completion) and is_last_req (request FIFO -> lane VRGATHER FSM return-to-IDLE). Both were set on the last *input* element (vrgat_cnt == vl-1), but the FIFOs are only pushed for *selected* elements. When the trailing input elements are unselected, neither marker is ever pushed, so the masku never retires and the lane FSM never leaves REQUESTING (blocking the next vcompress) -> hang. Tag the last *selected* element instead: a selected element is the last output iff no mask bit is set in (vrgat_cnt, vl). The whole mask is in masku_operand_alu_seq when vl fits in one word, so look ahead there and set both markers on that element's FIFO entry. For vl beyond one word keep the legacy behavior. Root-caused via FST waveform analysis (the masku scan completes and the index FIFO drains, but vcompress_issue_end never asserts; separately the lane FSM stays in REQUESTING for want of is_last_req). Verified with a Spike-vs-Ara differential sweep (vcompress_sweep, 4 lanes, VLEN=4096): trailing-unselected, last-selected, all-selected, all-unselected, multi-word vl=20, and back-to-back ops all match Spike; before the fix the trailing-unselected case hangs. Refs pulp-platform#450
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 mask-unit correctness fixes (two are liveness/deadlock bugs), each atomic
and accompanied by a Spike-vs-Ara differential or self-checking reproducer.
vcpop.m/vfirst.moperand to the active element range, fixing wrong counts whenvl < VLMAX.vcompresselement as the terminator (last_idx/is_last_req), fixing a completion hang when trailing elements are unselected.vid.vto avoid a lane deadlock (also closes vid.v causes hang in vsim/verilator #437, thevid.vLMUL=4 hang).Verification
Reproducers
vcpop_vl,vcompress_sweep,vid_drain_deadlock, andvid_m4_hangare checked against the Spike golden model (or self-check + terminate for the
deadlock cases) and pass on the Verilated Ara model at 2/4/8/16 lanes.
Checklist
Fixes #446
Fixes #448
Fixes #437
Fixes #450
Developed with SAIGE, Fermions' autonomous RTL/EDA debugging agent; root-caused, tested, and signed off by the submitter (@saurav-fermions).