riscvarchive / riscv-v-spec

Working draft of the proposed RISC-V V vector extension
https://jira.riscv.org/browse/RVG-122
Creative Commons Attribution 4.0 International
973 stars 272 forks source link

example of vmv.v.* is inconsistent with the definition in spec-1.0.pdf? #947

Open Oxyw opened 8 months ago

Oxyw commented 8 months ago

I noticed that in Appendix A.8 (page 106 of spec-1.0.pdf), there is

vmv.v.x v5, t0, v0.t

However, previously in Section 11.16 (page 55), vmv.v.* instructions were defined to have vm=1 (encoded as unmasked instructions). It seems there might be a mismatch between the example and definition.

dkravitz-sifive commented 8 months ago

Though the instruction is encoded with vm=1, the sentence immediately prior is clear that

the vmv.v.x and vmv.v.i variants splat a scalar register or immediate to all active elements of the destination vector register group where "active" implies taking account of all the mechanisms that could delineate excluded elements: vstart, vl, and mask (section 5.4, p24).

On Mon, Mar 18, 2024 at 6:04 AM WU Xieyuan @.***> wrote:

I noticed that in Appendix A.8 (page 106 of spec-1.0.pdf), there is

vmv.v.x v5, t0, v0.t

However, previously in Section 11.16 (page 55), vmv.v.* instructions were defined to have vm=1 (encoded as unmasked instructions). It seems there might be a mismatch between the example and definition.

— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-v-spec/issues/947, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASDNWSMWJBYMM7YFFOVNMUDYY233XAVCNFSM6AAAAABE3ID6HGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGE4TCNZWGA4TANI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

topperc commented 8 months ago

The assembly parser does not allow v0.t as an operand to vmv.v.x. So the code is invalid.

aswaterman commented 8 months ago

I suppose we could've made vmv.v.x vd, rs, v0.t a pseudo-op for vmerge.vxm vd, vd, rs, v0, though that ship has probably sailed and it doesn't matter much anyway.

In any event, @ericlove fixed this bug last year in 5b04775a9c020bcd76367c20b707e060d4b995e9 as part of a rewrite to use a more accurate algorithm.