Repository navigation
Keep a finished divide in Execute when a flush arrives during StallM - #1925
Open
davidharrishmc wants to merge 4 commits into
Open
davidharrishmc wants to merge 4 commits into
davidharrishmc wants to merge 4 commits into
Conversation
A divide mispredicted as a jump raises FlushE in its DONE cycle; when StallM held it in Execute the divider FSM went IDLE and restarted with stale operands. Reset the FSMs only when Execute advances; add an opt-in testbench ExternalStall injector and the divflushstall test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: David Harris <David_Harris@hmc.edu>
This was referenced Oct 5, 2026
…v.sv on rv32gc Use FlushE & ~StallE, matching flopenrc, which clears only when enabled. rv64gc divides on the FPU, so add divflushstall32 for rv32gc to exercise div.sv. RV32 self-checking tests live in tests/coverage32 (built by 'make coverage', run by the self-checked coverage32gc suite) so the nightly lockstep run over tests/coverage on rv64gc does not pick them up. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: David Harris <David_Harris@hmc.edu>
StallE equals StallM whenever FlushE is asserted: FlushECause masks the divider's own StallECause, and LatestUnstalledE implies ~StallE. FlushE & ~StallM is therefore the same condition, and it needs no new StallE port through fpu, fdivsqrt, mdu, the core and testbench_fp. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: David Harris <David_Harris@hmc.edu>
davidharrishmc
added a commit
to davidharrishmc/cvw
that referenced
this pull request
Oct 6, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: David Harris <David_Harris@hmc.edu>
rosethompson
reviewed
Oct 6, 2026
| {StallLength, StallDelay} <= InstrM[31:20]; | ||
| else if (StallDelay != 0) StallDelay <= StallDelay - 1; | ||
| else if (StallLength != 0) StallLength <= StallLength - 1; | ||
| assign RVVIStall = (StallDelay == 0) & (StallLength != 0); |
Contributor
There was a problem hiding this comment.
Since we are repurposing the RVVIStall, would it make sense to rename this ExternalStall? It won't break my hardware RVVI because it uses it's own tb and top level.
Contributor
Author
The net now has two drivers, the RVVI wrapper and the stall injector, so name it after the core port it drives. rvvitbwrapper keeps its RVVIStall port. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: David Harris <David_Harris@hmc.edu>
davidharrishmc
added a commit
to davidharrishmc/cvw
that referenced
this pull request
Oct 6, 2026
Brings in the ExternalStall rename of the testbench net. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: David Harris <David_Harris@hmc.edu>
davidharrishmc
marked this pull request as draft
October 6, 2026 22:26
davidharrishmc
marked this pull request as ready for review
October 6, 2026 22:27
This was referenced Oct 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge together with #1930, which contains this PR's commits. The stall injector added here also exposes the bug #1930 fixes: during an injected stall, trap CSRs (mepc/mcause, sepc/scause/stval) can be written for a trap that is never taken. With this PR alone, lockstep reports such a mismatch at a test's final ecall when the injected stall lands there. Merging #1930 merges both; if this PR is merged first, merge #1930 immediately after.
A divide that the BTB predicts as a jump raises FlushE in its DONE cycle. If StallM held it in Execute, the divider FSMs went IDLE and the divide restarted with stale forwarded operands: 100/7 gave 0, and an fdiv could flip sign. The FSMs now reset only when the Execute pipeline register clears (
FlushE & ~StallM, which equalsFlushE & ~StallEbecause a flush clears the divider's own Execute stall), which also makes a coverage exclusion obsolete.This is reachable only through ExternalStall (the RVVI packetizer in the FPGA builds). The PR therefore adds an opt-in testbench stall injector, triggered by the custom-use HINT
slti x0, x0, imm, and two tests:divflushstallon rv64gc, which divides on the FPU (fdivsqrtfsm.sv)divflushstall32on rv32gc, which uses the integer divider (div.sv)rv64gc never exercises
div.sv, and cvw had no RV32 coverage tests, so this addstests/coverage32/, built bymake coverageand run by a self-checkedcoverage32gcsuite in the rv32gc regression. It is a separate directory because the nightly lockstep run executes every ELF undertests/coverage/on rv64gc. Part of #1921.🤖 Generated with Claude Code