Simple pipelined processor designing issues

Question

I am practicing Verilog HDL programming and I cannot fix the problem of this simple 5-staged pipeline processor. The problem I am facing is in th LD instruction which loads double word from memory into register. I am failing at saving the output data from each stage. How can I save the output data for next stage? Can you point out my mistake in the code below:

module Processor(
    input CLK,
    input [31:0] Instruction,
    input [63:0] Data_In,
    output [31:0] Inst_Addr,
    output [31:0] Data_Addr,
    output [63:0] Data_Out,
    output Mem_Read,
    output Mem_Write
);

////////////////////////////////////////////

reg[31:0] pc;
reg[31:0] regs[0:31];

reg[31:0] stage1;

reg[31:0] data_addr;
reg[63:0] data_out;
reg[63:0] data_in;
reg[31:0] inst;

reg [5:0] funct;
reg [5:0] opcode;
reg [4:0] rs;
reg [4:0] rt;
reg [4:0] rd;
reg [15:0] imm;
reg [4:0] shamt;    
reg [1:0] inst_type;

reg [31:0] rs_value;
reg [31:0] rt_value;
reg [31:0] rd_value;
reg mem_read, mem_write;

initial begin
    pc = 0;
end

assign Inst_Addr = pc;

always @ (posedge CLK) begin    
    /////////////IF//////////////////////////////
    pc = pc + 4;
    $display("%h Fetched instruction: %h", Inst_Addr, Instruction);
    stage1 = Instruction;
    //////////////ID//////////////////////////////  

    opcode[5:0] = stage1[31:26];

    case(opcode)
        0:
            begin /* R-type */
                assign rs = stage1[25:21];
                assign rt = stage1[20:16];
                assign rd = stage1[15:11];
                assign shamt = stage1[10:6];
                assign inst_type = 2'b00;
            end
        default:
            begin /* I type */
                assign rs = stage1[25:21];
                assign rt = stage1[20:16];
                assign shamt = stage1[10:6];
                assign imm = stage1[15:0];
                assign inst_type = 2'b01;
            end
    endcase

    assign funct = stage1[5:0];

    /////////////EX////////////////////////////////




    // Fetching operands
    assign rs_value = regs[rs];
    assign rt_value = regs[rt]; 

    // Performing operations
    assign data_in = Data_In; // LD 

    case(inst_type)
        2'b00: /* R-type */
            begin
                case(funct)
                    6'h20: /* DADD */
                        begin
                            assign rd_value = rs_value + rt_value;
                        end
                    6'h22: /* DSUB */
                        begin
                            assign rd_value = rs_value - rt_value;
                        end
                    6'h26: /* XOR */
                        begin
                            assign rd_value = ~ (rs_value | rt_value);
                        end
                endcase
            end
        2'b01: /* I-type */
            begin
                case(opcode)
                    6'b110111: /* LD */
                        begin
                            assign data_addr = rs + imm;
                            assign rt_value = data_in;                      
                            assign mem_read = 1;
                            assign mem_write = 0;
                        end
                    6'b111111: /* SD */
                        begin
                            assign data_out = rt_value;
                            assign data_addr = rs + imm;
                            assign mem_write = 1;
                            assign mem_read = 0;
                        end
                    6'b011000: /* DADDI */
                        begin
                            assign rt_value = rs_value + imm;
                        end
                endcase
            end
    endcase

    //LD, SD,DADD, DSUB, XOR, DADDI, HALT           



    ///////////////WB///////////////////////////////

    case(inst_type)
        2'b00: /* R-type */
            begin
                regs[rd] = rd_value;
            end
        2'b01: /* I-type */
            begin
                case(opcode)    
                    6'b110111: /* LD */
                        begin
                            regs[rt] = rt_value;
                        end
                    6'b011000: /* DADDI */
                        begin
                            regs[rt] = rt_value;
                        end
                endcase
            end
    endcase
end

//////////////MEM//////////////////////////////

    assign Data_Addr = data_addr;

    assign Data_Out = data_out; // SD
    assign Mem_Write = mem_write; 


    assign Mem_Read = mem_read; 


////////////////////////////////////////////////
endmodule

Answer 1

Generally you do not use assign with registers, there are valid cases explained here. The following is not one of them:

reg data_in;
always @* begin 
  assign data_in = Data_In; // LD 
end

Should be:

reg data_in;
always @* begin 
  data_in = Data_In; // LD 
end 

Most of your variable assignments happen in-side an edge triggered block (implied flip-flop). In this case you should really be using non-blocking assignments (<=). What you end up with is:

reg data_in;
always @(posedge clk) begin 
  data_in <= Data_In; // LD 
end 

I suspect that some parts (below) of the design should be put into a separate combinatorial section so that they are working in parallel to the addr decodes, and not effect by delays.

// Fetching operands
assign rs_value = regs[rs];
assign rt_value = regs[rt]; 

The following is highlighting the above in more detail

always @ (posedge CLK) begin    
//...

case(opcode)
  0: begin /* R-type */
    assign rs        = stage1[25:21];
    assign rt        = stage1[20:16];
    assign rd        = stage1[15:11];
    assign shamt     = stage1[10:6];
    assign inst_type = 2'b00;
  end
  default: begin /* I type */
    assign rs        = stage1[25:21];
    assign rt        = stage1[20:16];
    assign shamt     = stage1[10:6];
    assign imm       = stage1[15:0];
    assign inst_type = 2'b01;
  end
endcase
//...

should be:

always @ (posedge CLK) begin    
//..

case(opcode)
  0: begin /* R-type */
    rs        <= stage1[25:21];
    rt        <= stage1[20:16];
    rd        <= stage1[15:11];
    shamt     <= stage1[10:6];
    inst_type <= 2'b00;
  end
  default: begin /* I type */
    rs        <= stage1[25:21];
    rt        <= stage1[20:16];
    shamt     <= stage1[10:6];
    imm       <= stage1[15:0];
    inst_type <= 2'b01;
  end
endcase
//...

While it is possible to mix blocking (=) and non-blocking (<=) in edge trigged always blocks I would avoid until you better understand the implications of coding like this. I personally never mix the two styles so that the code is easier to read.

Therefore I would separate out any code which is to happen in the same cycle (combinatorial). An example of this would be adding the following in parallel with the clocked process.

always @* begin
  // Fetching operands
  rs_value = regs[rs];
  rt_value = regs[rt];
end

always @(posedge CLK) begin
end