So, almost ten years ago to the day, I posted an article on implementing a synchronous FIFO. Well, take the read portion of that FIFO implementation with a grain of salt.

Asynchronous Read

To summarize, here is the portion of the FIFO which implements the memory.

 reg [width-1:0] mem [depth-1:0];
  always @(posedge clk)
    begin
      if (writing)
	mem[wr_ptr] <= wr_data;
      rd_ptr <= next_rd_ptr;
    end

  assign rd_data = mem[rd_ptr];

Here is a timing diagram of writing one piece of data to the FIFO and then immediately reading it out.

Looks great, doesn’t it?

The Problem

This code, however, does not work on Xilinx FPGAs from 6 series and above. Both XST and Vivado will happily implement the equivalent of this for the read logic. And by happily, I mean without error or warning.

always @(posedge clk)
  rd_data <= mem[rd_ptr];

As you and I can tell, that is not the same thing. And it causes the design in my old post to behave in subtly incorrect ways. You can see this when the FIFO is leaving the empty state.

The Solution

To solve this problem we need to implement a bypass register to hold the write data along with an output mux to select the bypass register when we are reading immediately following a write.

Here is the code which does the synchronous read from the RAM.

reg [width-1:0] rd_mem;
  always @(posedge clk)
    rd_mem <= mem[next_rd_ptr];

Here is the bypass register, and the code to determine when the bypass register should be used instead of the RAM output.

reg [width-1:0] bypass_reg;
always @(posedge clk)
  bypass_reg <= wr_data;

reg use_bypass;
always @(posedge clk)
  use_bypass <= writing && wr_ptr == next_rd_ptr;

And here is the output MUX.

assign rd_data = use_bypass ? bypass_reg : rd_mem;

An Alternate Approach

In looking at the Vivado Synthesis Guide (ug901), you can see in the section called RW_ADDR_COLLISION on page 63, a description of an attribute which allows the write data to take priority over the read data. If you synthesize the sync_fifo design with the synchronous_read parameter set to zero or one you will see that the same muxing logic is created either way. With synchronous_read set to one, the muxing is explicit. With it set to zero, then the mux logic is implicit and Vivado will add it for you. It looks like it is safe to use either way. Unfortunately, the attribute is not supported in XST, so there you will need to use the explicit bypass logic with the synchronous read style.

Here is an example of how to set the attribute when you declare the RAM in Verilog.

(* rw_addr_collision = "yes" *)
reg [width-1:0] mem [depth-1:0];

Conclusion

I actually find this pretty shocking that both XST and Vivado generate incorrect code without any error or warning. Clearly, this is a bug and makes me wonder what other constructs it is implementing incorrectly. You can download the complete synchronous FIFO design here.