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.

Logic scope coding approach

Status
Not open for further replies.
Use verilog 2001 ports saves repeated typing.

Defines bad, parameters good. Dont use defines
 

Make sure you can have a ram connected entirely to a port and that the tools still infer the correct ram type. eg, BRAM or DMEM. I suspect the tools will not understand this and will create a large register array.

The coding style is not software-centric. Each part of the code strongly suggests how synthesis will work. eg, clearly defining a small circuit, a counter, or a comparator. The criticism of "software" style design is that the logic makes sense (excluding mis-use of non-blocking assigns) but it is a mystery how the synthesis tools will work. Often the tools will not find a good solution or even a solution at all.
 

mis-use of non-blocking assigns

1) Which verilog module are you referring to ?

2) Besides, I have modified the memory block to be a separate module as in https://github.com/promach/internal_logic_analyzer/blob/development/rtl/memory_block.v

3) Why is the data reg unused as shown below ?

Screenshot from 2017-09-18 10-32-33.png
 

1.) This is not an issue with your code that I saw. This is a common issue with software developers who first attempt to use VHDL/Verilog.
2.) The tools have always had some issues with inferring BRAM/DMEM based RAMs/ROMs. Even the smallest deviation from the example code in the synthesis manual is enough to make me suspect that RAM will not be inferred optimally.
3.) Not sure, I suspect that this output port is not used later in the design, or that there is a clock/reset issue that causes the value to never be used.
 

Should I just split https://github.com/promach/internal_logic_analyzer/blob/master/rtl/check_data.v#L15 or the combinatorial logic at line 15 below across several registers ?


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
`include "define.v"
 
module check_data (clk, data_out, data_in, test_failed);  // verifying the memory (circular buffer) read and write processes
 
input clk;
input [(`DATA_WIDTH-1) : 0] data_out;  // memory output
input [(`DATA_WIDTH-1) : 0] data_in;   // memory input
 
output reg test_failed = 0;
 
wire data_out_is_valid;
 
always @(posedge clk)
begin
    if(((data_out + `MEMORY_SIZE + `ALIGNMENT_DELAY) != (data_in + 1)) && data_out_is_valid)  // due to "delay.v"
    test_failed <= 1;
end
 
assign data_out_is_valid = (data_in >= `MEMORY_SIZE + `USER_HOLDOFF + `ALIGNMENT_DELAY) && 
               (data_in < `MEMORY_SIZE + `MEMORY_SIZE + `USER_HOLDOFF + `ALIGNMENT_DELAY);
 
endmodule

 

Should I just split https://github.com/promach/internal_logic_analyzer/blob/master/rtl/check_data.v#L15 or the combinatorial logic at line 15 below across several registers ?


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
`include "define.v"
 
module check_data (clk, data_out, data_in, test_failed);  // verifying the memory (circular buffer) read and write processes
 
input clk;
input [(`DATA_WIDTH-1) : 0] data_out;  // memory output
input [(`DATA_WIDTH-1) : 0] data_in;   // memory input
 
output reg test_failed = 0;
 
wire data_out_is_valid;
 
always @(posedge clk)
begin
    if(((data_out + `MEMORY_SIZE + `ALIGNMENT_DELAY) != (data_in + 1)) && data_out_is_valid)  // due to "delay.v"
    test_failed <= 1;
end
 
assign data_out_is_valid = (data_in >= `MEMORY_SIZE + `USER_HOLDOFF + `ALIGNMENT_DELAY) && 
               (data_in < `MEMORY_SIZE + `MEMORY_SIZE + `USER_HOLDOFF + `ALIGNMENT_DELAY);
 
endmodule


that looks like a super long path. whether it is appropriate for your design or it has to be pipelined, only you can tell.
 

Code:
if (data_out + `MEMORY_SIZE + `ALIGNMENT_DELAY) != (data_in + 1)) && data_out_is_valid)
This isn't a software if statement, this is a deep combinational circuit.

a `DATA_WIDTH add operation, with a `DATA_WIDTH compare operation with another `DATA_WIDTH add operation that has a `DATA_WIDTH bitwise AND operation.

Yeah that is a lot of logic and is unlikely to run at high clock rates in an FPGA.

Still can't condone the use of `defines, they can get you into trouble with things getting redefined by someone else's code, due to using defines with the same name. Parameters should be used to both make code both reusable and keep things localized to a module. Using parameters also allows you to use generates for multiple copies of a module with different parameters defined for each instance.

Old time Verilog users, who neglect to update their skills will use defines ad nauseum as all they know is Verilog 83 (a.k.a the original Gateway design's verilog). Most of the online Verilog tutorials seem to be written by Verilog coders still stuck in an 80's-90's timewarp.

- - - Updated - - -

Logically I'm not sure your comparison works the way you expect.

It looks to me like the != compare will be true every time the RHS bitwise AND has a 0 on data_out_is_valid, so unless the data_out_is_valid never goes low at some point the compare will be true and the fail flag will get set.
 

the != compare will be true every time the RHS bitwise AND has a 0 on data_out_is_valid

Could you tell us more on this above statement ?
 

Could you tell us more on this above statement ?
Uh, I was assuming you have some background in logic design...was I wrong?

say data_out with the additions is data_out_add and the RHS is called data_in_add...

Code:
data_out_add != data_in_add && data_out_valid
will be a true statement (i.e. NOT EQUAL) when data_out_valid is 0 in all conditions but one.
The statement will only be false if data_out_add is exactly 0 which can only occur if data_out + 8 + 3 = 0 or data_out is exactly 4294967285, which will result in a rollover of the 32-bit data_out_add to become 2**32 (a 1 followed by 32 zeros).

The whole approach of this if statement screams I write software and don't know what statements like this look like as hardware. It also screams I don't even know what the assembly language for this looks like after I compile my C code.

- - - Updated - - -

I really think what you wanted to do was only do a comparison when data_out_vaild is a 1. As written that is not what your logic does.
 

The statement will only be false if data_out_add is exactly 0

Sorry for the software-style if. My friend and I do not understand what you mean by your statement above.
 

I mostly disagree.

First, && is logical-and and binds weaker than !=. The expression is equivalent to "(data_out_add != data_in_add) && data_out_valid". The copy/pasted code is not parenthetically balanced though, not sure if there was a previous version or not. The code doesn't need parentheses either, but that annoys many people as sometimes you'll see someone mess up ternary operators.

Second, something else has to be wrong to get a fully intra-clock domain path to fail by 10ns. That means that two 32b adds in parallel followed* by xnor-reduce can't complete in 10ns, let alone the original constraint + 10ns. For most FPGAs, this would be a completely reasonable operation for 10ns. People write (x+1 == limit) and don't even care. That is roughly as complex as this logic.

My guesses would be:
1.) the ram doesn't get inferred and thus creates a big mux. This is the part of the design that is software-ish -- the modules kinda want to take a reference to a ram object and have access to the internal state.
2.) the tools gave up early and made no effort to improve timing.
3.) the contstraints are not correct and cause issues like #2.

@promach, can you paste the full worst path?



* this also assumes really bad routing between the additions and the comparison. The additional logical-and probably gets sucked into the comparison as well, unless the FPGA only has LUT4s.
 

Sorry for the confusion I created.

The setup timing violation is due to my mistake in my sdc constraint file. I have set my system clock to have a period of 1ns, or equivalently 1Ghz.

However, I am still facing bug in hardware testing and still debugging check_data.v module.
 

My advice remains the same:

1.) assign some reasonable clock constraint. since the worst case slack was 10ns and you set the constraint to 1ns, try 10ns. It is possible the tools notice the impossible constraint and fail-fast.
2.) use the circuit schematic tool and/or synthesis results to show that the ram is a ram and not an array of registers.

--edit: ok, it looks like the memory size is 8, as in 8 elements. This is probably fine without the inferred ram. However, you should go back and create a design that has a useful buffer size.
 
Last edited:

Last edited:

I suppose inferred RAM coding is different across FPGA vendors, right ?

Can we infer dual port memories as RAM the same way on both Xilinx and Altera vendors ?

Its pretty much the same for both. There are some subtleties for some features, but if you are going this far, you will probably need to use the vendor's own IP blocks.
 

This memory block is now inferred as RAM instead of register array.

However, some inevitable warnings are shown http://quartushelp.altera.com/14.1/mergedProjects/msgs/msgs/wtdb_analyze_comb_latches.htm


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
`include "define.v"
 
module memory_block (clk, write_enable, waddr, raddr, data_write, data_read);
 
input clk, write_enable;
input [(`ADDR_WIDTH-1) : 0] waddr;
input [(`ADDR_WIDTH-1) : 0] raddr;
input [(`DATA_WIDTH-1) : 0] data_write;  // data to be written into memory
output reg [(`DATA_WIDTH-1) : 0] data_read;  // data read out from memory
 
reg [(`DATA_WIDTH-1) : 0] memory [(`MEMORY_SIZE-1) : 0];
 
always @(*)
begin
    if (write_enable)   
        memory[waddr] = data_write;
end
 
always @(*)
begin
    if (!write_enable)  
        data_read = memory[raddr];
end
 
endmodule

 
Last edited:

First, && is logical-and and binds weaker than !=.
Boy did I ever misread the code promach posted, I kept reading it as a bitwise AND instead of the logical AND operation.

promach sorry for the confusing posts. :???:
 

you are getting latches because you are never using clk. Change your always statements to always@(posedge clk) and the blocking (=) to non-blocking (<=).

Also take the write_enable out of the read logic. You should read something on every clock.

John Eaton
 

Status
Not open for further replies.

Similar threads

Part and Inventory Search

Welcome to EDABoard.com

Sponsor

Back
Top