Skip to content

fix(riscv): reject vadc/vsbc with vd == v0 - #198

Closed
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-vadc-vsbc-vd-v0
Closed

fix(riscv): reject vadc/vsbc with vd == v0#198
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-vadc-vsbc-vd-v0

Conversation

@carlosqwqqwq

@carlosqwqqwq carlosqwqqwq commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

fix(riscv): reject vadc/vsbc with vd == v0

Closes #193

Summary

The RISC-V V extension reserves vadc/vsbc when the destination aliases v0, because the carry/borrow-in is read from v0 in every implemented form. vd == v0 now raises an illegal-instruction trap.

Validation

  • CPU regression covers reserved vd == v0 and legal controls for vadc.vv/vadc.vxm/vadc.vim and vsbc.vv/vsbc.vxm (vsbc has no immediate form).
  • 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 shared vd == 0 check looks semantically correct, and the focused unit test passed locally (1 passed; 0 failed). The regression test only exercises the vv encodings, though, while vadc/vsbc are accepted with vd == v0 #193 covers all implemented forms. Please add reserved and legal controls for vadc.vxm, vadc.vim, and vsbc.vxm as well. (vsbc.vi does not exist.)

  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.
vadc/vsbc read the carry/borrow-in from v0, so aliasing v0 as the destination is reserved in every implemented form (vv, vxm, vim; vsbc has no immediate form).
@carlosqwqqwq

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Updated as suggested:

  • Regression extended beyond the vv encodings: reserved vd == v0 and legal controls now cover vadc.vxm, vadc.vim, and vsbc.vxm (vsbc.vi correctly omitted) alongside vadc.vv/vsbc.vv.
  • 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 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.

vadc/vsbc are accepted with vd == v0

2 participants