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.
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.