Skip to content

fix(riscv): gate SMIR system lift on Zifencei/Zicsr/F/V - #222

Closed
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-lifter-system-gates
Closed

fix(riscv): gate SMIR system lift on Zifencei/Zicsr/F/V#222
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-lifter-system-gates

Conversation

@carlosqwqqwq

Copy link
Copy Markdown
Contributor

fix(riscv): gate SMIR system lift on Zifencei/Zicsr/F/V

Closes #221

Summary

The SMIR system lifter lifted FENCE.I without Zifencei and CSR operations without Zicsr, and modeled the fcsr/vector CSRs without F/V. The extension gates now reject the disabled encodings with InvalidEncoding.

Validation

  • Regression covers fence.i, csrr fcsr, and csrr vl under a minimal profile (must be rejected) and under the full profile (must lift).
  • cargo test --lib: 1 new test passes.

Notes

Translated-path gate, independent from the direct-interpreter CSR gate.

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 SMIR system lifter lifted FENCE.I without Zifencei and CSR ops without Zicsr (and fcsr/vector CSRs without F/V). The extension gates now reject the disabled encodings.

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

I reviewed the focused commit d6eb9f9c against issue #221 and compared it with #216/#203. The Zifencei, Zicsr, F, and V guards belong at the system-lifter boundary and are semantically distinct from #216's configured FP/vector decoder profile fix, so this should remain a separate PR.

Three changes are needed before this is merge-ready:

  1. This PR still contains 55043ddc, the closed refactor(riscv): split vector semantics out of cpu.rs #202 vector-module split. Please rebase onto current master and drop that commit so the PR contains only the focused system-lifter change.
  2. The regression has duplicate adjacent #[test] attributes. The focused filter registers and executes it twice (2/2 with the identical name); please remove one marker.
  3. The current test compares only an all-disabled profile with an all-enabled profile, so it cannot distinguish the new Zicsr gate from the F/V CSR gates. Please add independent profiles: Zicsr enabled with F disabled, Zicsr enabled with V disabled, and F/V enabled with Zicsr disabled. That will directly prove each guard rather than allowing one condition to mask another.

The existing focused controls pass, but the independent gate matrix is needed for the four-condition claim.

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

Thank you @carlosqwqqwq for identifying the SMIR extension-profile gaps and proposing a fix. We independently reimplemented and validated the Zifencei, Zicsr, F, and V gates in #231, where this is consolidated with #218, #220, and #224. No commits from this branch were taken. To keep review and CI on one maintained branch, I’m closing this PR as superseded by #231. You are credited as co-author on the consolidation commit—please review #231, particularly the independent profile combinations.

@19h 19h closed this Aug 11, 2026
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.

SMIR system lifter ignores Zifencei/Zicsr/F/V gates

2 participants