Tweak ibex_tracer.sv to build with recent versions of Verilator #2075
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
rswarbrick
force-pushed
the
ibex-tracer-verilator
branch
from
0d4fa6c
to
1407fa5
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Recent versions of Verilator complain about the existing code because it updates decoded_str and data_accessed in functions, and Verilator doesn't trace the call graph far enough to realise that these are only triggered from a single process. It then warns that unpredictable scheduling of always_comb makes this hard to predict. To avoid the warning, we schedule explicitly! The trick is to update the two variables with non-blocking assignments on the previous negedge. This works... sort of. Unfortunately, the older version of Verilator that we use in CI was upset with what I originally wrote because it (I think incorrectly) thought I was mixing blocking and non-blocking assignments to the file_handle variable. Gah! This version skirts around that problem by deleting the final block that was calling $fclose() to cleanly close the file handle. As far as I can tell, this happens at the end of simulation anyway so there's no real need to keep it. I suspect this isn't quite true if the Verilator simulation is running inside some other process. In that case, maybe the "I'm finishing a simulation" logic might not call the equivalent of C's atexit(). But I suspect we'll have other problems too, that aren't caused by this change, so I'm not too worried :-)
This seems a little silly, but Verible's "always_ff_nonblocking" check tries to ensure that there are no variable updates using blocking assignments from within an always_ff block. An initialisation as part of a declaration (like we do for file_name_base) is allowed, but the update to the "fh" local variable from the previous commit is not. This seems a little silly to me, but we can cheat here too! We don't want to open the output file until we have something to write, so we run the code that opens the file on the preceding negedge and use a nonblocking assignment to update file_handle, which will have the right value so that we can write out an instruction on the posedge that follows. Bleurgh.
rswarbrick
force-pushed
the
ibex-tracer-verilator
branch
from
1407fa5
to
49f486c
Compare
|
Darn. This ain't gonna work. The cunning trick was to use a non-blocking assignment in But it turns out that VCS doesn't allow non-blocking assignments to dynamic types (here: string). That's quite reasonable of VCS, but it scuppers my grand plan! I'll try to come up with another approach, but I'll drop this PR for now. |
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.
These currently fail, complaining about accesses to
decoded_stranddata_accessed. Verilator doesn't trace through far enough to see that these all come from the samealways_combprocess and can't race with each other.This PR switches things so that we update those two variables with a non-blocking assignment on a clock negedge, then use them on the following posedge. It satisfies Verilator, and should be equivalent in any sensible system.