# SystemVerilog implementation of an N-bit prefix adder logic design

I want some feedback about my code (anything is welcome). It is working, but it feels like a clumsy implementation. Because I am self-learning from a book without an answers section, it becomes difficult to know how to proceed.

module prefix_adder #(parameter N=4)
(input logic [2**N-1:0] a, b,
input logic cin,
output logic [2**N-1:0] s);

genvar i, j;
logic [2**N-1:0] g[N:0], p[N:0];

assign p[0][0] = 0;
assign g[0][0] = cin;

generate
for (j=0; j<(N**2-1); j++) begin
assign p[0][j+1] = a[j] | b[j];
assign g[0][j+1] = a[j] & b[j];
end
endgenerate

generate
for(i=1; i<=N; i++) begin
for (j=0; j<N**2; j++) begin
if(j%2**i <= 2**(i+1) && j%2**i >= 2**(i-1)) begin
assign p[i][j] = p[i-1][j] & p[i-1][j-1-(j%(2**i))+2**(i-1)];
assign g[i][j] = g[i-1][j] | p[i-1][j] & g[i-1][j-1-(j%(2**i))+2**(i-1)];
end
else begin
assign p[i][j] = p[i-1][j];
assign g[i][j] = g[i-1][j];
end
end
end
endgenerate

generate
for (j=0; j<N**2; j++) begin
assign s[j] = a[j] ^ b[j] ^ g[N][j];
end
endgenerate
endmodule


The code uses good indentation, and it makes good use of for loops to avoid code duplication. It's great that you used a parameter to make the design easily scalable to other bit widths.

While it is common to use brief names for the module ports for an adder (like a, b and s), because they are self-evident, you might want to give meaningful names for the internal signals, g and p. At the very least, you should add comments to the code to describe why you chose those names. If you are following a known algorithm, you should add comments describing the algorithm and provide a reference to a book, paper, website, etc.

The sum does not work for large values of the inputs. For example, when a and b are their maximum values ('1), the sum will overflow. To handle the maximum values, you need to increase the width of the s output. However, if that is not the design intent, and you assume restrictions on the input values, you should add comments to the code regarding those restrictions.

If there are any assumed restrictions on the parameter N, you should also document that with comments.

When a line has multiple operators, I prefer to use extra parentheses around each term to avoid potential operator precedence issues and to make the code easier to read.

I realize you are using this as a way to learn SystemVerilog, but you should not get too bogged down in trying to design adders with specific implementations. It is common to use synthesis tools to convert high-level Verilog code into hardware, and those tools are great at optimizing the design for speed/area/power. It is most common to design an adder with a single line of code:

assign s = a + b;


I strongly recommend testing your code with a simple self-checking testbench, such as this example.

• Thanks, toolic. I had one more thing to ask. Is the generate block written with good practices? Commented Aug 21, 2023 at 22:46
• @MiguelOrtega: Yes, the generate blocks look good. Commented Aug 21, 2023 at 23:07
• I would recommend to add parenthesis in all expressions like j%2**i . You missed some in the condition of the 'if' statement. Commented Aug 22, 2023 at 1:10