Mixing blocking and non-blocking assign in Verilog (or not!)

Question

I'm implementing a simple serializer in Verilog, but I do not understand the nuances of when blocking assigns can cause problems. I'm specifically having trouble understanding part of this answer. "However, you should never use blocking assignments for synchronous communication, as this is nondeterministic."

I'm building a block that takes, as an input:

• A bit clock
• A 5-bit parallel data input (the value to be serialized)
• A "Data valid" signal that indicates valid 5-bit data is present

As an output, I have:

• Serial data out
• A "Complete" signal that indicates it's time for a new 5-bit value
• A "Transmitting" signal that's high whenever there's valid serial data going out on the bus

Whenever data valid goes high, the block starts outputting the 5-bit value, one bit a time, starting at the next rising edge of the bit clock. When the last bit is out on the wire, the block signals "complete" so a new 5-bit value can be made available.

Omitting some of the reset logic, the code to do this looks like this:

always @ (posedge clk) begin
    if(shiftIndex == 0) begin
        if(dataValid == 1) transmitting = 1; //Blocking assign
        else transmitting = 0; //Blocking assign
    end

   //Need the blocking assign up above to get this part to run
   //for the 1st bit
   if(transmitting == 1) begin
       shiftIndex <= shiftIndex + 1;
       dataOut <= data5b[shiftIndex];

       if(shiftIndex == 4) begin
           complete <= 1;
           shiftIndex <= 0;
       end
       else begin
           complete <= 0;
       end
   end
end

Now, I can write the block with all non-blocking assigns, but I feel that it hurts readability. That would look something like this:

always @ (posedge clk) begin
    if(shiftIndex == 0) begin
        if(dataValid == 1) begin
            transmitting <= 1; //Non-blocking now
            shiftIndex <= shiftIndex + 1;  //Duplicated code
            dataOut <= data5b[shiftIndex]; //Duplicated code
            complete <= 0;                 //Duplicated code
        end
        else transmitting <= 0;
    end

   //Now, this only runs for the 2nd, 3rd, 4th, and 5th bit.
   else if(transmitting == 1) begin
       shiftIndex <= shiftIndex + 1;
       dataOut <= data5b[shiftIndex];

       if(shiftIndex == 4) begin
           complete <= 1;
           shiftIndex <= 0;
       end
       else begin
           complete <= 0;
       end
   end
end

Both appear to do what I want in simulation, and I favor the 1st one because it's easier for me to read but since I don't understand why using blocking assignments for synchronous communication is nondeterministic, I'm worried that I've coded up a ticking time bomb

The Question: Am I doing something wrong in the 1st code that's going to blow up when I try to synthesize this? Is the 2nd code preferable despite being a bit harder (for me anyway) to read? Is there some 3rd thing I should be doing?

Answer 1

When using the blocking (=) assignment the value is available to use in the next line of code. This implies it is combinatorial and not driven from a flip-flop.

In simulation it looks like it is driven from a flip-flop because the block is only evaluated on positive clock edge, in reality it is not which might break the interface.

I am of the faction which says never mix styles, as it can be a problem in code reviews and refactoring. The refactor, if a module needs to output a new signal and it is seen that it already exists they just change to be an output. At first glance looked like it was a flip-flop because it was in a always @(posedge clk block.

Therefore I would recommend to NOT mix styles, but pull out the section that is combinatorial and put it in its own block. Does this still meet your requirements? if not then you would have had problem.

I do not see how data valid is controlled but it can change the output transmitting, potentially transmitting could also glitch as it is from a combinatorial decode, not driven cleanly from a flip-flop. The receiving interface might be async, glitches could cause lock up etc.

always @* begin
  if(shiftIndex == 0) begin
    if(dataValid == 1) transmitting = 1; //Blocking assign
    else transmitting = 0; //Blocking assign
  end
end

always @ (posedge clk) begin
  if(transmitting == 1) begin
    shiftIndex <= shiftIndex + 1;
    dataOut    <= data5b[shiftIndex];

   if(shiftIndex == 4) begin
       complete   <= 1;
       shiftIndex <= 0;
   end
   else begin
       complete   <= 0;
   end
  end
end