Syncronous FIFO - flag generation

[std_match]

I don't see how it can be "unsafe"...

It not "unsafe" in the meaning "may not work". I mean that you don't know what the synthesis tool will do. Do you want to write code that can generate 1 or 2 adders, when it is enough with 1 adder?

 

[shaiko]

It not "unsafe" in the meaning "may not work". I mean that you don't know what the synthesis tool will do.

Clear.

Regarding this process:

write_flags_and_queue : process ( clock , reset ) is
begin
   if reset = '1' then       
      full <= '0' ;
      write_pointer <= ( others => '0' ) ;	
   elsif rising_edge ( clock ) then
      if write_request = '1' and full = '0' then
         write_pointer <= write_pointer + 1 ;
      end if ;
      if 
      ( write_pointer_next ( write_pointer_next ' high - 1 downto 0 ) = 
      read_pointer ( read_pointer ' high - 1 downto 0 ) ) and 
      ( write_pointer_next ' high /= read_pointer ' high ) then
      full < = '1' ;
   else
      full < = '0' ;
   end if ;
end process write_flags_and_queue ;

Please point out why it won't work with a combined read/write operation...

 

[std_match]

Please point out why it won't work with a combined read/write operation...

The "full" flag can be set even if there is a read request in the same clock cycle.

 

[TrickyDicky]

The problem, as has already been pointed out, is that the full and empty flags are delayed by 1 clock compared to the read and write pointers.

 

[std_match]

The problem, as has already been pointed out, is that the full and empty flags are delayed by 1 clock compared to the read and write pointers.

shaiko is now comparing with "write_pointer_next" which is assigned "write_pointer + 1" outside of a clocked process, so it is correct (except for simultaneous read and write).

 

[shaiko]

I'm simulating the design with simultaneous read/write and can't make it fail.
Please point out the conditions in which you think it will fail.
For example, when the fifo is one address from being full and a simultaneous write/read request occurs, when the fifo is one address from being empty and a simultaneous read/write occurs, etc...

Help me find the exact failure mode in the current code.

 

[std_match]

I'm simulating the design with simultaneous read/write and can't make it fail.
Please point out the conditions in which you think it will fail.
For example, when the fifo is one address from being full and a simultaneous write/read request occurs, when the fifo is one address from being empty and a simultaneous read/write occurs, etc...

It is exactly the cases you mention. The "empty" and "full" flags will be set for one clock cycle in those situations. Maybe "fail" is a strong word for this, but it is not optimal.

 

[shaiko]

Thanks,
I'll retest it.

- - - Updated - - -

Found it!
And your previously suggested code does fix it:

signal write_pointer, read_pointer, write_request, read_request;
variable new_write_pointer;
 
-- write process
if reset = '1' then
  full <= '0' ;
  write_pointer <= ( others => '0' ) ;  
elsif rising_edge(clock) then
  new_write_pointer := write_pointer + 1;
  if write_request = '1' and full = '0' then
    write_pointer <= new_write_pointer;
    if new_write_pointer = read_pointer then
      full <= '1';
    end if;
  end if;
  if read_request = '1' then
    full <= '0';
  end if;
end if;

I was also thinking,
What if we to take the flag generation circuit out of the clocked process?
Hence:

write_queue : process ( clock , reset ) is
begin
   if reset = '1' then       
      write_pointer <= ( others => '0' ) ;	
   elsif rising_edge ( clock ) then
      if write_request = '1' and full = '0' then
         write_pointer <= write_pointer + 1 ;
      end if ;
   end if ;
end process write_queue ;



full <= '1' when
      ( write_pointer_next ( write_pointer_next ' high - 1 downto 0 ) = 
      read_pointer ( read_pointer ' high - 1 downto 0 ) ) and 
      ( write_pointer_next ' high /= read_pointer ' high )
      else '0' ;

Would it work?

 

[TrickyDicky]

It would work, but having unregistered outputs will obviously add to the delay for any other circuits that use the full or empty flag.

 

[shaiko]

Is this fail proof ?

entity fifo_management is													
port								
( 		
   clock : 	in std_logic ;           
   reset : 	in std_logic ;           
   write_request :  in std_logic ;           
   read_request :   in std_logic ;           

   full :		out std_logic ;           
   empty :  	out std_logic ;   		  
   data_out : 	out std_logic_vector ;
   read_address : 	out std_logic_vector ( 2 downto 0 ) ;
   write_address : 	out std_logic_vector ( 2 downto 0 ) 
) ;       	
end entity fifo_management ;



architecture rtl_fifo_management of fifo_management is

signal new_write_pointer : unsigned ( 3 downto 0 ) ;
signal new_read_pointer : unsigned ( 3 downto 0 ) ;

begin
	
	
	
   write_address <= write_pointer ( write_pointer ' high - 1 downto 0 ) ;
   read_address <= read_pointer ( read_pointer ' high - 1 downto 0 ) ;
   
   new_write_pointer <= write_pointer + 1 ;	
   new_read_pointer <= read_pointer + 1 ;	

   write_domain : process ( clock , reset ) is
      begin
         if reset = '1' then    
            write_pointer <= ( others => '0' ) ;
            full <= '0' ;
         elsif rising_edge ( clock ) then
            if write_request = '1' and full = '0' then
               write_pointer <= new_write_pointer ;
	   if new_write_pointer = read_pointer then
	      full <= '1' ;
	   end if ;	
	end if ;
          if read_request = '1' then
             full <= '0' ;
          end if ;	
       end if ;
   end process write_domain ;
	
 
	
   read_domain : process ( clock , reset ) is
      begin
         if reset = '1' then    
            read_pointer <= ( others => '0' ) ;
            empty <= '0' ;
         elsif rising_edge ( clock ) then
            if read_request = '1' and empty = '0' then
               read_pointer <= new_read_pointer ;
	   if new_write_pointer = read_pointer then
	      empty <= '1' ;
	   end if ;	
	end if ;
          if write_request = '1' then
             empty <= '0' ;
          end if ;	
       end if ;
   end process read_domain ;
 
	
	
end architecture rtl_fifo_management ;

 

[TrickyDicky]

Why not write a testbench and find out?
It looks ok, but I would still hammer it with a good random testbench.

 

[std_match]

Is this fail proof ?

No, you have 2 typos in the read process in the following line

	   if new_write_pointer = read_pointer then

 

[TrickyDicky]

No, you have 2 typos in the read process in the following line

	   if new_write_pointer = read_pointer then

Hence the need for a good testbench - syntax checking and testing via forum is a risky development method.

 

[Miralipoor]

Hi
Both of them are extremely inefficient. Actually it is better to use your code on CPU rather that FPGA.Your architectures didn't have characteristic of hardwired architecture.Fifo simply implemented with SR-16 and one additional FF can keep trace of last corrupted place in fifo.Status of FiFo(IsEmpty,IsFull) is a simple function of state of additional FF in head of FiFo or tail of it.

 

[TrickyDicky]

Can you please explain a little clearer?

 

[shaiko]

Miralipoor,
Please elaborate...
A code example will make things much clearer

 

[Miralipoor]

Hi
it is a sample of my library

---

Code VHDL - [expand]
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
--In the name Of *****
--Author Mehdi Miralipoor
--Description : High performance Fifo Module
--Date:
--Revision:
Library IEEE;
use IEEE.std_logic_1164.all;
use IEEE.std_logic_unsigned.all;
Library UNISIM;
use UNISIM.vcomponents.all; 
Entity HFifo is
  Generic (DataWidth:natural:=8;Depth:natural:=10);
  port
  (
   clk,rst:std_logic;
   DataOut:out std_logic_vector(DataWidth-1 downto 0);
   DataIn:in std_logic_vector(DataWidth-1 downto 0);
   pop:in std_logic;
   push:in std_logic;
   isFull:out std_logic;
   isnEmpty:out std_logic
  );
end HFifo;
Architecture RTL of HFifo is
signal beans:std_logic_vector(Depth-1 downto 0);
type  fifoArrayT is Array(Depth-1 downto 0) of std_logic_vector(DataWidth-1 downto 0);
signal fifoArray:fifoArrayT;
signal en:std_logic_vector(Depth-1 downto 0);
begin
  isnEmpty<=beans(0) ;
  isFull<=  beans(Depth-1) and beans(Depth-2);
  en(0)<=pop or not(beans(0));
  ens_ins:for  i in Depth -2 downto 1 generate
    en(i)<=(not (beans(i-1)) )or (not (beans(i)))  or pop ;
  end generate;  
  en(Depth-1)<=(not beans(Depth-2)) or (push and (not beans(Depth-1))) or pop  ;
 
 
    beanDepthMDepth_1 : FDCE_1
                port map (
                     Q  => beans(Depth-1),
                     C => clk, 
                     CE => en(Depth-1),
                     CLR => rst,
                     D => push
                     );  
    
 
  
  beans_ins:for  i in Depth -2 downto 0   generate
            BeansFDCE :  FDCE_1
                port map (
                     Q  => beans(i),
                     C => clk, 
                     CE => en(i),
                     CLR => rst,
                     D =>  beans(i+1)
                     );  
 
    end generate;                       
 
   WordGenerate0:for j in  DataWidth-1 downto 0 generate
         Word0FDCE : FDCE_1
                    port map (
                     Q  => fifoArray(Depth-1)(j),
                     C => clk, 
                        CE => en(Depth-1),
                        CLR => rst,
                        D => DataIn(j)
                        );   
    end generate;                       
  
  FifoArray_ins:for  i in Depth -2  downto 0  generate
        WordGenerate:for j in DataWidth-1 downto 0 generate
            WordsFDCE : FDCE_1
                            port map (
                                 Q  => fifoArray(i)(j),
                             C => clk, 
                             CE => en(i),
                             CLR => rst,
                             D => fifoArray(i+1)(j)
                             );  
        end generate;                       
   end generate;    
  DataOut<=fifoArray(0);
 
  
end RTL;

---

Regards

 

[TrickyDicky]

This is a very small fifo. What happens if you need a depth of 2k, you would usually use a BRAM for storage. Hence the need for read and write pointers.

- - - Updated - - -

PS> As a code tip - please add some comments, and behavioural code (rather than directly instantiating each register) wouldnt go amiss.

 

[Miralipoor]

"This is a very small fifo. What happens if you need a depth of 2k, you would usually use a BRAM for storage. Hence the need for read and write pointers."
Size of Fifo strictly dependent of resource of your target FPGA.Your design have at least one adder and two comparator that decrease your clk frequency dramatically.If this fifo be used in a first block of high frequency device probably it will not work.
"PS> As a code tip - please add some comments, and behavioural code (rather than directly instantiating each register) wouldnt go amiss."
Behavioral code is good but when you write code in more abstract fashion, you lose something that is vital for high performance hardware(i.e. manual placement).As you can see in good practices most of them write code in RTL rather than behavioral or structural except for TestBench Writing and environment modeling. It must be mentioned that in general FPGAs are using in situation that CPU can not handle it.In Co-Design there is a fundamental principle and it is "Use software
as much as you can".

 

[std_match]

I like your high-speed shift-register FIFO, but I think it only can be motivated if the clock frequency is too high for a block RAM based FIFO or if the size is very small.
Each data bit in your FIFO will consume about the same amount of logic resources as each bit in the counters and comparators for the RAM+pointers+comparators solution.
The high-speed FIFO will consume more logic resources if it can store more than about 100 bits (assuming that there is a free block RAM available for the other solution).
A normal block RAM size is 36 Kbits. A shift-register based FIFO of that size will be huge.
There are a lot of block RAMS in current FPGA's. It is common that not all are needed.

I looked at the FDCE_1 flip-flop you are using. It is clocked on the negative edge. Is there a special reason for this?
On what device did you develop the shift register FIFO? Which families has the FDCE_1?