Issue with Logic in Verilog

Question

I'm trying to write a multiplier based on a design. It consists of two 16-bit inputs and the a single adder is used to calculate the partial product. The LSB of one input is AND'ed with the 16 bits of the other input and the output of the AND gate is repetitively added to the previous output. The Verilog code for it is below, but I seem to be having trouble with getting the outputs to work.

module datapath(output reg [31:15]p_high,
                output reg [14:0]p_low,
                input [15:0]x, y,
                input clk); // reset, start, x_ce, y_ce, y_load_en, p_reset, 
               //output done);

reg [15:0]q0;
reg [15:0]q1;
reg [15:0]and_output;
reg [16:0]sum, prev_sum; 
reg d_in;
reg [3:0] count_er;

initial
 begin
      count_er <= 0;
      sum <= 17'b0;
      prev_sum <= 17'b0;
 end

always@(posedge clk)
begin
        q0 <= y;
        q1 <= x;
        and_output <= q0[count_er] & q1;
        sum <= and_output + prev_sum;    
        prev_sum <= sum;
        p_high <= sum;
        d_in <= p_high[15];
        p_low[14] <= d_in;
        p_low <= p_low >> 1;
        count_er <= count_er + 1;
 end
 endmodule

I created a test bench to test the circuit and the first problem I see is that, the AND operation doesn't work as I want it to. The 16-bits of the x-operand are and'ed with the LSB of the y-operand. The y-operand is shifted by one bit after every clock cycle and the final product is calculated by successively adding the partial products.

However, I am having trouble starting from the sum and prev_sum lines and their outputs are being displayed as xxxxxxxxxxxx.

Answer 1

You don't seem to be properly resetting all the signals you need to, or you seem to be confusing the way that nonblocking assignments work.

After initial begin:

• sum is 0
• prev_sum is 0
• and_output is X

After first positive edge:

• sum is X, because and_output is X, and X+0 returns X. At this point sum stays X forever, because X + something is always X.

You're creating a register for almost every signal in your design, which means that none of your signals update immediately. You need to make a distinction between the signals that you want to register, and the signals that are just combinational terms. Let the registers update with nonblocking statements on the posedge clock, and let the combinational terms update immediately by placing them in an always @* block.

I don't know the algorithm that you're trying to use, so I can't say which lines should be which, but I really doubt that you intend for it to take one clock cycle for x/y to propagate to q0/q1, another cycle for q to propagate to and_output, and yet another clock cycle to propogate from and_output to sum.

---

Comments on updated code:

• Combinational blocks should use blocking assignments, not nonblocking assignments. Use = instead of <= inside the always @* block.
• sum <= and_output + sum; looks wrong, It should be sum = and_output + p_high[31:16] according to your picture.

• You're assigning p_low[14] twice here. Make the second statement explicitly set bits [13:0] only:

  p_low[14]   <= d_in;
  p_low[13:0] <= p_low >> 1;
  

Answer 2

You are mixing blocking and nonblocking assignments in the same sequential always block, which can cause unexpected results:

d_in <= p_high[15];
p_low[14] = d_in;