search

jlpteaching / dinocpu

A teaching-focused RISC-V CPU design used at UC Davis
BSD 3-Clause "New" or "Revised" License
140 stars 39 forks source link

Added case to catch improper I-type implementation #129

Closed johnzl-777 closed 4 years ago

johnzl-777 commented 4 years ago

The provided alucontrol.scala solution from Lab 1 manages to pass all test cases for checking single-cycle I-type instruction implementation.

This is problematic considering that the alucontrol.scala file did not need any modification to pass cases (external wiring was still necessary but the internals were unmodified). The unmodified alucontrol.scala does not change its behavior when its itype input is set to true and still actively monitors the funct7 set of bits.

Ideally, this lack of a rigorous I-type implementation should be caught by the test cases. However, it is unsurprising it passed with flying colors considering that it takes a very specific set of conditions to trigger wrong behavior.

Before I explain the changes made, I'd like to note two things:

1.) The imm field of most I-type instructions covers the exact same set of bits as funct7 and rs2 in R-Type instructions 2.) The ALU opcodes for the R-type and I-type instructions are identical for related instructions (ADD vs ADDI, XOR vs XORI, etc.)

Now, if we take a look at the ADD and SUB instructions, we find them virtually indistinguishable except for a single bit in funct7 (0000000 vs 0100000). funct3 remains the same for the both of them.

The immediate variant of ADD shares the same funct3 bits as the R-type. However, if the implementations is improper a "magic value" in the imm field can trigger a subtraction by the ALU instead of an addition.

This goes back to the first point made above: the imm field overlaps with funct7 and rs2 for R-type. If funct7 is not truly, fully ignored, an imm value of 1024, which is equivalent to having rs2 be 0 and funct7 be 0100000 in an R-type will cause a subtraction instead of addition.

Hence, the reason for creating the addi-funct7 case and its accompanying RISCV assembly file.

This has not been tested yet (I don't have a cross-compiler on my system) but this seems to be the only feasible way I can think of to catch an improper implementation (at least, based off of the solutions files given)

powerjg commented 4 years ago

@JulianToya, can you review this?