Skip to content

fix(riscv): count only retired instructions in instret - #233

Closed
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-ecall-ebreak-instret
Closed

fix(riscv): count only retired instructions in instret#233
carlosqwqqwq wants to merge 2 commits into
HexRaysSA:masterfrom
carlosqwqqwq:fix-riscv-ecall-ebreak-instret

Conversation

@carlosqwqqwq

Copy link
Copy Markdown
Contributor

fix(riscv): count only retired instructions in instret

Closes #232

Summary

The retired-instruction counter counts instructions that retire. An ECALL or EBREAK leaves the interpreter through a non-Continue exit and must not be counted as a normally retired instruction. The affected step() path increments instret after every successful execute() return, including Ecall and Ebreak; the JIT fallback repeated the same rule. This makes counter observations differ between an architectural guest and RAX, and can affect code using instret for profiling or progress accounting.

The fix increments instret only for Continue and Wfi exits, while preserving the existing cycle counter increment.

Validation

  • Targeted test ecall_ebreak_do_not_increment_instret fails on the base checkout and passes with the fix.
  • cargo test --lib: full library test suite passes with no regressions.

No new upstream test files are added; this is a source-only change.

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.

19h commented Aug 11, 2026

Copy link
Copy Markdown
Member

Thank you for reporting and proposing this fix. I independently reimplemented and validated precise instret accounting for normal completion, ECALL, EBREAK, traps, and JIT fallback in the consolidation PR #231; no commits or code from this branch were taken. The rollup carries @carlosqwqqwq as co-author on its commits and includes direct and native regression coverage. I’m closing this PR as superseded by #231—please review the consolidated implementation there.

@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.

ecall/ebreak incorrectly increment the retired-instruction counter

2 participants