Continue to Site

Welcome to EDAboard.com

Welcome to our site! EDAboard.com is an international Electronics Discussion Forum focused on EDA software, circuits, schematics, books, theory, papers, asic, pld, 8051, DSP, Network, RF, Analog Design, PCB, Service Manuals... and a whole lot more! To participate you need to register. Registration is free. Click here to register now.

Register Log in

Help me in understanding Verilog constructs

Status
Not open for further replies.
G

Ganesan_R

Junior Member level 1
Joined
Sep 1, 2016
Messages
16
Helped
0
Reputation
0
Reaction score
0
Trophy points
1
Activity points
237
Dear Sir,

I am new to understanding Verilog as this language requires thinking in terms of synthesis.

While doing some program I found that:
begin
buf_inm[row][col] =temp_data;
#1 mux_data=buf_inm[row][col];
end


gave correct results than

begin
buf_inm[row][col] =temp_data;
mux_data=buf_inm[row][col];
end


in terms of assignments of variables.

Can anybody explain what is the difference between these two?

In any other higher level languages construct 2 (without delay) would have given correct assignments.

Thanking you,

Yours sincerely,
R. Ganesan.
 
Last edited by a moderator:

Ganesan_R said:
Can anybody explain what is the difference between these two?

In any other higher level languages construct 2 (without delay) would have given correct assignments.
Click to expand...

Because Verilog is not a high level Software Language, it's a Hardware Description Language.

Having to use # delays suggests you aren't coding hardware but are expecting the results of the statement to behave like the typical software language (i.e. statements executed in sequence unless a branching statement is executed), so you are trying to force the "execution order" of the statements.

# delays are never required if you are coding for synthesis and they are ignored by the synthesis tool. If you think it is working with the # delay then your interpretation of the simulation is probably wrong. Given you are using blocking statements I doubt this will function properly as you are trying to create an asynchronous memory, which doesn't exist in an FPGA. If you have this in a edge triggered always block and are using blocking assignments (instead of non-blocking assignments) then that could also explain the simulation showing the wrong thing.

You really should post all the code so we can determine in what context you've done the assignments.
 

give us an example of simulation for the 2 cases please.
I can not catch what does this saying mean
"gave correct results than"
 

Dear Sir,

I thank you for your reply.

I am attaching Verilog codes both source and test bench as .txt files.

I am simulating a fifo_counter and drawing its push and pop values as mux data.

mux_data is correctly being pushed to push and pop values if delay is included.

push and pop tasks are part of initial block. Blocking assignment statements <= do not work in this case.

Pl. advise.

Yours sincerely,
R. GANESAN

