This is a 32-bit ALU with a zero flag:
F2:0 Function
000 A AND B
001 A OR B
010 A + B
011 not used
100 A AND B
101 A OR B
110 A − B
111 SLT
SLT is "set less than"; it sets the least the output of ALU to 1 if A < B.
This is the ALU module:
module alu(input logic [31:0] a, input logic [31:0] b, input logic [2:0] f, output logic [31:0] out, output logic zero);
logic [31:0]tmp;
always @(a, b,f)
begin
if (f == 3'b000) // And
out = a & b;
else if( f == 3'b001) // Or
out = a | b;
else if( f == 3'b010) // Add
out = a + b;
else if( f == 3'b100) // New and
out = a & ~b;
else if( f == 3'b101) // New or
out = a | ~b;
else if( f == 3'b110) // SUB
out = a - b;
else if( f == 3'b111) // SLT
begin
tmp = a - b;
out[31:1] = 31'h0;
out[0] = (tmp[31] == 1'b1);
end
if (out == 32'h00000000)
zero = 1;
else
zero = 0;
end
endmodule
This is the testbench I built for the code:
module alu_tb;
reg[31:0] a;
reg [31:0] b;
reg[2:0] f;
wire [31:0] out;
wire zero;
alu DUT (a,b,f,out,zero);
initial begin
$dumpfile("alu.vcd");
$dumpvars(0, DUT);
$monitor("A = 0x%x, B = 0x%x, f=0b%b\n\tOut = 0x%x, z = %b", a, b,f, out, zero);
f = 3'b010; // 0 + 0
a = 32'h0000_0000;
b = 32'h0000_0000;
#10
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%s0 + 0 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b010; // 0 + (-1)
a = 32'h0000_0000;
b = 32'hFFFF_FFFF;
#10
if ( out !== 32'hFFFF_FFFF | zero !== 1'b0)
$display("\t%s0 + (-1) failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'hFFFF_FFFF, 1'b0, "033円[0m");
f = 3'b010; // 1 + (-1)
a = 32'h0000_0001;
b = 32'hFFFF_FFFF;
#10
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%s1 + (-1) failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b010; // FF + 1
a = 32'h0000_00FF;
b = 32'h0000_0001;
#10;
if ( out !== 32'h100 | zero !== 1'b0)
$display("\t%s0xFF + 1 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h100, 1'b0, "033円[0m");
f = 3'b110; // 0 - 0
a = 32'h0000_0000;
b = 32'h0000_0000;
#10;
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%s0 - 0 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b110; // 0 - (-1)
a = 32'h0000_0000;
b = 32'hFFFF_FFFF;
#10;
if ( out !== 32'h1 | zero !== 1'b0)
$display("\t%s0 - (-1) failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h1, 1'b0, "033円[0m");
f = 3'b110; // 1 - 1
a = 32'h0000_0001;
b = 32'h0000_0001;
#10;
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%s1 - 1 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b110; // 100 - 1
a = 32'h0000_0100;
b = 32'h0000_0001;
#10;
if ( out !== 32'hFF | zero !== 1'b0)
$display("\t%s100 - 1 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'hFF, 1'b0, "033円[0m");
f = 3'b111; // SLT 0, 0
a = 32'h0000_0000;
b = 32'h0000_0000;
#10;
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%sSLT 0, 0 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b111; // SLT 0, 1
a = 32'h0000_0000;
b = 32'h0000_0001;
#10;
if ( out !== 32'h1 | zero !== 1'b0)
$display("\t%sSLT 0, 1 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h1, 1'b0, "033円[0m");
f = 3'b111; // SLT 0, -1
a = 32'h0000_0000;
b = 32'hFFFF_FFFF;
#10;
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%sSLT 0, -1 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b111; // SLT 1, 0
a = 32'h0000_0001;
b = 32'h0000_0000;
#10;
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%sSLT 1, 0 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b111; // SLT -1, 0
a = 32'hFFFF_FFFF;
b = 32'h0000_0000;
#10;
if ( out !== 32'h1 | zero !== 1'b0)
$display("\t%sSLT -1, 0 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h1, 1'b0, "033円[0m");
f = 3'b000; // -1 & -1
a = 32'hFFFF_FFFF;
b = 32'hFFFF_FFFF;
#10;
if ( out !== 32'hFFFF_FFFF | zero !== 1'b0)
$display("\t%s 0xFFFFFFFF & 0xFFFFFFFF failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'hFFFF_FFFF, 1'b0, "033円[0m");
f = 3'b000; // -1 & 12345678
a = 32'hFFFF_FFFF;
b = 32'h1234_5678;
#10;
if ( out !== 32'h1234_5678 | zero !== 1'b0)
$display("\t%s0xFFFFFFFF & 0x12345678 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h12345678, 1'b0, "033円[0m");
f = 3'b000; // 12345678 & 87654321
a = 32'h1234_5678;
b = 32'h8765_4321;
#10;
if ( out !== 32'h02244220 | zero !== 1'b0)
$display("\t%s0x12345678 & 0x87654321 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h02244220, 1'b0, "033円[0m");
f = 3'b000; // -1 & 0
a = 32'hFFFF_FFFF;
b = 32'h0000_0000;
#10;
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%s0xFFFFFFFF & 0x0 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
f = 3'b001; // -1 | -1
a = 32'hFFFF_FFFF;
b = 32'hFFFF_FFFF;
#10;
if ( out !== 32'hFFFF_FFFF | zero !== 1'b0)
$display("\t%s0xFFFFFFFF | 0xFFFFFFFF failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'hFFFFFFFF, 1'b0, "033円[0m");
f = 3'b001; // 12345678 | 87654321
a = 32'h1234_5678;
b = 32'h8765_4321;
#10;
if ( out !== 32'h9775_5779 | zero !== 1'b0)
$display("\t%s0x12345678 | 0x87654321 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h97755779, 1'b0, "033円[0m");
f = 3'b001; // 0 | -1
a = 32'h0000_0000;
b = 32'hFFFF_FFFF;
#10;
if ( out !== 32'hFFFF_FFFF | zero !== 1'b0)
$display("\t%s0x0 | 0xFFFFFFFF failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'hFFFFFFFF, 1'b0, "033円[0m");
f = 3'b001; // 0 | 0
a = 32'h0000_0000;
b = 32'h0000_0000;
#10;
if ( out !== 32'h0 | zero !== 1'b1)
$display("\t%s0 | 0 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h0, 1'b1, "033円[0m");
$finish;
end
endmodule
Improvement to the ALU code or the testbench will be appreciated.
2 Answers 2
For the design
Issues
The biggest issues with your code is out is an inferred latch because the condition f == 3'b011 is undefined. This type of latch is not ideal as they can cause area and timing issues. To remove the latch, simply assign out to a determinate value; out = a, out = b, out = 0, or other constant (not out = out which is still a latch).
Depending on your synthesizer, tmp may be treated as a latch. If it is, you should move the tmp = a - b above the if/case statement. Or code STL differently and not need tmp.
Improvements
Other than using declaring the port data types as logic, you have not used any SystemVerilog constructs.
always @(a, b,f) is a one Verilog 2001 way of declaring a combinational block and its sensitivity list. Verilog 1995 required or instead of ,. Verilog 2001 also introduced auto-sensitivity (@* or the synonymous @(*)) which is preferred over manually managed sensitivity lists; especially when the list is long. Your @(a, b,f) is not wrong; it could be better. For more refer to IEEE1800-2012 § 9.4.2.2 Implicit event_expression list.
SystemVerilog when one step further and introduced always_comb as an improvement over always @*. always_comb throws a compiling error when one of the variables it is assigning is also assigned by any other block (if not caught here it would have been an error in synthesis). It also allows the designer to specify the intention of the block to lint, lec, synthesis, and other tools that the block is combinational logic. This allows those tools to flag an warning/error if they detect a latch in the logic. (There is always_latch for when you want a latch). For more refer to IEEE1800-2012 § 9.2.2.2.2 always_comb compared to always @*.
You may consider changing chained else-ifs statements into an case-statement. It can be easier for the synthesizer to detect a a full case logic with a case statement than else-if statements. Since you are using SystemVerilog, you may want to consider the unique or priority prefix depending your your target area/performance/design-constraints.
For the test bench
The test bench is very brute forced testing only about 21 conditions; not testing an conditions where f == 3'b100 or f == 3'b101. There are trillions of legal combinations. It unreasonable to check all of them, but you need to check the major conferrers: all legal values of f, overflow, underflow, and random values.
I suggest adding a clock to your test bench to synchronizes randomization and checking. This way you can randomize your values and use assertions to check them. Read about assertions in IEEE1800-2012 § 16. Example of an assertion:
a_SLT : assert property(@(posedge clk) f==3'b111 |-> out == (a<b))
else $error("%0t : STL failed out == %0h expected %0h with a = %h, b = %h, was out = %h", $time, out, (a<b), a, b);
You might also want to look into functional coverage (IEEE1800-2012 § 19), to get an idea that your test bench has covered and spot possible coverage holes.
There are plenty of advance test bench practices and strategies; such as UVM and formal analysis, that I will not cover. For your assignment automated checking and constrained randomization should get you what you need.
Reminder: a and b are unsigned, therefore 32'hFFFF_FFFF is 4294967295, not -1
Overview
The previous answer is thorough, and there are many good suggestions that I won't repeat in this review.
There is a lot to like about the approach:
- Good partitioning between design and testbench
- Uses the recommended good coding practice using blocking assignments for combinational logic
- The testbench is self-checking
- You dump VCD waveforms which is crucial for debugging
Design
Comments
It is common to add a header comment summarizing the design code, such as:
/*
Arithmetic Logic Unit (ALU)
It implements many common ALU functions, like addition, subtraction, etc.
SLT means "set less than".
*/
Naming
Generally, the module and signals have meaningful names. However, here are some suggestions.
f is a bit vague. I recommend func_sel for "function select".
out is too generic. result is more common for an ALU.
Layout
Instead of one very long line:
module alu(input logic [31:0] a, input logic [31:0] b, input logic [2:0] f, output logic [31:0] out, output logic zero);
it is customary to have one port per line:
module alu (
input logic [31:0] a,
input logic [31:0] b,
input logic [2:0] f,
output logic [31:0] out,
output logic zero
);
Partitioning
The temptation is to cram as much code into a single always block as possible.
However, this often leads to complicated code.
It is better to partition the code to avoid this complication. In this case,
the logic for zero should be removed from the always block.
We can also simplify the code by removing the if/else:
assign zero = (out == 32'h0000_0000);
Testbench
Display
For better debugging, it is helpful to add $time to display statements
to show the simulation time.
Also, instead of displaying a 0x for hex values, prefer 'h. Since x
is also a legal 4-state value, seeing "x" in the output it ambiguous:
$monitor($time, " A = 'h%x, B = 'h%x, f='b%b\n\tOut = 'h%x, z = %b", a, b, f, out, zero);
Race condition
$monitor is fine for quick debugging, but using $display inside an always block
is often more meaningful. We can leverage the idea from the previous answer to create a clock
signal inside the testbench for driving inputs and sampling outputs. For example, inputs
could be driven on the posedge of the clock, and outputs could be sampled on the negedge.
This assures that signals will be sampled at a different time from when they are changing.
This avoids potential simulation race conditions.
Connections
The module instance uses connections-by-order:
alu DUT (a,b,f,out,zero);
This often leads to a common Verilog pitfall. It is better to use connections-by-name:
alu DUT (
.a (a),
.b (b),
.f (f),
.out (out),
.zero (zero)
);
Although it is more verbose, it also is more self-documenting.
SystemVerilog
As mentioned, you are already using some SV features (logic).
You could also change reg to bit in the testbench for the design inputs:
bit [31:0] a;
bit [31:0] b;
bit [2:0] f;
Typically, you don't need to drive z/x on your DUT input ports. The 2-state values could result in better simulation performance.
SV allows you to specify time units for delays:
#10;
would be:
#10ns
assuming your units are ns.
DRY
Lines like this are mostly repeated throughout the code:
f = 3'b111; // SLT 0, 1
a = 32'h0000_0000;
b = 32'h0000_0001;
#10;
if ( out !== 32'h1 | zero !== 1'b0)
$display("\t%sSLT 0, 1 failed.\tExpected out = 0x%0x, z = %b%s","033円[0;31m", 32'h1, 1'b0, "033円[0m");
This would be a good use for a task, where you pass in the values to drive on the inputs
and expected output values.
More checks
The testbench currently checks 5 of the 8 possible values for f.
You should add checks for the 2 other supported functions (4 and 5),
as well as the unsupported function (3).