Feature request: Always advance clock by one cycle at the end of failed tests

[KasperHesse]

When finishing a test, the very last updates to signals are very hard to identify in the VCD file, as shown in the screenshot attached. While rdData, dmaNum and pktLen do change, their values are not shown.

While the values can be read in the signal listing on the left, they can't be read directly from the waves in GTKWave.

If the clock were to always auto-advance by one clock cycle when a simulation finishes, the final signal updates would also be possible to view in GTKWave. This would also make debugging failed tests easier, as they otherwise stop when a signal does not not have the expected value.
(I recognize that advancing the clock does not really make sense when a test fails. I figure this should be implemented as a "fake" clock cycle, maintaining all signal values for the last cycle solely for the purposes of VCD debugging.

[michael-etzkorn]

Couldn't you simply add c.clock.step() to the end of your test for this?

[KasperHesse]

Obviously that's a solution for tests that work, but for tests that fail, I can't find a way to inject a clock cycle when expect fails and raises an exception.

[michael-etzkorn]

Ah, I see

[ekiwi]

I think this would be a good feature. However, there are some problems that we would have to solve.

First, we need to distinguish between:

1. a failure because of an expect and
2. a failure because an assert or stop statement in the circuit fired.

In case 2, the problem is that the VCS and IVerilog backends will just immediately kill the simulator execution and there isn't really a good way for us to continue the simulation, unless there is an appropriate option for each simulator. On Treadle and Verilator it should be doable to continue executing even after an assertion fires. This is something we worked to enable last summer.

Case 1 is easier because we control the failure in software. I think adding a step around here might work:

chiseltest/src/main/scala/chiseltest/internal/GenericBackend.scala

Line 194 in c600388

Context().env.checkpoint()

. I.e., Context().env.checkpoint() will potentially want to throw an exception and in that case we would have to step once before actually throwing it.

However, there could be some unintended side effects of doing an extra clock step at the end of the test. I am mostly concerned about what should happen if a assert or printf statement in the circuit is active. That way, the user would get shown a printout from a cycle after the test failed which might be confusing. Do you all have any thoughts on what the behavior should be?

[KasperHesse]

I definitely think the circuit should be "idle", such that assert/printf statements do not get toggled. Otherwise, it might muddy the printout in the console

I don't know much about the structure of VCD files, but would it be possible to modify the generated file post-fact, simply extending all waves by one clock cycle?

[ekiwi]

I definitely think the circuit should be "idle", such that assert/printf statements do not get toggled. Otherwise, it might muddy the printout in the console

That will be hard to do in a simulator independent way. However, we might be able to only add this feature for Verilator and Treadle. Feel free to get started on a PR. I won't have time to actually work on any improvements, but I should be free enough to review a PR.

I don't know much about the structure of VCD files, but would it be possible to modify the generated file post-fact, simply extending all waves by one clock cycle?

That could work for VCDs, but we also support LXT, FST and FSDB (for some simulators).