multiple drive issue when receiving inputs from external source in verilog synthesis

Question

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

Answer 1

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:

• Formating your code correctly is very important, make sure your code is always clean as it helps spot bugs. I reformatted it and there seems to be a missing begin..end in the last block of the IDLE state
• Always begin..end your blocks, it will avoid so many bugs
• Watch out for the size of things, I see you declared your variables to be 32 bits reg [31:0] but are only assigning them using 31'd which is 31 bits. Use '0 syntax to zero fill for any size.
• Setting registers to '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.