core-v-verif icon indicating copy to clipboard operation
core-v-verif copied to clipboard

Diff'ing isacov and tracer logs

Open MikeOpenHWGroup opened this issue 4 years ago • 7 comments

This issue is motivated by pull request #636.

That PR brought in updates to the ISA coverage model and the ability to dump-and-diff tracer vs coverage logs. The coverage model updates are necessary and resolve some minor issues affecting properly sampling of decoded instructions. However, the dump-and-diff adds additional complexity to the environment, both in the form of dump utilities in the environment code and additional targets in the Makefiles that are not required to verify the DUT. On the other hand, the dump utilities and targets add a debug capability that has been useful in resolving issues with the ISA functional coverage code.

The question this issue raises is what to do about these dump utilities and Makefile targets. I see three options:

  1. Keep them and maintain them. As discussed in #636, this will either require on-going updates to the format of the tracer logs, or the definition of standard tracer logs that all CORE-V tracers/RVFI must produce.
  2. Ignore them. If unused, the dump utilities and Makefile targets do not affect proper operation of the environment.
  3. Delete them. Keeping unused code in the environment leads to bloat-ware.

For the record, my vote is for option 3, delete.

MikeOpenHWGroup avatar Jun 01 '21 18:06 MikeOpenHWGroup

Thanks for the issue @MikeOpenHWGroup. This is something we need to resolve.

My assumption is that the design folks are going to maintain exactly as you describe: some type of tracing tools that provides a table of instructions executed, including an assembly listing. This would not just be a reiteration of the RVFI fields (although some fields would make sense). I would think anyone integrating the core would want this basic functionality to be present, and perhaps independent of the verification infrastructure.

An example listing:

Time                    Cycle   PC      Instr   Decoded instruction     Register and memory contents
128.000 ns              37 00000080 0000d197 auipc            x3, 0xd000           x3=0000d080
131.000 ns              38 00000084 53018193 addi             x3, x3, 1328         x3=0000d5b0  x3:0000d080

If design is not going to support a tracer, then option #3 above would apply as we simply wouldn't have anything to compare to.

@Silabs-ArjanB @silabs-halfdan any comment on the above?

strichmo avatar Jun 01 '21 21:06 strichmo

My assumption is that the design folks are going to maintain exactly as you describe: some type of tracing tools that provides a table of instructions executed, including an assembly listing. This would not just be a reiteration of the RVFI fields (although some fields would make sense). I would think anyone integrating the core would want this basic functionality to be present, and perhaps independent of the verification infrastructure.

Fully agreed with this. We need some kind of tracing functionality that can be turned on for any simulation so that we can see 'which instructions (i.e. simple disassembly) were actually executed at what time'. I have no requirements on the format (as long as it show the disassembly, pc, and time stamp, and ideally the register or memory update as shown in the example above) or how this tracing would need to be turned on/off.

Silabs-ArjanB avatar Jun 02 '21 06:06 Silabs-ArjanB

I agree timestamp and disassembly would be a useful feature. I'll make a ticket for myself to add this in the RVFI tracer. A standardized tracer log format sounds like a good idea to reduce future maintenance.

halfdan-dolva avatar Jun 02 '21 07:06 halfdan-dolva

I am trying to understand the scope of what makes this problematic. "additional complexity to the environment" should indeed be for essential complexity and not accidental complexity. The coverage agents logging should probably be there anyway, as all of our agents have infrastructure in place for logging capabilities (though I don't know how detailed isacov's logs must be). Adding targets to makefiles might bloat them and make reading and maintenance difficult. One alternative is to have a "utilities.mk" or put it in a script in "bin/".

The complexity might not be as bad as it seems? If we wait until the 40x rvfi implementation is finished, we can decide on how it should standardize(?) its logging format (the tracer might be updated to just probe rvfi signals). For instance, sed 's/\(.*\) \(.*\)/\1/' | awk '{$$1=$$2=$$3=$$4=$$5=""; $$0=$$0; $$1=$$1; print $$0}' is very cryptic to read, but if the log was e.g. tab-separated one could possibly just do awk -F'\t' '{print $$4}'.

I might have overlooked several things, but I think the isacov_logdiff: target is fairly lightweight albeit currently a bit cryptic, and I thing $(POST_TEST) is fairly lightweight too. One could maybe control the dumping/diffing from uvm, but would it add a bit heavier infrastructure as one needs to pass the necessary information (env variables) to it and the sed/awk complexity still remains? I might prefer, without a strong opinion, to wait for the new rvfi-probing tracer and to keep the makefile targets.

silabs-robin avatar Jun 03 '21 09:06 silabs-robin

Good points and questions @silabs-robin. Complexity creeps in step-by-step and eventually becomes problematic. Adding little-used features to the environment will over time lead to "bloat-ware" and few will remember what awk -F'\t' '{print $$4}' is for.

I also prefer waiting for the updated RVFI based tracer as RVFI will be a main-line feature of our cores and the verification environments and just about everyone who works on CORE-V design and verification will know what they are about.

MikeOpenHWGroup avatar Jun 03 '21 13:06 MikeOpenHWGroup

@MikeOpenHWGroup Can we close this issue/question? There has been no activity on this discussion since june 21.

silabs-hfegran avatar Jan 11 '24 13:01 silabs-hfegran

Yes, it seems that this issue has gone stale and we have unconsciously voted for option 2 (ignore the logs). Having said that, @halfdan-dolva made the following comment:

I agree timestamp and disassembly would be a useful feature. I'll make a ticket for myself to add this in the RVFI tracer.

I could not find any such Issue on the CV32E40X repo, but perhaps my search-strings were not creative enough. Before we close this Issue, let's discuss with Halfdan.

MikeOpenHWGroup avatar Jan 11 '24 16:01 MikeOpenHWGroup