C Extension Decoding

[ben-marshall]

Hi there

I think there are some mismatches between riscv-formal and the spec (v20190305-Base-Ratification) with the C extension decoding.

• Some C instructions require that operand fields take a specific value in order to be correctly decoded.
• This means that either rs1/rd != 0, or rs2 != 0`.
• A subset of the RV32 C instructions in riscv-formal do not perform these check on the operand fields.

The following instructions are missing operand-field value checks for rd/rs1!=0 in their spec_valid signal assignment:

• c.li
• c.lui
• c.addi
• c.slli
• c.mv
• c.add

The checks are missing both in the generated code insns/insn_*.v and in the generator code insns/generate.py.

If you agree this is a problem, I'm happy submitting a PR to fix it, though what that would do to cores currently using / passing the checks I can't say.

[cliffordwolf]

Just looking at the first one: The spec says that C.LI with rd=0 is a hint instruction, not a reserved opcode.

If your core decodes C.LI with rd=0 as illegal instruction then your core is broken.

This models are formally verified against Spike. I f their behavior is incorrect then so is the behavior of spike. And spike is the golden reference implementation.

[cattius]

Hi Clifford,

C.LI with rd=0 and the other documented hint instructions certainly shouldn't be decoded by any core as illegal instructions, but would it not be better to distinguish them in riscv-formal as distinct instructions from C.li, C.lui, etc., for testing? Given the spec says they must either act as no-ops or have a defined hint implementation, but must never modify architectural state, conceptually it seems wrong to define them as having an architectural effect. Of course in practice having rd=0 should mean that the instructions do nothing and are equivalent to no-ops, but not distinguishing the hint instructions might potentially lead to insidious bugs - perhaps an x0 register in a software emulator which incorrectly can be written to, but would still pass these tests? It seems a concerning precedent to just ignore the hint instructions. Outside of riscv-formal I've found this has led to issues in other tools, such as disassemblers incorrectly decoding them as either compressed instructions or illegal instructions, and I can imagine a developer assuming that they could use riscv-formal's instruction definitions as references for developing their own tool.

I've been testing various RISC-V implementations for (inadvertent) undocumented instructions recently, so these kind of bugs slipping through verification have been on my mind. Would be interested to hear your thoughts on this - thanks!

[cliffordwolf]

C.LI with rd=0 and the other documented hint instructions certainly shouldn't be decoded by any core as illegal instructions, but would it not be better to distinguish them in riscv-formal as distinct instructions from C.li, C.lui, etc., for testing?

That would be possible, but I don't see it adding anything. All hint instructions already are NOPs when interpreted as regular instruction, and that's exactly what all cores do, at least until actual hint instructions are standardized.

There are no additional properties to assert for hint instructions. So what would we gain?

Given the spec says they must either act as no-ops or have a defined hint implementation, but must never modify architectural state

And that's exactly the semantic you get from riscv-formal.

Last time I checked spike is also doing the same thing.

are equivalent to no-ops, but not distinguishing the hint instructions might potentially lead to insidious bugs - perhaps an x0 register in a software emulator which incorrectly can be written to, but would still pass these tests?

Writing anything that isn't 0 to x0 would be identified as bug by riscv-formal. And that has nothing to do with hint instructions at all. It's the same if you have uncompressed instructions that are effectively NOPs.