Code Verilog - [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

`timescale 1ns / 1ps
`include "C:/Users/GANESAN/NoC_RG_3_copy/NoC_RG_3_copy.srcs/sources_1/new/fifo_buf.v"
`include "C:/Users/GANESAN/NoC_RG_include/NoC_RG_include.srcs/sources_1/new/NoC_RG_include.vh"
//`include "C:/Users/GANESAN/NoC_RG_mux_2_copy/NoC_RG_mux_2_copy.srcs/sources_1/new/NoC_RG_mux_2.sv"
module NoC_RG_mux_2(clk,mux_in_data,mux_sel,mux_out_data,rst,rd_enm,wr_enm,buf_fullm,buf_emptym,buf_inm,buf_outm,fifo_counterm,mux_data);
input clk;
input wire [`DATA_SIZE-1:0] mux_in_data [5];
input wire  [2:0] mux_sel ;
output reg [`DATA_SIZE-1:0] mux_out_data;
reg [`DATA_SIZE-1:0] muxdata[5];
input wire rst;
input wire [7:0] buf_inm [0:3][0:3];
input wire  rd_enm [0:3][0:3], wr_enm[0:3][0:3];
output reg  buf_fullm [0:3] [0:3];
output reg  buf_emptym [0:3] [0:3];
output reg  [7:0] buf_outm [0:3][0:3];
output reg  [3:0] fifo_counterm [0:3][0:3] ;
input wire [7:0]mux_data;
genvar i,j,k;
    generate
         for (i=0;i<=`rowsize-1;i=i+1)
         begin
             for (j=0;j<=`colsize-1;j=j+1)
             begin
                fifo_buf f1(.clk(clk),.rst(rst),.rd_en(rd_enm[i][j]),.wr_en(wr_enm[i][j]),.buf_full(buf_fullm[i][j]),.buf_empty(buf_emptym[i][j]),.buf_in(buf_inm[i][j]),.buf_out(buf_outm[i][j]),.fifo_counter(fifo_counterm[i][j])); 
              end
         end   
     endgenerate
  //mux coding starts  
   always @(mux_sel)
    mux_out_data=mux_in_data[mux_sel];
//mux coding ends 
endmodule


Code Verilog - [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
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127

`timescale 1ns / 1ps
//////////////////////////////////////////////////////////////////////////////////
`include "C:/Users/GANESAN/NoC_RG_include/NoC_RG_include.srcs/sources_1/new/NoC_RG_include.vh"
`include "C:/Users/GANESAN/NoC_RG_3_copy/NoC_RG_3_copy.srcs/sources_1/new/fifo_buf.v"
module tb_NoC_RG_mux_2();
reg [`DATA_SIZE-1:0] mux_in_data [5];
reg  [2:0] mux_sel;
wire [`DATA_SIZE-1:0] mux_out_data;
 //parameter rowsize=4;
 //parameter colsize=4;
 reg  clk,rst;
 reg [7:0] buf_inm [0:3][0:3];
 reg  rd_enm [0:3][0:3], wr_enm[0:3][0:3];
 wire  buf_fullm [0:3] [0:3];
 wire buf_emptym [0:3] [0:3];
 wire  [7:0] buf_outm [0:3][0:3];
 wire  [3:0] fifo_counterm [0:3][0:3];
 reg [3:0] i,j;
 reg [7:0] temp_data;
 reg [7:0] mux_data;
  //Mux code ends here
NoC_RG_mux_2 f2 (.clk(clk),.mux_in_data(mux_in_data),.mux_sel(mux_sel),.mux_out_data(mux_out_data),.rst(rst),.rd_enm(rd_enm),.wr_enm(wr_enm),.buf_fullm(buf_fullm),.buf_emptym(buf_emptym),.buf_inm(buf_inm),.buf_outm(buf_outm),.fifo_counterm(fifo_counterm),.mux_data(mux_data));
initial
begin
$display("%m");
rst=1'b1;
clk=1'b0;
#4 rst =1'b0;    
temp_data<=0;
mux_data<=0;
//mux_out_data<=0;
for (i=0;i<=3;i=i+1)
begin
           for (j=0;j<=3;j=j+1)
              begin           
                rd_enm[i][j]=0;
                wr_enm[i][j]=0;
                end
end 
//buf_out_mux=0;
//$display("fifo_buf_mux f2 rd_enm %d %d=%d",i,j,f2.rd_enm[2][1]);
//$display("My instance name is: %m");
push(2,1,120);
push(3,2,90);
pop(2,1,temp_data);
push(0,2,63);
/*pop(2,1,temp_data);
pop(3,2,temp_data);
pop(3,2,temp_data);
pop(3,2,temp_data);
push(3,2,61);
push(3,2,75);
push(3,2,80);
push(3,2,90);
push(3,2,100);
pop(3,2,temp_data);
pop(3,2,temp_data);
pop(3,2,temp_data);
pop(3,2,temp_data);
pop(3,2,temp_data);*/
end
always
#5 clk <= ~clk;
task pop;
input [3:0] row,col;
output [7:0] temp_data;
//output [7:0] mux_data;
begin
     if (!buf_emptym[row][col])
     begin
     rd_enm[row][col]=1'b1;
      @(posedge clk) #2  rd_enm[row][col]=1'b0;
     temp_data=buf_outm[row][col];
     #1 mux_data=temp_data;
     $monitor("Buf_emptym=  %d",buf_emptym[row][col]);
      $monitor ("Popped location [row] [col] %d %d value=  %d",row,col,temp_data);
      $monitor ("mux_data= %d",mux_data);
      end
      else
      $monitor("buffer %d %d is empty ",row,col); 
      end        
endtask
 
task push;
input [3:0] row,col;
input [7:0] temp_data;
begin
       if (!buf_fullm[row][col])
       begin
      wr_enm[row][col]=1;
      buf_inm[row][col] =temp_data;
      #1 mux_data=buf_inm[row][col];
       $monitor ("Pushed location [row] [col] %d %d value=  %d",row,col,buf_inm[row][col]);
       $monitor("mux_data= %d",mux_data);
       @(posedge clk)#2  wr_enm[row][col]= 0;
         end
       else
      $monitor("buffer %d %d is full",row,col);
          
  end 
 endtask 
 
 
 //Mux code starts here
 always @ (mux_data)
  begin
   $monitor ("mux_data= %d",mux_data);
   end
   
   genvar a;
   generate
   for ( a=0; a<5; a++)
       begin
        assign mux_in_data[a]=mux_data;
       end 
   endgenerate 
      initial
     begin
     clk=0;
     mux_sel=1;
     #5 mux_sel=4;
     #10 mux_sel=1;
       end
 always @ (mux_sel)
         $monitor ("mux_sel= %d mux_out_data= %d",mux_sel,mux_out_data);
   
endmodule
 

Attachments

  • NoC_RG_mux.txt
    1.4 KB · Views: 52
  • tb_NoC_RG_mux.txt
    3.3 KB · Views: 60
Last edited by a moderator:

pop task:
------
rd_enm[row][col]=1'b1;
@(posedge clk) #2 rd_enm[row][col]=1'b0;
temp_data=buf_outm[row][col];
#1 mux_data=temp_data;
-------

Two racing cases can happen in your task:

1/ if you remove "#2", temp_data could get the old value of buf_outm[row][col].
2/ if you remove and #1, mux_data could get the old value of temp_data.

For simulation, you can add the delay to eleminate the racing condition.
We are not sure which event happen first in computer machine if there is many things ( blocking assignments = ) were required at the same time.

What happen if you remove those delays case by case ?

A waveform is better to understand how your problem was.
 

slutarius said:
pop task:
------
rd_enm[row][col]=1'b1;
@(posedge clk) #2 rd_enm[row][col]=1'b0;
temp_data=buf_outm[row][col];
#1 mux_data=temp_data;
-------

Two racing cases can happen in your task:

1/ if you remove "#2", temp_data could get the old value of buf_outm[row][col].
2/ if you remove and #1, mux_data could get the old value of temp_data.

For simulation, you can add the delay to eleminate the racing condition.
We are not sure which event happen first in computer machine if there is many things ( blocking assignments = ) were required at the same time.

What happen if you remove those delays case by case ?

A waveform is better to understand how your problem was.
Click to expand...
Your point 1 & 2 don't make any sense. They are both blocking assignments.

Blocking assignment block all further assignments until the current assignment is complete. They therefore are assigned in the order written from top to bottom. So the red highlighted text makes no sense (unless you meant "<=' non-blocking assignments).

I think it is a case of the assignment being seen by the clock inside the FIFO code at the time the signal(s) transition in the the testbench. Basically it is a breakdown in the simulator's scheduling due to the way the testbench is coded. But as you didn't supply all the code I can't verify your current testbench in a simulator to see if this is the case.
 

Dear Sir,

I thank slutarius for his kind insight.

I have verified simulation after removing the delays as noted. NoC_RG_mux_delay pertains to Tcl console after removing delay #1. NoC_RG_mux_2 pertains to Tcl console after removing #2.

Sometimes mux_data gets prior value some times value ahead as can be noted from the screen shot.

I have also enclosed the complete code as required by ads-ee. Pl. go thro' and advise.



Ads-ee may pl. advise how I could have better coded test bench. The delay in fifo_counter source code (not test bench) may pl. be ignored as they do not affect simulation results. (I have added it for my later extensions as landmarks)

Source code:

Code Verilog - [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

//`timescale 1ns / 1ps
module fifo_buf #(parameter buf_size=4,data_size=8,ptr_size=2) (clk,rst,rd_en,wr_en,buf_full,buf_empty,buf_in,buf_out,fifo_counter);
input  rd_en,wr_en;
input rst,clk;
input  [data_size-1:0] buf_in;
output reg [data_size-1:0] buf_out;
output reg buf_full,buf_empty;
reg [data_size-1:0] buf_mem[buf_size-1:0];
reg[ptr_size-1:0] rd_ptr,wr_ptr;
output reg [buf_size:0] fifo_counter;
 
always @ (fifo_counter)
begin
if (fifo_counter==buf_size)
  buf_full <= 1;
  else 
  buf_full<=0;
if (fifo_counter==0)
  buf_empty <=1;  
  else
  buf_empty<=0;
end  
 
always @(posedge clk or posedge rst)
    begin
         if (rst)
            fifo_counter <= 0;
         else 
         begin
         if (!buf_full&& wr_en && !buf_empty && rd_en)
             fifo_counter <= fifo_counter;
         else if (!buf_full&& wr_en)
             fifo_counter <= fifo_counter+1;
         else if (!buf_empty && rd_en) 
            fifo_counter <= fifo_counter-1; 
         else
            fifo_counter <= fifo_counter;
         end    
    end   
 
 always @(posedge clk or posedge rst)
    begin
        if (rst)
            begin
            wr_ptr <= 0;
            rd_ptr<=0;
            end
        else 
        begin
        if (!buf_full&& wr_en)
              #1 wr_ptr <= wr_ptr+1;
        else
            wr_ptr <= wr_ptr; 
         if (!buf_empty && rd_en) 
              #1 rd_ptr <= rd_ptr+1;
          else
             rd_ptr <= rd_ptr;    
          end      
    end
 
 always @(posedge clk)
    begin
        if (!buf_full&& wr_en)
            buf_mem[wr_ptr] <= buf_in;
        else
             buf_mem[wr_ptr] <=  buf_mem[wr_ptr];  
    end             
         
always @(posedge clk or posedge rst)
        begin
            if (rst)
                buf_out <= 0;
            else 
            begin
            if (!buf_empty && rd_en) 
                buf_out <= buf_mem[rd_ptr];
                 else
                 buf_out <= buf_out; 
            end     
        end                        
endmodule

Your insights are quite helpful and I am eagerly awaiting your replies.

I have one more issue: The code overshoots the device IO pin utilisation. What modifications I can do in the source codes not to exceed device IO utilisation.

Thanking you,

Yours sincerely,
R. Ganesan.

- - - Updated - - -

Dear Sir,

I have one more point.

You have mentioned for test bench you can add delay.

How test benches are realized (synthesized) in real world? Is it only software testing of hardware fpga board?

Thanking you,

Yours sincerely,
R. Ganesan
 
Last edited by a moderator:

Ganesan_R said:
I have verified simulation after removing the delays as noted. NoC_RG_mux_delay pertains to Tcl console after removing delay #1. NoC_RG_mux_2 pertains to Tcl console after removing #2.

Sometimes mux_data gets prior value some times value ahead as can be noted from the screen shot.
Click to expand...
What screen shots?

Use the Insert Image icon in the Quick Reply pane to easily add a picture uploaded from your computer.

Ganesan_R said:
I have also enclosed the complete code as required by ads-ee. Pl. go thro' and advise.
Click to expand...
you didn't supply the .vh file I'll just guess the values for the `defines (BTW this is really a bad practice to rely on `defines)

Ganesan_R said:
Ads-ee may pl. advise how I could have better coded test bench. The delay in fifo_counter source code (not test bench) may pl. be ignored as they do not affect simulation results. (I have added it for my later extensions as landmarks)
Click to expand...
Type out all words, I really hate the abbreviations used by non-english speakers it makes a post difficult to read (pl. I guess stands for please :-x)
  • Well for starters you could format better, I have to reformat you code just to read it (haven't you ever heard of whitespace, it comes in handy to make things easily readable).
  • Not part of you testbench, but you could use Verilog 2001 module port definitions instead of antiquated (prehistoric) Verilog 84-95 ones.
  • Your task could be entered potentially at the same time as the clock edge transition, which means you might just end up with a glitch on the rd_enm signal instead of a single clock cycle pulse. I would probably synchronize the task to the clock edge on entrance to the task then set the rd_enm high and remove it on the next clock edge, to ensure you get a full clock cycle pulse. If you want to be able to send a pop/push on every clock cycle then you should synchronize to the clock before the first push (then you can leave the tasks alone).

Ganesan_R said:
I have one more issue: The code overshoots the device IO pin utilisation. What modifications I can do in the source codes not to exceed device IO utilisation.
Click to expand...
Use less pins. If this is a module that is supposed to be used in a bigger design, then don't allow the tools to insert I/O when running it through synthesis/implementation.

Ganesan_R said:
You have mentioned for test bench you can add delay.

How test benches are realized (synthesized) in real world? Is it only software testing of hardware fpga board?
Click to expand...
A simulation testbench can include both non-synthesizble and synthesizable code. You don't synthesized testbenches you simulate with testbenches. You can write a synthesizable testbench (e.g. Xilinx MIG can produce a synthesizable testbench to allow you to run both a simulation and build the IP into a device to run a memory test).
 

Yes @ads-see, I was wrong saying it as blocking assignment. Thank you !

To the OP, it is better to provide the waveform of your simulation.
But surely, the coding style is not good and need to be improved. I belive there are ton of verilog guidelines for you to follow.
 

Dear Sir,

I thank both of them for reply to my posts.

I am herewith adding the screen shot jpeg file.

I am also pasting verilog header file:


Code: 
`timescale 1ns / 1ps
`ifndef _NoC_include_vh_
`define _NoC_include_vh_
`define rowsize 4
`define colsize 4
`define DATA_SIZE 8 //Put here required data size
`define BUF_SIZE 4
`define buf_size 4
//define log2 function here 
function integer log2; 
input integer value;
begin
 value=value-1;
 for (log2=0;value>0;log2=log2+1)
   value=value>>1;
 end
 endfunction

I tried following your advice in synchronizing with clock pulse but the coding with synchronization could not be completed by me as some signals being input are set in main code and some signals being output are set in the main code.

I am unable to carry intermediate signals from test bench to main program as intermediate signals are set in test benches for example rd_enm [j] in the test bench as I have to loop again in the main program even if I carry it.

So far the bonded IOB pin numbers are 584 as against 200 that is available. Yes the design is part of an entire NoC design. I do not know if IOB pin numbers exceed the Zynq demo board will I be able to practically burn and test the entire program?

On other points of advice I will try to improve.

Thanking you,

Yours sincerely,
R. Ganesan
 
Last edited by a moderator:

Ganesan_R said:
I tried following your advice in synchronizing with clock pulse but the coding with synchronization could not be completed by me as some signals being input are set in main code and some signals being output are set in the main code.
Click to expand...
What does this have to do with your testbench? Your testbench is supposed to emulate the top level code where your code is instantiated.

Seems to me this code is part of an NoC that wasn't designed up front and is some ad-hoc design by multiple people that didn't do any up front system design. If you don't have a good idea how you are supposed to receive or output data from your module (i.e. protocol, timing, etc) then you won't be able to write a testbench to test your design properly.

Ganesan_R said:
I am unable to carry intermediate signals from test bench to main program as intermediate signals are set in test benches for example rd_enm [j] in the test bench as I have to loop again in the main program even if I carry it.
Click to expand...

It's obvious you have a software mindset given the usage of main, loop, and program every time you discuss anything. Throw that mindset out the window, think of what you are doing as hooking up ICs on a board with wires.


Ganesan_R said:
So far the bonded IOB pin numbers are 584 as against 200 that is available. Yes the design is part of an entire NoC design. I do not know if IOB pin numbers exceed the Zynq demo board will I be able to practically burn and test the entire program?
Click to expand...
To test a design the steps are usually:

  • Run the code with a testbench in a simulator (i.e. Vivado simulator or Modelsim)
  • Implement the entire design and test with the top level module in the system OR build synthesizable BIT (i.e. Built in Test) code with an interface to perhaps some debug cores for test observation.
The BIT design will have very little of the pins of your design exposed to the outside world, mostly these will just be things like clocks and reset. The rest of the pins in your design would be connected to the BIT module.
 
Dear Sir,

As per your suggestion I completely reworked the entire code and test bench to synchronize with clock.

The relevant source code is as follows:

Code Verilog - [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

`timescale 1ns / 1ps
module fifo_buf #(parameter buf_size=4,data_size=8,ptr_size=2) (clk,rst,rd_en,wr_en,buf_full,buf_empty,buf_in,buf_out,fifo_counter);
input  rd_en,wr_en;
input rst,clk;
input  [data_size-1:0] buf_in;
output reg [data_size-1:0] buf_out;
output reg buf_full,buf_empty;
reg [data_size-1:0] buf_mem[buf_size-1:0];
reg[ptr_size-1:0] rd_ptr,wr_ptr;
output reg [buf_size-1:0] fifo_counter;
 
always @(fifo_counter)
begin
   if (fifo_counter == 0)
      buf_empty=1;
   else
      buf_empty=0;   
   if (fifo_counter == buf_size)
      buf_full=1;
   else
      buf_full=0;   
end
 
always @(posedge clk or posedge rst)
begin
if (rst)
  fifo_counter <= 0;
else if (!buf_full && wr_en)
  fifo_counter <= fifo_counter + 1;
else if (!buf_empty && rd_en)
  fifo_counter <= fifo_counter - 1;
else if ((!buf_empty && rd_en) &&  (!buf_full && wr_en))
  fifo_counter <= fifo_counter;
else
  fifo_counter<=fifo_counter;
end   
 
always @(posedge clk or posedge rst)
begin
   if (rst)
     buf_out <= 0;
   else
     begin
        if (!buf_empty && rd_en)
           buf_out<=buf_mem[rd_ptr];
        else 
           buf_out<=buf_out;
     end
end
                
always @(posedge clk)
begin
  if (!buf_full && wr_en)
     buf_mem[wr_ptr]<=buf_in;
  else
     buf_mem[wr_ptr]<=buf_mem[wr_ptr];  
end   
 
always @(posedge clk or posedge rst)
begin
  if (rst)
    begin
      wr_ptr<=0;
      rd_ptr<=0;
    end
    else
    begin   
      if (!buf_empty && rd_en) rd_ptr<=rd_ptr+1;
         else rd_ptr<=rd_ptr;
      if (!buf_full && wr_en) wr_ptr<=wr_ptr+1;
         else wr_ptr<=wr_ptr;
    end
end                    
endmodule


The relevant test code is reproduced below:

Code Verilog - [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
89
90
91

module tb_NoC_RG();
reg clk,rst,rd_en,wr_en;
reg [7:0] buf_in;
wire [7:0] buf_out;
wire [3:0] fifo_counter;
reg [7:0] temp_data,temp_data_out;
reg counter_full,counter_empty;
fifo_buf fb1 (.clk(clk),.rst(rst),.rd_en(rd_en),.wr_en(wr_en),.buf_full(buf_full),.buf_empty(buf_empty),.buf_in(buf_in),.buf_out(buf_out),.fifo_counter(fifo_counter));
initial
begin
clk=0;
rst=1;
rd_en=0;
wr_en=0;
//temp_data=0;
//buf_in=0;
#3 rst=0;
//buf_empty=0;
//buf_full=0;
@ (posedge clk) push(127);
@ (posedge clk) push(64);
@ (posedge clk) pop(temp_data);
@ (posedge clk) push(32);
@ (posedge clk) push(110);
@ (posedge clk) push(55);
@ (posedge clk) push(210);
@ (posedge clk) pop(temp_data);
@ (posedge clk) pop(temp_data);
@ (posedge clk) pop(temp_data);
@ (posedge clk) pop(temp_data);
@ (posedge clk) pop(temp_data);
@ (posedge clk) push(77);
@ (posedge clk) push(88);
@ (posedge clk) pop(temp_data); 
end
/*push(127);
push(64);
pop(temp_data);
push(32);
push(110);
push(55);
push(210);
pop(temp_data);
pop(temp_data);
pop(temp_data);
pop(temp_data);
pop(temp_data);
push(77);
push(88);
pop(temp_data);
end*/
 
always
#5 clk<=~clk;
 
task pop;
output [7:0] temp_data;
begin
if (!buf_empty)
      begin
      temp_data = buf_out;
      temp_data_out = temp_data;
      rd_en=1'b1;
      /*@(posedge clk)*/ #2 rd_en=0; 
      $display("fifo_counter= %d Popped data= %d",fifo_counter,buf_out);
      $display("temp_data_out= %d",temp_data_out);
       end
 else
     $display("buffer empty");
end     
endtask    
 
task push;
    input [7:0] temp_data;
begin 
      if (!buf_full)
      begin
                 
                buf_in = temp_data;
                temp_data_out = buf_in;
                $display("fifo_counter=%dPushed data= %d",fifo_counter,buf_in);
                $display("temp_data_out=%d",temp_data_out);
                 wr_en =1'b1;
                 /*@(posedge clk)*/#2 wr_en = 0;
      end
                
      else
                $display ("buffer full"); 
end     
 endtask   
endmodule


Now push temp_data_out is correctly is printing, but pop temp_data_out is missing temp_data by one clock cycle.

Screen print is listed below:

Code dot - [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

fifo_counter= 0Pushed data= 127
temp_data_out=127
fifo_counter= 1Pushed data=  64
temp_data_out= 64
fifo_counter=  1 Popped data= 127
temp_data_out=   0
fifo_counter= 1Pushed data=  32
temp_data_out= 32
fifo_counter= 2Pushed data= 110
temp_data_out=110
fifo_counter= 3Pushed data=  55
temp_data_out= 55
buffer full
fifo_counter=  3 Popped data=  64
temp_data_out= 127
fifo_counter=  2 Popped data=  32
temp_data_out=  64
fifo_counter=  1 Popped data= 110
temp_data_out=  32
fifo_counter=  0 Popped data=  55
temp_data_out= 110
buffer empty
fifo_counter= 0Pushed data=  77
temp_data_out= 77
fifo_counter= 1Pushed data=  88
temp_data_out= 88
fifo_counter=  1 Popped data=  77
temp_data_out=  55


Relevant screen shot is enclosed herewith.

Any insight on this is eagerly awaited.

Yours sincerely,
R. Ganesan.
 
Last edited by a moderator:

the fifo counter needs the read+write case as highest priority. As written, that case can't be reached because the write case is reached instead.
 

Dear Sir,

I am not getting your point.

The tasks are executed in the sequence they are presented.

Both push and pop outputs are OK.

Buf_out and Buf_in are reflected in temp_data values in both push and pop.

temp_data_out is equal to temp_data in push and lags behind one clock cycle to temp_data value in pop.

Use of non blocking assignments do not solve this issue.

Where I am wrong?

R. Ganesan
 

My comment could be unrelated to you problem. I wasn't sure what your issue was. The code, other than the logic issue, matches a fairly standard FIFO. That said, there are multiple types of FIFOs. Perhaps you were expecting a different output?

My point was for the basic logic -- you have "if A, else if B, else if A and B" as the structure. If A and B are true, then the structure does "if A". A is true, so there is no reason to check B.
 

Ganesan_R said:
Dear Sir,

As per your suggestion I completely reworked the entire code and test bench to synchronize with clock.

The relevant source code is as follows:
Click to expand...
Never said to synchronize every push pop only the first (reread post #8), as you added a @(posedge clk) on every push/pop you've just delayed everything by another clock cycle as there is already a @(posedge clk) inside the tasks. If you wanted to transfer on every clock cycle you needed to only have the single @(posedge clk) to synchronize the beginning first push/pop to the clock edge then all the reset of them would be synchronous to the clock edges by default due to the @(psoedge clk) inside the task.

You removed the @(posedge clk) from the tasks which makes your read_en and wr_en less than a clock cycle long. You are also getting a race condition between clk and the wr_en as the wr_en is transitioning at the clock rising edge and the write is occurring on that pulse leading edge. Take a look at the simulation captured in the figure below. The non full clock wide pulses (which won't emulate the actual hardware driving your module) are only 2 ns wide due to the #2 in the push pop tasks instead of the @(posedge clk) that should have been left there.

From what I can tell, you're still thing of this as software and may not have had enough digital design courses/training. FFs won't work reliably if you transition sub clock wide pulses right at the rising edge of the clock on the FF. You should pick up a good text on digital design and learn that before using Verilog.

BTW, you really need to use indentation in your code it's nearly unreadable and would be utterly unreadable if it was a 1000 line file.

Capture.PNG

- - - Updated - - -

If you change the code to correctly delay the testbench inputs to the FIFO UUT code then the simulation fails due to the handling of the full and empty flags. As they would actually be delayed in real hardware.

I suggest you read the following paper on designing a FIFO and understand why that design is coded that way.http://www.sunburst-design.com/papers/CummingsSNUG2002SJ_FIFO1.pdf
 

Dear Sir,

I did as you instructed.

But still in pop temp_data_out is not following temp_data.

I thank you for your lucid explanation of my coding style.

The test bench code is reproduced below:

Code: 
module tb_NoC_RG()
reg clk,rst,rd_en,wr_en;
reg [7:0] buf_in;
wire [7:0] buf_out;
wire [3:0] fifo_counter;
reg [7:0] temp_data,temp_data_out;
reg counter_full,counter_empty;
fifo_buf fb1 (.clk(clk),.rst(rst),.rd_en(rd_en),.wr_en(wr_en),.buf_full(buf_full),.buf_empty(buf_empty),.buf_in(buf_in),.buf_out(buf_out),.fifo_counter(fifo_counter));
initial
begin
clk=0;
rst=1;
rd_en=0;
wr_en=0;
//temp_data=0;
//buf_in=0;
#3 rst=0;
//buf_empty=0;
//buf_full=0;
@ (posedge clk) push(127);
/*@ (posedge clk)*/ push(64);
/*@ (posedge clk)*/  pop(temp_data);
/*@ (posedge clk)*/ push(32);
/*@ (posedge clk)*/  push(110);
/*@ (posedge clk)*/  push(55);
/*@ (posedge clk)*/  push(210);
/*@ (posedge clk)*/  pop(temp_data);
/*@ (posedge clk)*/  pop(temp_data);
/*@ (posedge clk)*/ pop(temp_data);
/*@ (posedge clk)*/ pop(temp_data);
/*@ (posedge clk)*/pop(temp_data);
/*@ (posedge clk)*/ push(77);
/*@ (posedge clk)*/push(88);
/*@ (posedge clk)*/ pop(temp_data); 
end
/*push(127);
push(64);
pop(temp_data);
push(32);
push(110);
push(55);
push(210);
pop(temp_data);
pop(temp_data);
pop(temp_data);
pop(temp_data);
pop(temp_data);
push(77);
push(88);
pop(temp_data);
end*/

always
#5 clk<=~clk;

task pop;
output [7:0] temp_data;
begin
@(posedge clk)
begin
if (!buf_empty)
      begin
      temp_data = buf_out;
      temp_data_out = temp_data;
      rd_en=1'b1;
      /*@(posedge clk)*/ #2 rd_en=0; 
      $display("fifo_counter= %d Popped data= %d",fifo_counter,buf_out);
      $display("temp_data_out= %d",temp_data_out);
       end
 else
     $display("buffer empty");
end
end 
    
endtask    

task push;
    input [7:0] temp_data;
begin 
@(posedge clk)
begin
      if (!buf_full)
      begin
                 
                buf_in = temp_data;
                temp_data_out = buf_in;
                $display("fifo_counter=%dPushed data= %d",fifo_counter,buf_in);
                $display("temp_data_out=%d",temp_data_out);
                 wr_en =1'b1;
                 /*@(posedge clk)*/#2 wr_en = 0;
      end
                
      else
                $display ("buffer full"); 
end 
end    
endtask   
endmodule

Pl. advise why still temp_data_out is not the same as temp_data only in task pop. (In push temp_data_out is the same as temp_data)

Regarding indenting I will go thro' how to do it in Vivado 2015.2 2 which I currently use.

Thanking you,

Yours sincerely,
R. Ganesan.
 

For practical purposes, I suggest replacing tabs with spaces. At least display tabs so it is clear what is/isn't a tab. The advantage here is that the code will look the same as long as a fixed width font is used. The topic of tabs/spaces and other editor annoyances draws strong opinions among developers. You'll be able to find extensive debates about the best formatting.


Code: 
      temp_data = buf_out;
      temp_data_out = temp_data;
      rd_en=1'b1;
using the blocking assignment, temp_data_out = temp_data = buf_data. Likewise, there may be issues with rd_en as a result of the blocking assignment. I'm not sure what you expect, nor why you are using a task. For a simple test, I suggest not using the task construct. Once you get that working you can re-add the task if you feel it is applicable.
 

Dear Sir,

Thank you for your reply.

After analysing the entire code once again the only conclusion I can draw is immediate assignment does not take into account register propogation delays.

While temp_data value is populated, it takes some time for it to get assigned (register delay) with new value. Immediate assignment of temp_data_out populates only old value of temp_data to temp_data_out. Adding some delay to temp_data_out gives correct results.

I want confirmation of my analysis from forum members on my understanding.

Thanks.

Yours sincerely,
R. Ganesan.
 

Ganesan_R said:
While temp_data value is populated, it takes some time for it to get assigned (register delay) with new value. Immediate assignment of temp_data_out populates only old value of temp_data to temp_data_out. Adding some delay to temp_data_out gives correct results.

I want confirmation of my analysis from forum members on my understanding.

Thanks.

Yours sincerely,
R. Ganesan.
Click to expand...

ads-ee said:
If you change the code to correctly delay the testbench inputs to the FIFO UUT code then the simulation fails due to the handling of the full and empty flags. As they would actually be delayed in real hardware.
Click to expand...
That is what I was alluding to in my previous post. I never apply data to a UUT using a task directly I always delay the outputs of the task by a #1 so they don't fall on the clock edge as what you are seeing could happen depending on the delta delays of your code. In fact I usually add in what I expect the external delays will actually be as a parameter constant. Though once you do this you will notice the same issue I saw with the delayed handling of the full and empty flags (you over/underflow the FIFO).

Due to Verilog's scheduling it might actually work without delays if you add a assignment of the task outputs to another combinational always block that just reassigns the task output to another signal.
e.g.
Code: 
task
...
task_output = something;
...
endtask

always @* uut_input = task_output;
If you try this report if it works.
 
Status
Not open for further replies.

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Back
Top