Search code examples
verilogcpu-registerstest-bench

Assign first register to zero and do not write


I'm a newbie in Verilog trying to create a register file which contains 32-bit registers. I'm able to write and read everything correctly, however, the first register (let's name it as R0) which is at address 5'b00000 must always be equal to 0 and must not be altered at any time. When reading it on a test bench, the problem comes up when R0 suddenly becomes "xxxxxxxx" instead of being 0 or 00000000. The rest of the registers were read correctly. What may I be doing wrong in the code and what could be the workaround for this one? Below is the code:

module regfile (
    clk,
    nrst,
    rd_addrA,
    rd_addrB,
    wr_addr,
    wr_en,
    wr_data,
    rd_dataA,
    rd_dataB
);

//Input and output ports
input wire clk;
input wire nrst;
input wire [4:0] rd_addrA;
input wire [4:0] rd_addrB;
input wire [4:0] wr_addr;
input wire wr_en;
input wire [31:0] wr_data;
output reg [31:0] rd_dataA;
output reg [31:0] rd_dataB;

reg [31:0] regfile[0:31];
integer i;

always @ (nrst)
    begin: RESET
        if(nrst == 0) begin
            for(i = 0; i < 32; i++) begin
                regfile[i] = 0;
            end
        end
    end

always @(rd_addrA or rd_addrB)
    begin: READ
        if(rd_addrA) begin
            case (rd_addrA)
                5'b00000: rd_dataA = regfile[0];
                5'b00001: rd_dataA = regfile[1];
                5'b00010: rd_dataA = regfile[2];
                5'b00011: rd_dataA = regfile[3];
                5'b00100: rd_dataA = regfile[4];
                5'b00101: rd_dataA = regfile[5];
                5'b00110: rd_dataA = regfile[6];
                5'b00111: rd_dataA = regfile[7];
                5'b01000: rd_dataA = regfile[8];
                5'b01001: rd_dataA = regfile[9];
                5'b01010: rd_dataA = regfile[10];
                5'b01011: rd_dataA = regfile[11];
                5'b01100: rd_dataA = regfile[12];
                5'b01101: rd_dataA = regfile[13];
                5'b01110: rd_dataA = regfile[14];
                5'b01111: rd_dataA = regfile[15];
                5'b10000: rd_dataA = regfile[16];
                5'b10001: rd_dataA = regfile[17];
                5'b10010: rd_dataA = regfile[18];
                5'b10011: rd_dataA = regfile[19];
                5'b10100: rd_dataA = regfile[20];
                5'b10101: rd_dataA = regfile[21];
                5'b10110: rd_dataA = regfile[22];
                5'b10111: rd_dataA = regfile[23];
                5'b11000: rd_dataA = regfile[24];
                5'b11001: rd_dataA = regfile[25];
                5'b11010: rd_dataA = regfile[26];
                5'b11011: rd_dataA = regfile[27];
                5'b11100: rd_dataA = regfile[28];
                5'b11101: rd_dataA = regfile[29];
                5'b11110: rd_dataA = regfile[30];
                5'b11111: rd_dataA = regfile[31];
                default: rd_dataA = 16'hXXXX;
            endcase
        end

        if(rd_addrB) begin
            case (rd_addrB)
                5'b00000: rd_dataB = regfile[0];
                5'b00001: rd_dataB = regfile[1];
                5'b00010: rd_dataB = regfile[2];
                5'b00011: rd_dataB = regfile[3];
                5'b00100: rd_dataB = regfile[4];
                5'b00101: rd_dataB = regfile[5];
                5'b00110: rd_dataB = regfile[6];
                5'b00111: rd_dataB = regfile[7];
                5'b01000: rd_dataB = regfile[8];
                5'b01001: rd_dataB = regfile[9];
                5'b01010: rd_dataB = regfile[10];
                5'b01011: rd_dataB = regfile[11];
                5'b01100: rd_dataB = regfile[12];
                5'b01101: rd_dataB = regfile[13];
                5'b01110: rd_dataB = regfile[14];
                5'b01111: rd_dataB = regfile[15];
                5'b10000: rd_dataB = regfile[16];
                5'b10001: rd_dataB = regfile[17];
                5'b10010: rd_dataB = regfile[18];
                5'b10011: rd_dataB = regfile[19];
                5'b10100: rd_dataB = regfile[20];
                5'b10101: rd_dataB = regfile[21];
                5'b10110: rd_dataB = regfile[22];
                5'b10111: rd_dataB = regfile[23];
                5'b11000: rd_dataB = regfile[24];
                5'b11001: rd_dataB = regfile[25];
                5'b11010: rd_dataB = regfile[26];
                5'b11011: rd_dataB = regfile[27];
                5'b11100: rd_dataB = regfile[28];
                5'b11101: rd_dataB = regfile[29];
                5'b11110: rd_dataB = regfile[30];
                5'b11111: rd_dataB = regfile[31];
                default: rd_dataB = 16'hXXXX;
            endcase
        end
    end

always @ (posedge clk)
    begin: WRITE
        if(wr_en == 1'b1) begin
            if(wr_addr != 5'd0) begin
                regfile[wr_addr] = #1 wr_data;
                //$display("%X", regfile[wr_addr]);
            end
            else begin
                $display("R0: %X", regfile[wr_addr]);
            end
        end
    end

endmodule

Thanks a lot for the help.


Solution

  • if(rd_addrA) is interpreted as if(rd_addrA>0). Therefore reading regfile[0] unreachable. Because all entries are valid, you should not need the if-statement.

    Other issues not related to your main question:

    Your code is not synthesizable because regfile is assigned in two different always blocks. You need to merge the reset and write of regfile into one always block. always @(posedge clk) for synchronous reset (recommenced for FPGA). always @(posedge clk or negedge nrst) for asynchronous reset (recommenced for ASIC).

    Specifying the sensitivity in combinational blocks is old style only requited for the 1995 edition. Since 2001 auto selectivity (always @* or the synonymous always @(*)) is the perfected way to start combinational blocks. Module header style recommendations have also changed since the Verilog-1995, so you should read up on that too. Verilog-1995 style is still valid and supported; just no longer perfected.

    Synchronous logic (aka regs assigned by a clock or latch enable) should be assigned with non-blocking assignments (<=). You run a higher risk of behavioral miss-match between RTL and gates if you do not use blocking and non-blocking assignment appropriately.

    Your case statements can be simplified to one line: rd_dataA = regfile[rd_addrA];. But do check your synthesize results as some synthesizers do not optimize as well with this style compared to a case-statement.