ALU design for a 16-bit microprocessor

Question

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, which 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 the 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.

Would a blocking or non-blocking operator be appropriate in this case?

Answer 1

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 =.

Answer 2

Syntax errors

Before addressing your main concern, it is important to note that the posted code has syntax errors. If your simulation software did not generate compile errors, then it is not compliant with the Verilog IEEE Std.

The second case statement is missing its corresponding endcase keyword.

The ~& and ~| operators are illegal in the following lines of code:

        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

In this context, you are trying to use them as binary operators, but they can only be used as unary reduction operators. You need to review the code for the intended behavior.

Sensitivity lists

The second always block properly describes the desired sequential logic:

always@(posedge clk) begin

However, the first always block is problematic because it has an incomplete sensitivity list:

always@(chk_oflow) begin

The block is only sensitive to one signal (chk_oflow). Typically, this type of list is used to describe combinational logic, but you likely intended it to describe sequential logic. I recommend changing it to:

always @(posedge clk) begin

Parameters

It is great that you used a parameter for the constant value used throughout the code.

However, it is common practice to name a constant using all capital letters: WIDTH.

Since the code uses the same constant expression repeatedly (width+width), you could declare this as another parameter.

Naming

The signal named z is not very meaningful. It should be zero_flag. Additionally, since Verilog uses z as a special value for high-impedance, it is confusing to name a signal z.

You should give these other signals more meaningful names as well: o and n

Constant expression

The expression (2'b01 || 2'b10) resolved to the constant 1. You should review the code to make sure this was intentional.

Whitespace

When using vector signals in an expression, it is not common to have space between the signal name and the opening square bracket:

res_out [width:width-1]

This is more conventional:

res_out[width:width-1]

Port list

It is better to use ANSI-style module port lists to eliminate duplicating the signal lists.

Also, it is easier to understand the design if you declare one port per line.

module alu #(
    parameter WIDTH  = 16,           // ALU Width
    parameter WIDTH2 = WIDTH + WIDTH // ...
)
(
    input clk /* System Clock Input */,
    input rst /* Reset Result Register */,
    input en /* Enables ALU Processing */,
    input re /* ALU Read Enable */,
    input [4:0] opcode /* 5-bit Operation Code for the ALU. Refer to documentation. */,
    input signed [WIDTH-1:0] a_in /* Operand A Input Port */,
    input signed [WIDTH-1:0] b_in /* Operand B Input Port */,
    output z /* Zero Flag Register (Embedded in res_out) */,
    output n /* Negative/Sign Flag Register */,
    output reg o /* Overflow/Underflow/Carry Flag Register */,
    output reg cond /* Conditional Flag Register */,
    output [WIDTH-1:0] d_out /* Data Output Port */
);

Refer to IEEE Std 1800-2017, section 23.2.2.2 ANSI style list of port declarations