Verilog Synthesis fails on if statement containing two variables

Question

I encountered a problem with synthesis where if I had two variables in an if statement, Synthesis will fail (with a very misleading and unhelpful error message).

Given the code snippet below

case(state)
//other states here
GET_PAYLOAD_DATA:
        begin
                if (packet_size < payload_length) begin
                    packet_size <= packet_size + 1;
                    //Code to place byte into ram that only triggers with a toggle flag
                    next_state = GET_PAYLOAD_DATA;
                end else begin
                    next_state = GET_CHKSUM2;
                end
end

I get an error in Xilinx ISE during synthesis:

 ERROR:Xst:2001 - Width mismatch detected on comparator next_state_cmp_lt0000/ALB. Operand     A and B do not have the same size.

The error claims that next_state isn't correct, but if I take out payload_length and assign a static value to it, it works perfectly fine. As both packet_size and payload_length are of type integer, they are the same size and that is not the problem. Therefore I assume its a similar problem to for loops not being implementable in hardware unless it is a static loop with a defined end. But If statements should work as it is just a comparator between 2 binary values.

What I was trying to do here is that when a byte is received by my module, it will be added into RAM until the the size of the entire payload (which I get from earlier packet data) is reached, then change to a different state to handle the checksum. As the data only comes in 1 byte at a time, I recall this state multiple times until the counter reaches the limit, then I set the next state to something else.

My question is then, how do I achieve the same results of calling my state and repeat until the counter has reached the length of the payload without the error showing up?

EDIT: Snippets of how packet_size and payload_length are declared, as requested in comments

integer payload_length, packet_size;

initial begin
    //other stuff
    packet_size <= 0;
end

always @ (posedge clk) begin
    //case statements with various states
    GET_PAYLOAD_LEN:
        begin
            if (rx_toggle == 1) begin
                packet_size <= packet_size + 1;
                addr <= 3;
                din <= rx_byte_buffer;
                payload_length <= rx_byte_buffer;
                next_state = GET_PAYLOAD_DATA;
            end else begin
                next_state = GET_PAYLOAD_LEN;
            end
        end

rx_byte_buffer is a register of the input data my module receives as 8 bits wide, while packet_size increments in various other states of the machine prior to the one you see above.

I have gotten around the error by switching the if statement conditionals around, but still want to understand why that would change anything.

Answer 1

There are some errors that stick out right away about the code, while they may not fix this problem, they will need to be corrected because it will cause a difference in simulation and hardware tests.

The nextstate logic needs to be in a different always block that does not change based on the posedge of clock. The sensitivity list needs to include things like "state" and/or "*". And if you wanted the nextstate logic to be registered like it is now (which you don't) you should use a nonblocking assignment, this is described in great deal in the cummings paper, provided below.

http://www.sunburst-design.com/papers/CummingsSNUG2000SJ_NBA_rev1_2.pdf

the code should look something like this:

always @ (*) begin
    //case statements with various states
    GET_PAYLOAD_LEN:
        begin
            if (rx_toggle == 1) begin
                packet_size_en = 1'b1;
    //these will need to be changed in a similar manner 
                addr <= 3;
                din <= rx_byte_buffer;
                payload_length <= rx_byte_buffer;
    /////////////////////////////////////////////////////
                next_state = GET_PAYLOAD_DATA;
            end else begin
                next_state = GET_PAYLOAD_LEN;
            end
        end

always@(posedge clk) begin 
   if(pcket_size_en)
       packet_size <= packet_size +1 ;
end 

Also, the first thing I would try is to make these a defined length, by making them of type reg (I assume that you wont be needing a signed number so it should have no difference on simulation), outside of generate blocks, you should try to not let synthesis play around with integers.