[dv] Use id_done to accurately track instruction monitor#2346
Open
SamuelRiedel wants to merge 1 commit intolowRISC:masterfrom
Open
[dv] Use id_done to accurately track instruction monitor#2346SamuelRiedel wants to merge 1 commit intolowRISC:masterfrom
id_done to accurately track instruction monitor#2346SamuelRiedel wants to merge 1 commit intolowRISC:masterfrom
Conversation
SamuelRiedel
requested review from
andreaskurth,
hcallahan-lowrisc and
rswarbrick
13 tasks
This refactors `instr_vif` to use `rvfi_id_done` instead of `instr_new_id` to track when a new instruction appears in the ID stage. This interface and signal are only used to keep track of instruction fetch errors by using the aforementioned valid signal and checking whether the `rvfi_order_id` has changed. However, if an instruction fetch error is consecutive to another error, `instr_new_id` will be gated, which leads us to miss the `fetch_err` in the verification. This will fix failing `riscv_mem_error_test`
SamuelRiedel
force-pushed
the
fix-mem-error-test
branch
from
6b721e2 to
129bbac
Compare
rswarbrick
approved these changes
Jan 5, 2026
Contributor
rswarbrick
left a comment
rswarbrick
left a comment
There was a problem hiding this comment.
Looks good to me, and thanks for the careful explanation.
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.
This refactors
instr_vifto uservfi_id_doneinstead ofinstr_new_idto track when a new instruction appears in the ID stage. This interface and signal are only used to keep track of instruction fetch errors by using the aforementioned valid signal and checking whether thervfi_order_idhas advanced. However,instr_new_idwill be gated if an instruction fetch error occurs after another error, causing us to overlook thefetch_errin the verification. With this change, we also no longer need to look atvalid_idsince thervfi_id_donealready contains this information.This will fix failing
riscv_mem_error_testin the nightly runs and the failing CI in #2324