🐛 [dispatcher] RVV correctness fixes (#436, #452, #453, #460)#467
Open
saurav-fermions wants to merge 6 commits into
Open
🐛 [dispatcher] RVV correctness fixes (#436, #452, #453, #460)#467saurav-fermions wants to merge 6 commits into
saurav-fermions wants to merge 6 commits into
Conversation
vmv.x.s, vcpop.m and vfirst.m are decoded in the OPMVV path, where use_vs1 defaults to 1'b1. For these instructions the rs1 field encodes the sub-opcode rather than a vector register, so leaving use_vs1 asserted makes the main sequencer raise a spurious hazard against whatever register number happens to occupy the rs1 field. Clear use_vs1 in the VWXUNARY0 block, mirroring the sibling 6'b010100 (VMSBF/VMSOF/VMSIF) block that already does this. Refs pulp-platform#436
Add a self-checking app that exercises the VWXUNARY0 spurious-vs1 hazard (pulp-platform#436). vmv.x.s decodes with rs1 = 0, which a buggy dispatcher reads as a false dependency on v0; the app writes v0 immediately before each vmv.x.s reading an unrelated source, in a tight chain. The dominant failure mode is serialization/deadlock rather than data corruption: a hang is caught by the testbench cycle-timeout, while the scalar read-back guards against result corruption. Build with `make bin/vmv_x_s_hazard`; run on Spike (`make spike-run-vmv_x_s_hazard`) or the Verilated model (`make simv app=vmv_x_s_hazard`). Refs pulp-platform#436
The per-vreg EEW tracker (eew_d) only tagged EMUL consecutive registers
from the base vd. Segment loads/stores write (nf+1) groups of EMUL
registers, so vd+1..vd+nf kept the reset-default EW8; a later read at
the real EEW saw a mismatch and triggered an on-read reshuffle that
byte-packed the data across lanes.
Detect segment memory ops (VLE/VLSE/VLXE/VSE/VSSE/VSXE with nf != 0) and
tag (nf+1)*EMUL consecutive destination registers.
Verified with a Spike-vs-Ara differential (vlseg_eew, vlseg4e16 at
e16/m1, 4 lanes, VLEN=4096): after the fix Ara matches Spike on all four
fields {1,2,3,4},{11,12,13,14},{21,22,23,24},{31,32,33,34}.
Refs pulp-platform#453
VXUNARY0 VF8 set `eew_vs2 = eew_q[vs2]`, i.e. the source register's last-written element width. When that tracked eew was wider than the 1/8-width view (e.g. the source was last written at e16), the ZExt8/SExt8 opqueue conversion - which only handles EW8 - falls through and the operand is forwarded unconverted, producing a wrong result. vf8 is only legal at SEW=e64, so the source elements are always EW8. Force `eew_vs2 = EW8`, mirroring how VF4/VF2 already use the computed narrower width rather than the tracked eew. Verified against Spike: `vzext.vf8` of e16 0xa04f now yields v14[0]=0x4f, v14[1]=0xa0 (Ara previously returned 0xa04f, 0). Refs pulp-platform#452
A masked vector instruction's destination group must not overlap the mask source v0 (RVV spec), unless it writes a mask or scalar result. vmerge and vfmerge are always masked by v0 and write a full vector, so vd=v0 is illegal. Ara executed it instead of trapping. Raise illegal-instruction when the merge form (vm=0) targets vd=v0, across the OPIVV/OPIVX/OPIVI integer encodings and the OPFVF float encoding. The vmv.v.*/vfmv.v.f forms (vm=1) share the funct6 but are unmasked and remain legal with vd=v0. Verified against Spike with a trap probe: vmerge.vim v0, v20, -2, v0 now traps with mcause=2 (illegal instruction), while the preceding legal vmv.v.x v0 setup does not trap. Refs pulp-platform#460
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.
Four dispatcher correctness fixes, each atomic and accompanied by a Spike-vs-Ara
differential reproducer (added under
apps/, onlymain.ctracked).use_vs1for VWXUNARY0 (vmv.x.s,vcpop.m,vfirst.m) so a stale vs1 read does not create a spurious hazard.vlseg<NF>e<EEW>withNF > 1andEEW != EW8corrupts vd+1..vd+nf on subsequent reads #453 — Tag the fullvd..vd+nfdestination range for segment loads/stores in the EEW tracker, fixing corruptedvd+1..vd+nfread-back.vsext.vf8/vzext.vf8source element width to EW8 instead of the tracked eew.vmerge.vimwith vd=v0 and v0.t does not raise illegal-instruction exception #460 — Reject a maskedvmerge/vfmergewhose destination isv0(overlaps the mask source) as illegal-instruction;vmv.v.*/vfmv.v.f(vm=1) stay legal.Verification
Each fix has a reproducer checked against the Spike golden model (
vmv_x_s_hazard,vlseg_eew,vzext_vf8,vmerge_v0_illegal); all pass on the Verilated Ara modelat 2/4/8/16 lanes.
Checklist
Fixes #436
Fixes #452
Fixes #453
Fixes #460
Developed with SAIGE, Fermions' autonomous RTL/EDA debugging agent; root-caused, tested, and signed off by the submitter (@saurav-fermions).