Difftest: delay step for both emu and simv #283
Conversation
For simv, we delay step to ensure DPIC_step behind DPIC_transfer. For emu, when signal enable is high on current cycle, it will be read by Software, however DPIC will be called next cycle because verilog will use previous cycle signals as always block condition. Such problem is not exposed when step size is fixed, so emu used to step before DPIC. Now we move delay logic to Gateway, so it can be shared by both emu and simv.
There was a problem hiding this comment.
This looks good to me.
I would like add some comments on why this happens. Whether the difftest_step is delayed compared with the DPI-C calls is decided by how the clock is toggled. According to https://github.com/OpenXiangShan/difftest/blob/master/src/test/csrc/verilator/emu.cpp#L589, in single_cycle, the clock is first assigned with 1 followed by an eval() call, which forms the posedge of the clock. In this eval(), DPI-C calls will be evaluated, thus transmitting the difftest values from the previous cycle to the C++ data structures. After single_cycle, however, we are seeing this cycle's difftest_step. This misalignment should be fixed in this PR.
However, I'm wondering why there are not any errors previously. Did we previously test the batch or other modes which change the difftest_step every cycle? Why didn't we find this before?
|
Also, any other signals should we fix for the one-cycle delay? Are there any top-level IOs which has one-cycle latency but we are misusing them previously? |
Previous step size is fixed even with Batch mode, it is batchSize or 0 at every cycle. And difftest_nstep will do nothing is step is 0. So the flow may be step(1th)-DPIC(1th)-step(2nd)-DPIC(2nd). When step size is fixed, step(2nd) may act the same as 1th, even it happens some cycles after DPIC(1th) |
Understood. Make sense. |
The problem with step is the separation of the control side and the data side, which may be easier to cause misalignment? When related IO is used at the same time, the behavior seems more predictive, |
For simv, we delay step to ensure DPIC_step behind DPIC_transfer. For emu, when signal enable is high on current cycle, it will be read by Software, however DPIC will be called next cycle because verilog will use previous cycle signals as always block condition.
Such problem is not exposed when step size is fixed, so emu used to step before DPIC. Now we move delay logic to Gateway, so it can be shared by both emu and simv.