Fix simx jalr target alignment#339
Conversation
There was a problem hiding this comment.
Pull request overview
This PR corrects RISC-V JALR target address alignment in the sim/simx emulator by clearing bit 0 of the computed target address, and adds a focused regression test to prevent the issue from reappearing.
Changes:
- Fix
sim/simxJALR next-PC computation to enforce(rs1 + imm) & ~1. - Add a new
tests/regression/jalrtest (host + kernel + startup) that fails deterministically when JALR doesn’t clear bit 0. - Wire the new regression into the top-level
tests/regression/Makefiletargets (build/run/clean).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
sim/simx/execute.cpp |
Masks off bit 0 when computing JALR target PC to match the ISA requirement. |
tests/regression/Makefile |
Adds the new jalr regression to aggregate build/run/clean targets. |
tests/regression/jalr/Makefile |
Build/run glue for the new regression test. |
tests/regression/jalr/start.S |
Minimal startup that calls main, dumps perf, and terminates. |
tests/regression/jalr/main.cpp |
Host-side test runner that launches the kernel and validates per-core pass signatures. |
tests/regression/jalr/kernel.cpp |
Kernel that uses an intentionally odd JALR target to distinguish correct vs incorrect alignment behavior. |
tests/regression/jalr/common.h |
Kernel argument struct for passing the destination buffer address. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
this bug cannot be created via compiler unless the compiler has a bug.. |
1c34778 to
91e315c
Compare
Thank you for the suggestion. We have moved the tests into tests/kernel/conform tree. fixed in 91e315c |
This fixes JALR target address alignment in sim/simx by clearing bit 0 of the computed target, as required by the RISC-V ISA.
It also adds a regression test under tests/regression/jalr.