Skip to content
This repository has been archived by the owner on Aug 19, 2024. It is now read-only.

Add Verilator 5.004 to CI #596

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add Verilator 5.004 to CI #596

wants to merge 1 commit into from

Conversation

jackkoenig
Copy link
Collaborator

@jackkoenig jackkoenig commented Jan 7, 2023

This PR is partially to demonstrate that Verilator 5.004 (and probably 5.002 but I haven't tested it) does not work with the Verilator JNA stuff. I have debugged a bit, here's what I know:

Individual tests or tests that pass seem to work, but if you have multiple tests in a row in the same JVM, it will hang when trying to close the NativeLibrary on this line:

Verilator 5 added support for SystemVerilog timing constructs, so my understanding is that it's a massive change to the underlying threading implementation.

My suspicion is that something has changed about how we should terminate the simulation--that there's some child thread running that isn't being stopped and is preventing JNA from closing the shared library. This is purely conjecture though, so take it with a grain of salt.

@ekiwi
Copy link
Collaborator

ekiwi commented Jan 9, 2023

Verilator 5 added support for SystemVerilog timing constructs, so my understanding is that it's a massive change to the underlying threading implementation.

I thought that by default the new timing feature would be off, but could not find any confirmation on that. You could try passing --no-timing.

The number of threads should also be 1 be default: https://verilator.org/guide/latest/exe_verilator.html#cmdoption-threads

@ekiwi
Copy link
Collaborator

ekiwi commented Jan 9, 2023

One possible source of the problem could be that Verilator 5 now makes all simulations "thread-safe", which could mean thread local variables which might not play well with the library loading and unloading. This is more of an uneducated guess though.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants