Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[svsim] End Verilator simulation on $finish #4702

Merged
merged 1 commit into from
Feb 19, 2025
Merged

Conversation

seldridge
Copy link
Member

Fix a bug in how Verilator simulations work in svsim where the simulation would not terminate when a $finish was called. This seems to be a bug in Verilator where it will not terminate the simulation (as mandated by the Verilog spec) unless you are running with time delays.

Fixes #4700.

Release Notes

Fix an svsim bug, which could manifest in Chiselsim, where when a simulation calls $finish that it does not immediately exit for Verilator backends.

@seldridge seldridge added the Bugfix Fixes a bug, will be included in release notes label Feb 18, 2025
@seldridge seldridge requested a review from jackkoenig February 18, 2025 19:29
Copy link
Contributor

@jackkoenig jackkoenig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM although it looks like the test isn't actually checking the log, it's just printing it. Or is the check the intercepted exception? A comment clarifying would be nice.

executionScriptLimit = None
) { controller =>
controller.run(1)
println(controller.readLog())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be checking something instead of just printing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that this was testing it properly, but it was not. The exception was coming from the simulation shutting down and then the read would fail. This isn't the best test.

I changed that in the force push to count the number of finishes in the simulation log.

Fix a bug in how Verilator simulations work in svsim where the simulation
would _not_ terminate when a `$finish` was called.  This seems to be a bug
in Verilator where it will not terminate the simulation (as mandated by
the Verilog spec) unless you are running with time delays.

Fixes #4700.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge force-pushed the dev/seldridge/issue-4700 branch from c29320b to 2ad9d60 Compare February 19, 2025 05:03
@seldridge seldridge enabled auto-merge (squash) February 19, 2025 05:04
@seldridge seldridge merged commit 36e4957 into main Feb 19, 2025
15 checks passed
@seldridge seldridge deleted the dev/seldridge/issue-4700 branch February 19, 2025 05:23
seldridge added a commit that referenced this pull request Feb 19, 2025
This reverts commit 36e4957.

This introduced nondeterministic build failures due to the simulation
exiting while trying to continue to read commands.  This would be better
fixed by having a dedicated message that indicates simulation failure.
I'll work on a patch that relands this with the suggested fixes.

Signed-off-by: Schuyler Eldridge <[email protected]>
seldridge added a commit that referenced this pull request Feb 20, 2025
This reverts commit 419bb58.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge mentioned this pull request Feb 20, 2025
seldridge added a commit that referenced this pull request Feb 21, 2025
This reverts commit 419bb58.

Fix an issue that can occur now that the simulation will aggressively try
to shut themselves down when they see a `$finish`.  The problem occurs
because svsim is assuming that it can check if the simulation process is
alive and then shut it down.  However, this occurs in a critical region.
Just because the process is alive when checked, doesn't mean it _will_ be
alive when you try to shut it down.

I considered an alternative solutoin here which would put the simulation
into a "finish" state and then respond with a new "finish" message which
the controller would then need to handle. (It would just throw exceptions
if you tried to interact with the simulation after it ended.)  This
approach can be revived.  However, it needs to _also_ figure out what to
do with VCS.

The Verilog spec (1800-2023 Section 20.2) states:

> The `$finish` system task causes the simulator to exit and pass control
> back to the host operating system.

My read of this is that the simulation, if it is allowed to call
`$finish`, can terminate at essentially any time.  Verilator diverges from
this and will allow multiple `$finish` system tasks in the same block,
without time delays, to execute.  Based on how we construct the testbench
for Verilator, we run into this bug.  (If we use time delays, I think
Verilator will behave the same as VCS.)  Adapting the simulation to use a
"finish" message is therefore aligning the simulation with the expectation
of how Verilator behaves and is likely _not_ going to work for VCS.
Therefore, I rejected this approach.

If we want to move in this direction in the future, a change to the
testbench to use a `final` block, or some way of keeping the simulation
alive after a `$finish` to still respond to the controller, may work.

Signed-off-by: Schuyler Eldridge <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Fixes a bug, will be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Chiselsim] Exit simulation on $finish
2 participants