I have the task of designing a ring counter in verilog using shift operator. Here is the code so far along with test bench :

module ring (

input wire [3:0] data,
input wire clk,
input wire rst,
input wire load,
output reg [3:0] q 
) ;

/*When the load pin is High, the data is loaded in the counter on clock transition.
* Otherwise the data is shifted  upon each clock. 
* Last output is fed back to the first input.

initial     q = 4'b1010 ;

reg tmp = 1'b0 ;

always @ (posedge clk, posedge rst)
    q <= 4'b0000 ;
else if(load)
    q[3] <= data[3] ;
    q[2] <= data[2] ;
    q[1] <= data[1] ;
    q[0] <= data[0] ;

    tmp = q[0] ;
    q = q >> 1 ;
    q[3] = tmp ;

Test bench

`timescale 1 ns / 1 ns

module ringTest ;

reg [3:0] data = 4'b1010 ;
wire [3:0] q = 4'b0000 ;
reg clk = 1'b0 ;
reg rst = 1'b0 ;
reg load = 1'b0 ;

//Instantiate uut
ring uut (  .data(data),

always  #10 clk = ~clk ;

/*Load data every 100 s
#95 load = 1'b1 ;
#5 load = 1'b0 ;
//Data to be loaded
if ( data == 4'b1111) data <= 4'b0000 ;
else    begin
    # 50 data <= data + 4'b0001 ;

Note : I have temporarily disabled load functionality bcoz the Counter wasnt working. So instead I chose to shift the data with an initial value, which also did not work. The output is always X.

Where am I going wrong ?

Thank you.

  • \$\begingroup\$ I don't see where your testbench ever asserts rst. Also, I'd advise resetting to something other than 4'b0000 or 4'b1111 if you actually want to see that the counter is doing something. \$\endgroup\$
    – The Photon
    Feb 16, 2015 at 17:51
  • \$\begingroup\$ I've temporarily disabled load and rst functionality. Neverthless, shouldn't my counter shift the initial value ? \$\endgroup\$ Feb 16, 2015 at 17:53
  • \$\begingroup\$ Maybe the simulator is confused because you assign to q in both modules? q is an output from your counter. Don't assign it in your testbench. \$\endgroup\$
    – The Photon
    Feb 16, 2015 at 17:57
  • \$\begingroup\$ @ThePhoton . Thanks a lot. It works after removing it from TB. Was stuck on it for 2 days. \$\endgroup\$ Feb 16, 2015 at 18:03
  • \$\begingroup\$ Feel free to answer your own question. I didn't post my comment as an answer because I was really just shotgunning in case it would help you find the problem. \$\endgroup\$
    – The Photon
    Feb 16, 2015 at 19:31

1 Answer 1


First of all, initial commands are not used in synthesizable Verilog code. And I am not sure if I follow why you are using it, given you have already a reset state for it.

Secondly, in you else statement, you are using a blocking assignment in "tmp = q[0]" which is ok, because it stores q[0] value in a variable internal to the process.

What is not OK, is continuing using blocking assignments in "q = q << 1" and in "q[3] = tmp". It does not make any sense, you are modeling a sequential output, you should model it using a non-blocking assignment like: "q <= q << 1" and "q[3] <= temp". I don't know what the simulator will do, but probably he will change q signal as soon as it executes this line, and not at the next rising edge clock as you should expect. As the synthesizer won't be capable of doing that, you are going to have hardware behavior which is not modeled by you simulation. (very serious)

I have also noticed you do not perform a reset in your module. This is surely a malpractice, the first thing you should do, before pumping any data into your module is to reset it.

Lastly, have you checked in you waveform that you have a rising edge at the same time you send a load signal level high? As far as I have read your test bench I do not believe this occurs.

So you don't reset you module, you don't send a load at a rising edge, and you are complaining your output is at X? Well, you should re-read all the basics of Verilog, and learn how to properly use a simulator.

  • \$\begingroup\$ Actually, I did all of this. But my design wouldnt work because as mentioned in the comments above, I was initialising an output port in test bench. So inorder to find out the error, I temporarily removed all non critical functionality like load and reset with an assumption that my counter should be able to shift any initial value as well. Thats why you see an initial block in main module. As far as non blocking assignment is concerned, i'll definitely change it now that you have pointed. \$\endgroup\$ Feb 17, 2015 at 6:34
  • 1
    \$\begingroup\$ @Plutoniumsmuggler Yes, I understand. You should never define the signal connected to an output from your DUT as regs, nor assign values to them, because it may lead to two entities driving the same signal. A good simulator would detect this condition, give an error and stop the simulation. \$\endgroup\$
    – gstorto
    Feb 17, 2015 at 8:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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