core-v-verif
core-v-verif copied to clipboard
Adding support for RVFI on cv32e40p env
First step toward supporting RVFI based test, done by merging the original env and testbench with the cv32e40x one. For now, only the ci tests are supported
Hi @YoannPruvost. Is this PR related to cv32e40p #717? Should that PR be completed before this one?
Hi @YoannPruvost. Is this PR related to cv32e40p #717? Should that PR be completed before this one?
Hi @MikeOpenHWGroup yes, the pull request on the core should be completed first. This update of the env requires the RVFI to be implemented on the cv32e40p.
Thanks @YoannPruvost. I added the DO NOT MERGE label. We can remove it when cv32e40p #717 has been merged. This PR will also need to update cv32e40p/sim/ExternalRepos.mk to point to the new hash of the cv32e40p repo.
Hi @MikeOpenHWGroup the external repo has been updated so we can merge this PR. Cheers
Thanks for the update @YoannPruvost - much appreciated. I'll approve and merge this once cv32e40p #719 is merged in.
@pascalgouedo, I have (finally) determined why I could not get this PR to work. There is an issue with cv32e40p_rvfi.sv in which a set of members of class insn_trace_t
are written to with non-blocking assignments. It is not legal SystemVerilog to use non-blocking assignments for automatic variables, and by default all object members in SV are automatic. It turns out that Cadence Xcelium allows this, but Metrics Dsim does not. (I believe Dsim's interpretation of the SV LRM is correct, so these should be compile-time errors under Xcelium as well.)
My first attempt to resolve the problem was to remove all non-blocking assignments (<=
) with blocking assignments (=
). This resolved the compile issue with Dsim but introduced functional errors in simulation. My second attempt was to declare all members of class insn_trace_t
to be static variables. This should be safe because as far as I can tell, the objects constructed from this class are "newed" once and only once during a simulation.
I believe that merging this PR will work for Cadence Xcelium, but it will kill our CI flow which runs under Dsim, so I would like to discuss the updates I think are necessary for both the cv32e40p and core-v-verif repos to resolve this. You should have a meeting invitation in your Inbox already.