Added Instruction_Decode.v #7
Conversation
Added top level module for the Instruction Decode stage, consisting of: - logic for immediate sign extension - logic for selecting register rs1, rs2, and rd from instruction - logic for selecting funct3 and funct7[5] from instruction - instantiating all sub-modules in the stage
Added module for Control FSM of RV32I Processor
Added Verilog modules for the ALU Decoder of the control unit along with the register file of the RV32I processor datapath. Also added is a TCL script generating a waveform of R, I, S, B, J instruction cases
Added Python testbench for the main control FSM of the multicycle RV32I processor, using the CocoTB framework
There was a problem hiding this comment.
The code looks mostly solid and functionally correct - not sure about the cocotb simulations but if you wanted to push some of the results/output files so we can see them as well. Add a separate src/tb directory for testbench code and simulation outputs
Some general comments - things like opcodes or hard-coded constants should generally be avoided - parametrizing them (and using the ../params.vh) to avoid clutter and make sure everyone sees the same parameters is probably for the best in cases like these. Other than that you can add more comments to explain what the outputs/some of the logic is for (especially the input/output ports so people who are interacting with the code know what to do with it)
|
|
||
| end | ||
|
|
||
| else if (instr[6:0] == 7'b0010011) begin //I-Type (excluding lw) |
There was a problem hiding this comment.
Parametrizing the 7-bit opcodes here for I-type/S-type/so on will probably help readability and debugging later
| input clk, | ||
| input regWrite, | ||
| input [31:0] dataIn, | ||
| output reg [31:0] baseAddr, //data read line #1 - from first source register |
There was a problem hiding this comment.
Is there a reason to name the register outputs this instead of like "regA/regB"?
| input wire [31:0] ResultData, | ||
| output wire AdrSrc, | ||
| output wire IRWrite, | ||
| //output wire RegWrite, |
There was a problem hiding this comment.
Regwrite is commented out in the module header but you assign to it in the code - is this deliberate?
src/Instruction_Decode/ALU_Decoder.v
Outdated
|
|
||
| always@(*) begin | ||
|
|
||
| if (ALUOp == 2'b00) ALUControl = 3'b000; |
There was a problem hiding this comment.
Assigning the ALUControl values to explicitly written binary values (3'b000) should probably be changed to parameters (something like ALU_OPCODE_ADD) so that way we can add more of them later and change them around if need be (since the current ones only cover add/sub/and/or/slt) and a full implementation needs more opcodes (xors/shifts/etc)
Added parameters representing opcodes for I, R, S, B, J, and LW types
Update ALU Decoder RTL code with Daniel's refactor
update Instruction Decode top module by instantiating Daniel's new ALU decoder + editing funct7 and funct3 extraction logic
Added top level module for the Instruction Decode stage, consisting of: