Add (and use) a reset signal.
Xilinx FPGAs have a global set/reset (GSR) signal that puts all registers in the their default state or as specified in the register declaration (this is documented in the XST User's Guide at the beginning of chapter 5). AFAIK, the @initial block is ignored.
However, things are chaotic when the FPGA starts up, because:
- The GSR is asynchronous.
- PLLs are not locked
- Not all PLLs lock at the same time
- There are race conditions everywhere
So the initial Flip-Flop values after the GSR are not enough.
Create a module that generates a reset signal for each clock domain. You can create it by by AND'ing relevant asynchronous reset signals, such as an external reset pin, PLL/DCM locked signals, and using it with a synchronizer, as follows:
(Source: How do I reset my FPGA?)
Rather than addressing the many problems in your source code, let me just show how I'd implement the module you describe.
First, I wouldn't use a sub-module to build the adder; synthesis tools are perfectly able to create adders from behavioral code. Secondly, an elaborate state machine isn't required; the module can simply produce a final result four clocks after each activation of the start
signal. I've added a done
signal to the module interface to make this explicit.
module seq_mult_4bit (
output [7:0] product,
output done,
input [3:0] a,
input [3:0] b,
input clock,
input start
);
reg [7:0] product;
reg [3:0] multiplicand;
reg [3:0] delay;
wire [4:0] sum = {1'b0, product[7:4]} + {1'b0, multiplicand};
assign done = delay[0];
always @(posedge clock) begin
if (start) begin
delay = 4'b1000;
multiplicand = a;
if (b[0]) begin
product <= {1'b0, a, b[3:1]};
end else begin
product <= {1'b0, 4'b0, b[3:1]};
end
end else begin
delay = {1'b0, delay[3:1]};
if (product[0]) begin
product <= {sum, product[3:1]};
end else begin
product <= {1'b0, product[7:1]};
end
end
end
endmodule
If you really want to use an external module for the adder (which is really the point of your question), simply substitute the wire declaration above with the following block of code:
wire [4:0] sum;
rca_4bit adder (
.sum (sum[3:0]),
.c_out (sum[4]),
.a (multiplicand),
.b (product[7:4]),
.c_in (0)
);
Let me know if you have any specific questions about how this implementation works.
Best Answer
This is a good example of mismatch in results between simulation and synthesis.
If you will synthesize this code and run a test on FPGA - it will work (at least you'll not see constant hash value). However, in simulation it behaves differently:
always @(*)
construct means "evaluate the following block of code any time any of the signals used on the right hand side of the assignment change". In your code these signals are:data_in
,poly
andhashValue
.Once again - synthesis tool will produce the correct logic for this code, therefore these kind of bugs are very dangerous.
There correct way to handle this is to define a clock signal and use a sequential
always @(posedge clk)
construct.Furthermore, it seems that you have at least two combinatorial loops in your code. Even if they are intended - this is very bad practice. You want to avoid using any synthesizable comb loop.
However, by inspection of your code, it seems that circular reference here is just for convenience - maybe it will not synthesize into comb loop. In this case you have two options:
function
construct.The first approach is the correct one - Verilog is not a programming language, and the statements you are using to describe logic must be maximally similar to the inferred logic. However, this approach may be tricky, since many algorithms are written in software forms. Therefore you might use the second approach, in which case the code will look like this (not tested):
Where
calcNewHashValue
is the function encapsulating thefor
loop from your code.Once again: if synthesis tool warns you about comb loops you mustn't use this design. In this case either think of other algorithmic implementation, or spread the calculation on several clock cycles.
You may also want to read this paper in order to get a deeper understanding of Verilog simulators behavior.
Except for that, your coding style is very bug-prone. As a guideline, I suggest you will define a separate
always
block for each signal. In other words - just one signal is assigned over inalways
block. Make exceptions only where you can't handle it other way, and think carefully before each such decision.