core-v-decoder

[MikeOpenHWGroup]

Task Outcome

• a UVM component that is configurable to accept 32-bit or 64-bit bit-vectors from either an Instruction Fetch bus (OBI or AXI) or an export.
• the bit-vectors are interpreted as RISC-V instructions and mapped to a data structure such as uvma_isacov_instr_c.
• mapped data structures are written to an analysis_port for use by, for example, ISACOV.

Background information

The purpose of this component is to retire use of the Spike dpi_dasm in core-v-verif. Creation of this task is motivated by on-going discussions in issue #1425 and pull-request #1611.

This component would be usable by an core environment.

[silabs-robin]

Just a note here: In the 40s dir (and rvfi agent) we have a lot of "support logic" which we use in formal. Several of the assertion modules also do a bit of decoding. So, if we actually go ahead and create this decoding component, then I think the "logic" of it should be in a file that is re-usable for assertions in formal.

[MikeOpenHWGroup]

In the 40s dir (and rvfi agent) we have a lot of "support logic" which we use in formal. ... the "logic" of it should be in a file that is re-usable for assertions in formal.

Understood. The goal here is to create a new monitor component, but not necessarily replace the RVFI agent, so the existing "support logic" would not be impacted.

[silabs-robin]

the existing "support logic" would not be impacted.

What I was thinking is that I would like if the decoder could be used similar to how the support logic is used today.

Say one needs rs1 of an instruction word, then one's assertion module doesn't need to create new signals and do all the [19:15] stuff itself, it could just refer to the core-v-decoder's logic.

[silabs-robin]

Another comment: The naming scheme of whatever signals will be in this decoder, should preferably have a clear link to what the signal is and does. For instance, among the current isacov signals, there is a distinction between 1) a field's pure bits extracted from the instruction word, vs 2) "expanded" fields like e.g. when a field is [4:1] the actual "value" of the field needs to append a 0 to represent the full [4:0]. Another example is in dasm, where the naming is so unintelligible that you have to look up the definition of the signal each time you want to reason about it.

[MikeOpenHWGroup]

Say one needs rs1 of an instruction word, then one's assertion module doesn't need to create new signals and do all the [19:15] stuff itself, it could just refer to the core-v-decoder's logic.

That is a good idea. As long as we maintain backward compatibility for existing users of uvma_isacov_instr_c, then adding enhancement such as this is fine.

[AyoubJalali]

That is a good idea. As long as we maintain backward compatibility for existing users of uvma_isacov_instr_c, then adding

Or we can make it optional, people can use what ever they want, core-v-decoder or dpi_dasm

[silabs-robin]

I have two comments.

Firstly: Here is the decoder https://github.com/openhwgroup/core-v-verif/tree/cv32e40s/dev/lib/isa_decoder . Let's merge it to master and close this task.

[silabs-robin]

Secondly, about naming schemes.

It is important because the nuances of decoding etc has caused many isacov bugs. (I have expounded on that several times in the past.)

Here is a (slightly edited) copy-paste of this comment https://github.com/openhwgroup/core-v-verif/pull/1895#discussion_r1191019360

---

Proposal:

1. Prefix isadecode_*.
   • So everything decoder-related can be easily grouped and identified.
2. Consistently use the same signal names.
   • E.g. abc_xyz = prefix_abc_xyz.
   • (And NOT splitting it like abc_xyz = abc_prefix_xyz.)
3. Consistenly use exactly the same signal names.
   • E.g. c_rdp = prefix_c_rdp.
   • (Instead of tiny differences like c_rdp = prefix_c_rd, without the "p".)
4. Keep a separation between "raw immediate" vs "interpreted immediate", etc [0]
   • E.g. blabla_raw and blabla_interpreted.
   • (Because toggle coverage of {imm[20:1], 1'b 0} will never be reached.)

Conclusion: c_decode_rd should be isadecode_c_rdp_interpreted. I.e. prefix_signalname_immediatelevel (or something similarly orderly).

---

There are multiple "levels" here...

ISACOV has uses for number 3 and 4, AFAIK.

1. Raw binary word - imm[20|10:1|11|19:12] | rd | opcode
   • (Name is not important. Call it "instr word" or whatever fits the use case.)
2. Raw bit field - imm[20|10:1|11|19:12]
   • (Name is not important, there is no real use case for this.)
3. Sorted bit field - imm[20] | imm[19:12] | imm[11] | imm[10:1]
   • (Call it "bitfield"?)
4. Interpreted immediate - imm[20] | imm[19:12] | imm[11] | imm[10:1] | 1'b 0 (appending 0, or +8, etc)
   • (Call it ???)
5. Applied immediates - E.g. interpreted_immediate + rs1_value
   • (Call it whatever fits the use case, e.g. "target address" etc.)

[silabs-hfegran]

a UVM component that is configurable to accept 32-bit or 64-bit bit-vectors from either an Instruction Fetch bus (OBI or AXI) or an export.

I think an instruction bus-monitor and a disassembler should be kept separately; a UVM-component that digests the bus-activity (and handles misalignment etc) would be a good idea, however it should not be tightly coupled with the disassembler itself, and rather feed data to a simple disassembler mechanism.

The conceptual idea behind the decoder that we implemented is to have one input union/struct that directly maps to the instruction word, and one output union/struct that fully covers the possible combinations of operands from the input structure with human readable (enums), bit-value formats (raw and sorted) and valid-qualifiers for fields. To translate any instruction word to the output representation, a single function call should be sufficient - and all written in synthesizable systemverilog (This is for us a must-have).This should make it relatively straightforward to generate a function that converts this union-representation to a string if needed; and I believe this again could be handled by a relatively simple mechanism.

[MikeOpenHWGroup]

I think an instruction bus-monitor and a disassembler should be kept separately;

Agreed. This requires us to have alignment on the definition of the data structure that represents an instruction. My preference would be to model this as a uvm_sequence_item, but we could also use union/structs. If the union/struct is already defined we could make this a member of the uvm_sequence_item and the functions to map between various representations (e.g. union-representation to a string) can also be members of the uvm_sequence_item.