Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm having a hard time figuring out if the code I wrote is purely combinatorial or sequential logic. I'm designing a simple 16-bit microprocessor (will be implemented on a Spartan 6) and I'm new to Verilog, HDL and FPGAs. The code for the microprocessor is complete, but I'm having second thoughts about the best practices behind the code.

I'm aware that since this is the first time coding for me, the code is not up to any standard, but I tried my best.

One of the most important elements is the ALU, and it has been designed like any other person would. It has overflow/underflow detection which I also asked about here and I quickly wrote some code for it, may or may not be correct.

But my question is whether I should be using the non-blocking operator (<=) or blocking operator (=) in the always block for the ALU. I know the standard practice is to use the blocking operator while designing combinatorial circuits versus using the non-blocking one, which is better for sequential circuits.

If I was to use blocking operators in the always block for the ALU, would be synthesized version be slower than if I use non-blocking operators? I plan to use the on-board 100MHz clock on my development board, so I was wondering if the ALU could keep up.

Here's the full code for the ALU:

module alu(clk, rst, en, re, opcode, a_in, b_in, o, z, n, cond, d_out);

// Parameter Definitions

parameter width = 'd16; // ALU Width

// Inputs

input wire clk /* System Clock Input */, rst /* Reset Result Register */, en /* Enables ALU Processing */, re /* ALU Read Enable */;
input wire [4:0] opcode /* 5-bit Operation Code for the ALU. Refer to documentation. */;
input wire signed [width-1:0] a_in /* Operand A Input Port */, b_in /* Operand B Input Port */;

// Outputs

output wire z /* Zero Flag Register (Embedded in res_out) */, n /* Negative/Sign Flag Register */;
output reg o /* Overflow/Underflow/Carry Flag Register */;
output reg cond /* Conditional Flag Register */;
output wire [width-1:0] d_out /* Data Output Port */;

// Internals

reg [1:0] chk_oflow /* Check for Overflow/Underflow */;
reg signed [width+width:0] res_out /* ALU Process Result Register */;

// Flag Logic

assign z = ~|res_out; // Zero Flag
assign n = res_out[15]; // Negative/Sign Flag
assign d_out [width-1:0] = res_out [width-1:0]; // Read Port

// Tri-State Read Control

assign d_out [width-1:0] = (re)?res_out [width-1:0]:0; // Assign d_out Port the value of res_out if re is true.

// Overflow/Underflow Detection Block

always@(chk_oflow) begin
    if(rst) o <= 1'b0;
    else begin
        case(chk_oflow) // synthesis parallel-case
            2'b00: o <= 1'b0;
            2'b01: begin
                if(res_out [width:width-1] == (2'b01 || 2'b10)) o <= 1'b1; // Scenario only possible on Overflow/Underflow.
                else o <= 1'b0;
            end
            2'b10: begin
                if((res_out[width+width]) && (~res_out [width+width-1:width-1] != 0)) o <= 1'b1; // Multiplication result is negative.
                else if ((~res_out[width+width]) && (res_out [width+width-1:width-1] != 0)) o <= 1'b1; // Multiplication result is positive.
                else o <= 1'b0;
            end
            2'b11: o <= 1'b0;
            default: o <= 1'b0;
        endcase
    end
end

// ALU Processing Block

