Given nested forks, if the inner fork had a `join` or `join_any` at the end,
`V3Sched::transformForks()` would decide that the fork's `VlForkSync` variable
should be passed in from the outside. This resulted in the `VlForkSync` getting
redeclared as a function argument. Ultimately, it led to C++ compilation errors
due to variable redeclaration.
Fixed by rearranging the `if`s that decide whether a variable should be passed
in or left as-is.
Event-triggered coroutines live in two stages: 'uncommitted' and 'ready'. First
they land in 'uncommitted', meaning they can't be resumed yet. Only after
coroutines from the 'ready' queue are resumed, the 'uncommitted' ones are moved
to the 'ready' queue, and can be resumed. This is to avoid self-triggering in
situations like waiting for an event immediately after triggering it.
However, there is an issue with `wait` statements. If you have a `wait(b)`, it's
being translated into a loop that awaits a change in `b` as long as `b` is
false. If `b` is false at first, the coroutine is put into the `uncommitted`
queue. If `b` is set to true before it's committed, the coroutine won't get
resumed.
This patch fixes that by immediately committing event controls created from
`wait` statements. That means the coroutine from the example above will get
resumed from now on.
This makes the implementation of the detection and propagation of the
suspendable property simpler and easier to read. More importantly, there are no
more jumps around the AST with the `visit` functions, which in some cases could
result in incorrect visitor context while in the `visit` function. See the added
test, which would cause Verilator to segfault before this patch.
In testing, verilation performance was not shown to be affected by this change.
Though there is a slight performance improvement from this patch, due to adding
one more check before refreshing class member cache.
Signed-off-by: Krzysztof Bieganski <kbieganski@antmicro.com>