Tweaks to Ibex to get core_ibex UVM to build with VCS #2037
Conversation
This file contains 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
Other code tries to pick up things like DATA_WIDTH through this agent package, that it imports. That doesn't seem unreasonable, but VCS complains because we're not re-exporting it here.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv
Outdated
Show resolved
Hide resolved
dv/uvm/core_ibex/common/ibex_cosim_agent/ibex_cosim_scoreboard.sv
Outdated
Show resolved
Hide resolved
This causes VCS to spit out an error because it's not technically allowed in SystemVerilog. The only things that we needed to import seems to have been the CSR_MHPMCOUNTER3* names. We can just refer to them explicitly.
The previous code caused VCS to complain that the "with" clause didn't use any of the constituent coverpoints. I *think* that VCS wasn't understanding that cp_interrupt_taken[5:4] does indeed depend on cp_interrupt_taken (concentrating on core_ibex_fcov_if for concreteness). Fortunately, the check is easy to express a different way. There, we were just asking that the top two bits are zero. Another way to say that is "if I shift everything else off the bottom, the result is zero". So we say it that way.
The previous version doesn't make sense if you read the classes in exactly the order they are defined in the file. It turns out that this is what VCS did: oops! Fortunately, the fix is pretty trivial: declare the classes the other way around.
Not doing so causes VCS to spit out a warning message. The intention seems to be that the initial call to $value$plusargs will evaluate to true and will put the value that was assigned into the disable_pmp_exception_handler variable, which then gets checked.
|
Incidentally, I'm pleased to say that we were closer than I thought. I just saw a successful test run of |
hcallahan-lowrisc
approved these changes
May 26, 2023
GregAC
approved these changes
May 26, 2023
|
Thanks for this @rswarbrick had been meaning to fix the VCS build for a while! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We're not quite there yet, but it's much closer*! I've put sensible comments on all the commits in the PR, but they are mostly to do with cleaving more carefully to the IEEE SystemVerilog spec and not assuming things that happen to be true for a different simulator.
(*) With the current code, running something like
make IBEX_CONFIG=opentitan SIMULATOR=vcs ISS=spike ITERATIONS=1 SEED=1 TEST=riscv_arithmetic_basic_test WAVES=0 COV=0gets as far as actually generating some binaries. Next: getting sensible trace logs!