always@(posedge clk) begin
    if(en && !rst) begin
        case(opcode) // synthesis parallel-case
            5'b00000: begin
                res_out [width-1:0] <= a_in [width-1:0]; // A
            end
            5'b00001: begin
                res_out [width-1:0] <= b_in [width-1:0]; // B
            end
            5'b00010: begin
                res_out [width-1:0] <= a_in [width-1:0] + 1'b1; // Increment A
            end
            5'b00011: begin
                res_out [width-1:0] <= b_in [width-1:0] + 1'b1; // Increment B
            end
            5'b00100: begin
                res_out [width-1:0] <= a_in [width-1:0] - 1'b1; // Decrement A
            end
            5'b00101: begin
                res_out [width-1:0] <= b_in [width-1:0] - 1'b1; // Decrement B
            end
            5'b00110: begin
                chk_oflow <= 2'b01;
                res_out [width:0] <= {a_in[width-1], a_in [width-1:0]} + {b_in[width-1], b_in [width-1:0]}; // Add A + B
            end
            5'b00111: begin
                chk_oflow <= 2'b01;
                res_out [width:0] <= {a_in[width-1], a_in [width-1:0]} - {b_in[width-1], b_in [width-1:0]}; // Subtract A - B
            end
            5'b01000: begin
                chk_oflow <= 2'b10;
                res_out [width+width:0] <= a_in [width-1:0] * b_in [width-1:0]; // Multiply A * B
            end
            5'b01001: begin
                res_out [width-1:0] <= ~a_in [width-1:0]; // One's Complement of A
            end
            5'b01010: begin
                res_out [width-1:0] <= ~b_in [width-1:0]; // One's Complement of B
            end
            5'b01011: begin
                res_out [width-1:0] <= ~a_in [width-1:0] + 1'b1; // Two's Complement of A
            end
            5'b01100: begin
                res_out [width-1:0] <= ~b_in [width-1:0] + 1'b1; // Two's Complement of B
            end
            5'b01101: begin
                if(a_in [width-1:0] == b_in [width-1:0]) cond <= 1'b1; // Compare A == B, set Conditional Register as result
                else cond <= 1'b0;
            end
            5'b01110: begin
                if(a_in [width-1:0] < b_in [width-1:0]) cond <= 1'b1; // Compare A < B, set Conditional Register as result
                else cond <= 1'b0;
            end
            5'b01111: begin
                if(a_in [width-1:0] > b_in [width-1:0]) cond <= 1'b1;// Compare A > B, set Conditional Register as result
                else cond <= 1'b0;
            end
            5'b10000: begin
                res_out [width-1:0] <= a_in [width-1:0] & b_in [width-1:0]; // Bitwise AND
            end
            5'b10001: begin
                res_out [width-1:0] <= a_in [width-1:0] | b_in [width-1:0]; // Bitwise OR
            end
            5'b10010: begin
                res_out [width-1:0] <= a_in [width-1:0] ^ b_in [width-1:0]; // Bitwise XOR
            end
            5'b10011: begin
                res_out [width-1:0] <= a_in [width-1:0] ~& b_in [width-1:0]; // Bitwise NAND
            end
            5'b10100: begin
                res_out [width-1:0] <= a_in [width-1:0] ~| b_in [width-1:0]; // Bitwise NOR
            end
            5'b10101: begin
                res_out [width-1:0] <= a_in [width-1:0] ~^ b_in [width-1:0]; // Bitwise XNOR
            end
            5'b10110: begin
                res_out [width-1:0] <= {a_in [width-2:0], 1'b0}; // Logical Left Shift A
            end
            5'b10111: begin
                res_out [width-1:0] <= {b_in [width-2:0], 1'b0}; // Logical Left Shift B
            end
            5'b11000: begin
                res_out [width-1:0] <= {1'b0, a_in [width-1:1]}; // Logical Right Shift A
            end
            5'b11001: begin
                res_out [width-1:0] <= {1'b0, b_in [width-1:1]}; // Logical Right Shift B
            end
            5'b11010: begin
                res_out [width-1:0] <= {a_in [width-1], a_in [width-1:1]}; // Arithmetic Right Shift A
            end
            5'b11011: begin
                res_out [width-1:0] <= {b_in [width-1], b_in [width-1:1]}; // Arithmetic Right Shift B
            end
            5'b11100: begin
                res_out [width-1:0] <= {a_in [width-2:0], a_in [width-1]}; // Rotate Left A
            end
            5'b11101: begin
                res_out [width-1:0] <= {b_in [width-2:0], b_in [width-1]}; // Rotate Left B
            end
            5'b11110: begin
                res_out [width-1:0] <= {a_in [0], a_in [width-1:1]}; // Rotate Right A
            end
            5'b11111: begin
                res_out [width-1:0] <= {b_in [0], b_in [width-1:1]}; // Rotate Right B
            end
            default: begin
                cond <= 1'b0;
                res_out [width-1:0] <= 0;
            end
    end else if(rst) begin
        cond <= 1'b0;
        chk_oflow <= 2'b0;
        res_out [width-1:0] <= 0;
    end
end

endmodule

There is a good chance I'll reduce the number of operations it performs to reduce the amount of unnecessary operations it does on both A and B, since less operations translates to better RISC performance.

My question is, would a blocking or non-blocking operator be appropriate in this case?

Edit: Apologies that the syntax highlighting seems poor.

share|improve this question
    
Just FWIW, the Xilinx tools will tell you how fast a particular design can run on their hardware. At least in my experience, their estimates are usually pretty accurate. –  Jerry Coffin Jul 7 at 11:37
    
The estimate for the previous processor design (I changed the architecture as a whole after seeing how poor the last one was) was a max of around 127MHz as far as I remember. It could perhaps handle it. –  Shreyas Vinod Jul 7 at 15:27

1 Answer 1

up vote 3 down vote accepted

non-blocking one, which is better for sequential circuits.

It is not better per-se but it is the correct way to simulate a flip-flop.

Combinatorial

always @* begin
  a = b;

Sequential (flip-flop)

always @(posedge clock) begin
  a <= b ;

In the examples above nothing would go wrong if you used the wrong type but think about

always @(posedge clock) begin
  b <= c ;
  a <= b ;

Which is the same as

always @(posedge clock) begin
  a <= b ;
  b <= c ;

We are specifying a delay line which is c -> b -> a. If we use the wrong type :

always @(posedge clock) begin
  b = c ;
  a = b ; //=c

We actually get c -> b and c -> a, b does not block and feeds directly into a. Mixing the styles is possible but unless done very carefully bugs can creep in. for the purpose of code review it is best not to mix them so that it is a clear cut case.

Which will give a different result to :

always @(posedge clock) begin
  a = b ;
  b = c ;

When implying parallel hardware you would not expect an order dependence like this.

Mixing styles, or using the wrong style can lead to RTL vs Gate level mismatch. ie using a <= in a combinatorial section always @* will give the desired result in simulation but synthesis will ignore this and give you the equivalent of =.

share|improve this answer
    
Most of the documentation on this online only sheds light on simulations. Some say that a non-blocking operator would evaluate the RHS of the expression when the always block is triggered, but only update the LHS at the next clock edge. This doesn't make sense to me as it wouldn't work in hardware. I've assumed that it will simply do it as quickly as possible (after obvious delays caused by hardware) and update the register regardless of how fast the clock is. Is this true? –  Shreyas Vinod Jul 7 at 15:33
    
Your kind of correct. always @(posedge clk) sum <= a+b sum is a flip-flop, a+b is a combinatorial input. when the positive edge of clk triggers the block a+b is copied to a temporary register, delta cycles are used to calculate other combinatorial loops etc, new values for a and b are set. Then sum the temporary (old) value of a+b is assigned to sum. It is just a way of simulating flip-flop behaviour. –  Munkymorgy Jul 7 at 17:00

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.