The synthesis I'm trying to do is on a GCD algorithm Finite State Machine that works using the "subtract if bigger" method. I will attach the code and try and formulate a decent question.
module GCD_R (A,B,out,nrst,act,clk,fla_g);
input [31:0] A, B;
input clk, act, nrst;
output reg fla_g;
output reg [31:0] out;
reg [3:0] state, next_state;
reg [31:0] A_reg, B_reg, Aint_reg, Bint_reg, out_reg;//2 registers will keep intermediate numbers+out, and next numbers
parameter IDLE = 3'b001;
parameter ABIG = 3'b010;
parameter BBIG = 3'b100;
reg next_flag;
always @(*)
case (state)
IDLE: begin
$display("start");
if(act == 0) begin
A_reg = A; //A,B are wires that contain numbers from an external source
B_reg = B; //first assign to A_reg and B_reg
out_reg = 31'bx;
next_flag = 1'b0;
next_state = IDLE;
end
if(act == 1)
if(A_reg==0) begin
out_reg = B_reg;
next_flag = 1'b1; //testbench will know when we stopped
Aint_reg = A_reg; //taking care not to infer latches
Bint_reg = B_reg;
next_state = IDLE;
end
else if (B_reg==0) begin
out_reg = A_reg;
next_flag = 1'b1;
Aint_reg = A_reg; //taking care not to infer latches
Bint_reg = B_reg;
next_state = IDLE;
end
else if (A_reg >= B_reg) begin
out_reg = 31'bx;
next_flag = 1'b0;
Aint_reg = A_reg;
Bint_reg = B_reg;
next_state = ABIG;
end
else begin
out_reg = 4'bxxx;
next_flag = 1'b0;
Aint_reg = A_reg;
Bint_reg = B_reg;
next_state = BBIG;
end
else
begin
Aint_reg = A_reg;
Bint_reg = B_reg;
out_reg = 4'bxxx;
next_flag = 1'b0;
next_state = 4'bx;
end
end
ABIG: begin
if (A_reg==0 | B_reg==0) begin
out_reg = 31'bx;
next_flag = 1'b0;
next_state = IDLE;
Aint_reg = A_reg;
Bint_reg = B_reg;
end
else if (B_reg > A_reg) begin
out_reg = 31'bx;
next_flag = 1'b0;
next_state = BBIG;
Aint_reg = A_reg;
Bint_reg = B_reg;
end
else begin
out_reg = 31'bx;
next_flag = 1'b0;
next_state = ABIG;
Aint_reg = A_reg - B_reg;
Bint_reg = B_reg;
end
end
BBIG: begin
if (A_reg==0 | B_reg==0) begin
out_reg = 31'bx;
next_flag = 1'b0;
next_state = IDLE;
Aint_reg = A_reg;
Bint_reg = B_reg;
end
else if (A_reg > B_reg) begin
out_reg = 31'bx;
next_flag = 1'b0;
next_state = ABIG;
Aint_reg = A_reg;
Bint_reg = B_reg;
end
else begin
out_reg = 31'bx;
next_flag = 1'b0;
next_state = BBIG;
Aint_reg = A_reg;
Bint_reg = B_reg - A_reg;
end
end
default: begin
out_reg = 31'bx;
next_flag = 1'b0;
next_state = 4'bx;
Aint_reg = A_reg;
Bint_reg = B_reg;
$display("%t: State machine not initialized/n",$time);
end
endcase
always @(posedge clk or negedge nrst)
if (~nrst) begin
state <= IDLE;//we get the new values by resetting first
out <= 4'bx;//we don't want anything there at the reset
fla_g <= 1'b0;
end
else begin
state <= next_state;//otherwise, get the next state and the next registers to the intermediate ones
A_reg <= Aint_reg;// 2nd assign to A_reg and B_reg- that's the problem
B_reg <= Bint_reg;
out <= out_reg;
fla_g <= next_flag;
end
endmodule
first, A and B are wires that will receive a list of numbers to compare via external source(some textfile in a testbench).
A_reg and B_reg are the intermediate registers that keep the numbers that we are checking the "if" statements on,
Aint_reg and Bint_reg are registers that keep the intermediate values of registers after the operations only to send them back to A_reg and B_reg when the clock is rising,
act decides whether the machine is on "on" mode and can perform the algorithm
nrst is the negative reset toggle
how was the problem formed:
you can see that at the beginning, there is an if(act == 0) condition. It was put there to ensure that the values at the A,B wires(received externally) will enter the registers there, instead of entering them in the sequential block while we were on the if(~nrst) condition since it made no sense entering a dynamic value while in reset.
This takes us to the current problem- I know that assigning a value to A and B both in the sequential and combinatoric blocks is what creates the problem, but I just can't find an alternative.
p.s. the fact that I'm using A_reg = A creates latches since I don't assign to A_reg anywhere else, and that's another problem since writing
Aint_reg = A_reg;
A_reg = Aint_reg;
to satisfy latches doesn't sit right with me.
p.s.2. I tried looking into the similar questions on the site, but owing to my lack of enough knowledge on the subject I couldn't relate my problem to the ones out there
I'll be glad to receive any help, thanks
EDIT: I deleted the nonblocking assignments in the if(~nrst) sequential block so as to not to multiple assign A_reg <= 0 there and A_reg = A in the combinatoric block, but the multiple assignment problem still bugs it somehow
EDIT2: seems that I forgot a fundamental thing- don't assign to the same variable in two different "always" block, but I just can't think of a good enough solution to assign the wires A,B to a register and not assign to it again in the sequential block
You are correct in identifying the major problem in your code is that you are not handling registers and combinational logic correctly. Whenever you have a register (register, not reg type, they are not the same and it is confusing for those new to Verilog), you need to define it in a particular way so synthesis and simulation tools can handle it. The safest practice is to create a clean separation between combinational logic and sequential storage (ie, actual registers). You started to do this with Aint_reg, Bint_reg, out_reg, etc; but you need to do it for all registered values whose values come from conbinational blocks. So, all these ideas together yields a struct for code like this (not completely your code but something similar):
input [31:0] A, B;
output reg [31:0] out;
reg [31:0] regA, regB;
reg [3:0] state, nextState;
always @(posedge clk or negedge rst) begin
if (~rst) begin
regA <= '0;
regB <= '0;
state <= IDLE;
end
else begin
regA <= nextA;
regB <= nextB;
state <= nextState;
end
end
always @(*) begin
// Default; I always do this here to ensure there are no cases in which I dont assign a combinational value for this block
// it always helps me keep track of which variables are assigned in which block
nextA = regA;
nextB = regB;
nextState = state;
out = '0;
case (state)
// In here is your case body, where you can reassign nextA, nextB, nextState and out
// where the assignments are only dependent on A, B, regA, regB and state (ie, the values NOT assigned in this block)
endcase
end
With this in mind, you need to have only the Aint_reg, Bint_reg, etc in the combinational block; so you dont need to assign A_reg, Breg, etc in the combinational block. Note also that if this will mean the timing of out will be delayed, so you can always bypass the register if you need to push a value out from a combination block immediately. From what I understand, you might be having issues with loading during a reset, which is understandable as so long as reset (nrst) is asserted (ie, 0), nothing will load. That is the entire point of a reset, to hold the system in a known state until it is lifted. So until nrst is deasserted, your module shouldnt do anything. A few other points:
begin..end in the last block of the IDLE statebegin..end your blocks, it will avoid so many bugsreg [31:0] but are only assigning them using 31'd which is 31 bits. Use '0 syntax to zero fill for any size.'x is not ideal, you should have registers just retain their state (as you do thing for register values and next register values).Hope this clarifies things for you.