Skip to content

fix(riscv): reject vmsbf/vmsif/vmsof with vd == vs2 - #197

Closed
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-vmsbf-vmsof-vd-vs2
Closed

fix(riscv): reject vmsbf/vmsif/vmsof with vd == vs2#197
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-vmsbf-vmsof-vd-vs2

Conversation

@carlosqwqqwq

@carlosqwqqwq carlosqwqqwq commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

fix(riscv): reject vmsbf/vmsif/vmsof with vd == vs2

Closes #192

Summary

The RISC-V V extension reserves the mask-prefix operations when the destination aliases the vs2 source register (vd == vs2), and the masked form must not target the mask register itself (!vm && vd == 0). Both cases now raise an illegal-instruction trap.

Validation

  • CPU regression covers vd == vs2 for vmsbf/vmsif/vmsof, masked vd == v0 for all three, and unmasked/masked legal controls.
  • cargo test --lib: cpu suite 72 passed, 0 failed.

Notes

The vector semantics moved to cpu/vector.rs in #202 so this change does not grow the oversized cpu.rs.

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

Review findings:

  1. The new vd == vs2 guard addresses the alias reported by vmsbf/vmsif/vmsof are accepted with vd == vs2 #192, and the focused unit test passed locally (1 passed; 0 failed). However, the same instruction family still accepts a masked prefix operation whose destination is the mask register: for example, masked vmsbf.m v0, v2, v0.t (0x5020a057) has vd != vs2, so it passes the new guard. RVV also requires the destination of masked vmsbf.m/vmsif.m/vmsof.m not to overlap v0.

    Please either extend the validation to cover !vm && vd == 0 and add masked reserved/legal cases, or file this as a high-impact follow-up before merging. Spec: https://docs.riscv.org/reference/isa/unpriv/v-st-ext

  2. Repository policy note: this PR adds to src/isa/riscv/cpu.rs, which is currently 7,221 lines / 264,801 bytes and exceeds both hard split triggers in AGENTS.md. The touched vector semantic group should be split here or in a prerequisite shared refactor.

cpu.rs exceeded the AGENTS.md hard split triggers (2000 lines / 150 kB). Move the vector (RVV) element access and data-path execution group, including set_vtype, into cpu/vector.rs with no behavior change.
The prefix ops write vd while reading vs2; aliasing the source register is reserved, and in the masked form the destination must not be the mask register v0.
@carlosqwqqwq

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Updated as suggested:

  • The masked-form gap is fixed: !vm && vd == 0 now traps for vmsbf/vmsif/vmsof (the destination must not be the mask register).
  • Regression extended: reserved vd == vs2 and masked vd == v0 for all three ops, plus unmasked/masked legal controls. Note the variant is selected by the vs1 field (1=vmsbf, 2=vmsof, 3=vmsif) with funct3=0b010.
  • Repository policy: the touched vector semantic group is split into src/isa/riscv/cpu/vector.rs in the prerequisite refactor refactor(riscv): split vector semantics out of cpu.rs #202; this PR now only touches vector.rs and the test module, so it no longer grows cpu.rs.

Verified locally: cargo test --lib --no-default-features --features smir-jit cpu suite 72 passed, 0 failed.

@19h

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

Thanks @carlosqwqqwq. This fix is being consolidated into rollup PR #203, where it shares centralized direct-interpreter, SMIR/JIT, and QEMU differential coverage with the related RVV corrections. Closing this PR in favor of #203; the relevant rollup commits credit you as co-author.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vmsbf/vmsif/vmsof are accepted with vd == vs2

2 participants