How to write program counter in SystemVerilog?

Question

I was trying to build an 8-bit program counter, and it should support branching. This is what I wrote:

module programme_counter (clock,sel,immediate,pc);
input [7:0] immediate;
input clock,sel;
output [7:0] pc;
begin
 always @ (posedge clock)
 if (pc<3'd256)
 if (sel)
 pc <= pc + 1;
 else
 assign pc = immediate;
 else
 pc <= 0;
end
endmodule

The functionality of this code should be:

at every rising edge of the clock signal:
 if pc value < 256
 if sel == 1
 pc at the next rising edge should be the pc before the next edge + 1
 if sel ==0
 pc at the next rising edge should be immediate value
 if pc value >= 256
 pc at the next rising edge should be 0

Can anyone help me to check if the code describes the function correctly?

• toolic: Since you are a new user: What should I do when someone answers my question?

Answer 1

One problem is that you declared the pc signal as 8 bits wide. This means pc can be any value from 0 to 255. It can not be 256 or greater. So, the expression if pc value < 256 it always true, and the expression if pc value >= 256 is never true.

Keep in mind that an 8-bit counter will automatically roll over. If pc is 255, on the next posedge of clock, it will increment and roll over to 0.

With that in mind, here is a version of your code that compiles without errors:

module programme_counter (clock,sel,immediate,pc);
 input [7:0] immediate;
 input clock,sel;
 output reg [7:0] pc;
 always @ (posedge clock)
 if (sel)
 pc <= pc + 1;
 else
 pc <= immediate;
endmodule

You must decide if it meets your requirements. The way to check if your Verilog code behaves the way you expect is to write a testbench, also using Verilog code. Refer to Can anyone help me to create a Verilog testbench?

Here are some syntax notes:

• You should not use the assign keyword inside an always block
• You should use nonblocking assignments (<=) everywhere in your sequential logic always block
• There's no need for the begin/end keywords outside the always block
• You should declare pc as a reg since you make procedural assignments to it (inside the always block)