[SOLVED] Shift register implementation and use of its value

[rogger201]

I desire to implement an operation which uses shifting of data and using the MSB for output side.
One data needs to be shifted 1 bit at a time and the MSB of this shifted data needs to be used in every cycle with another input data.
I am having an issue with the fact that the MSB bit isnt changing at all. As if that bit is not getting communicated with my module that uses this bit to send to the output.

I will type the code that I am using :

input [7:0] input_data;
input [7:0] send_data;
input [1:0] sel;
output reg [8:0] out;
reg [7:0] shift_data;

//shift register 
always @(posedge clock) begin 
  if (reset) shift_data = 0;  

  else if (load) shift_data = input_data;  

  else shift_data = {1'b0, shift_data[7:1]};
end 

//this is the module that makes use of this shifted data's MSB along with another input.
always@(input_data or send_data or sel)
begin 
case (sel)
2'b00: out = {send_data[5:7], shift_data[0]};
.
.
.
//other cases that are irrelevant here

It seems like the shift_data is not getting communicated with the case blocks. How can I make it work?

 

[FvM]

Are you reporting a problem in simulation? shift_data is missing in the sensitivity list. Try always @(*).

 

[rogger201]

Shift register implementation without load signal

I want to implement the shift register without the load signal

This is the original code

always @(posedge clock) begin 
  if (reset) shift_data = 0;  

  else if (load) shift_data = input_data;  

  else shift_data = {1'b0, shift_data[7:1]};
end

How do i make shift_data take the value of input_data without using Load signal?

 

[FvM]

The question makes no sense. There must be a criterion to decide if the shift register is shifted or loaded with a new value. The load signal can be e.g. generated based on a bit counter.

 

[rogger201]

Hi!

I always want to shift the data. The loading is only necessary on reset. Do we still need a load signal?

 

[ads-ee]

A shift register is written like this:

---

Code Verilog - [expand]
1
2
3
4
// right shift, shift register with no reset
always @ (posedge clk) begin
  shift_reg <= {serial_in_data, shift_reg[7:1]};  // shifts right one bit on every clock
end

---

---

Code Verilog - [expand]
1
2
3
4
// left shift
always @ (posedge clk) begin
  shift_reg <= {shift_reg[6:0], serial_in_data};   // shifts left one bit on every clock
end

---

---

Code Verilog - [expand]
1
2
3
4
5
6
7
8
// left shift with async reset
always @ (posedge clk, posedge rst) begin
  if (rst) begin
    shift_reg <= 0;
  end else begin
    shift_reg <= {shift_reg[6:0], serial_in_data};  // shifts left one bit on every clock cycle that is not during a reset
  end
end

---

---

Code Verilog - [expand]
1
2
3
4
5
6
7
8
9
10
// left shift with with async reset and load value
always @ (posedge clk, posedge rst) begin
  if (rst) begin
    shift_reg <= 0;
  end else if (load) begin
    shift_reg <= parallel_load_value[7:0];  // loads the entire shift register with a new value has nothing to do with in_data.
  end else begin
    shift_reg <= {shift_reg[6:0], serial_in_data};  // shifts left one bit on every clock cycle that is not during a reset or when load is active
  end
end

---

 

[FvM]

I always want to shift the data. The loading is only necessary on reset. Do we still need a load signal?

The shift register runs empty eight clock cycles after reset.

 

[rogger201]

The shift register runs empty eight clock cycles after reset.

Why?
I tried doing this

always@(posedge clock) begin 
if (reset) shift_reg = input_data;
else        shift_reg = {1'b0, input_data[7:1]};

So all of the data gets loaded in in the first cycle after reset .
This way its achieved in one cycle. Either way I want to use the last bit so I just keep shifting in 0's every clock. Am I going wrong somewhere?

 

[barry]

Why?
I tried doing this

always@(posedge clock) begin 
if (reset) shift_reg = input_data;
else        shift_reg = {1'b0, input_data[7:1]};

So all of the data gets loaded in in the first cycle after reset .
This way its achieved in one cycle. Either way I want to use the last bit so I just keep shifting in 0's every clock. Am I going wrong somewhere?

You keep ignoring post #2. PUT RESET IN YOUR SENSITIVITY LIST!!!

 

[ads-ee]

You keep ignoring post #2. PUT RESET IN YOUR SENSITIVITY LIST!!!

It doesn't have to be in the sensitivity list as it could just be a synchronous reset (i.e. LOAD signal)

- - - Updated - - -

Why?
I tried doing this

always@(posedge clock) begin 
if (reset) shift_reg = input_data;
else        shift_reg = {1'b0, input_data[7:1]};  [COLOR="#FF0000"][B]// ads-ee: this is not a shift operation it is an assignment of the input data bits with a single bit right shift (i.e. input_data/2).[/B][/COLOR]

So all of the data gets loaded in in the first cycle after reset .
This way its achieved in one cycle. Either way I want to use the last bit so I just keep shifting in 0's every clock. Am I going wrong somewhere?

That is not a shift register you are loading the same data every clock cycle.
Look at my shift register examples in post #6.

Also clarify what you mean by "I want to use the last bit", which bit are you trying to use and if you know which bit this is in the parallel input data why bother doing any shifting?

 

[barry]

It doesn't have to be in the sensitivity list as it could just be a synchronous reset (i.e. LOAD signal)

- - - Updated - - -



That is not a shift register you are loading the same data every clock cycle.
Look at my shift register examples in post #6.

Also clarify what you mean by "I want to use the last bit", which bit are you trying to use and if you know which bit this is in the parallel input data why bother doing any shifting?

The way he wrote it, I don't think it was a synchronous reset (full disclosure: I'm not a verilog guy, I could be wrong).

But, yes, he keeps loading the same data every clock.

 

[ads-ee]

The way he wrote it, I don't think it was a synchronous reset (full disclosure: I'm not a verilog guy, I could be wrong).

But, yes, he keeps loading the same data every clock.

Yes, it is a synchronous reset, that is how you write it in Verilog.

Unlike VHDL there isn't an elsif that checks for the clock edge. The sensitivity list triggers the always block so having the posedge reset would result in an asychronous reset behavior as the always block is triggered based on the reset going high regardless of the clock.

There isn't anything inherently wrong with synchronous resets, but it does require that the clock be working before a reset will take effect.

 

[rogger201]

I need to use each bit of the input_data in an incrementing manner in every clock cycle.
So in 1st clock, I want to make use of input_data[0]
in 2nd clock cycle, I want to use input_data[1] ... and so on.
The code I have written seems to work by taking the next bit in each clock. Does it seem wrong for what I want to implement?

 

[rogger201]

It works because in every clock I am appending 1 0 to its MSB and always making use of its LSB

 

[barry]

There's no way this should work. Although I'm not a verilog guy, even I can see that every clock you are reloading shift_reg with 0,input_data(7:1). If input_data doesn't change, neither will shift_reg. Just look at post #6. ADS did your work for you.

 

[ads-ee]

I need to use each bit of the input_data in an incrementing manner in every clock cycle.
So in 1st clock, I want to make use of input_data[0]
in 2nd clock cycle, I want to use input_data[1] ... and so on.
The code I have written seems to work by taking the next bit in each clock. Does it seem wrong for what I want to implement?

Now we have a good description. What you want is a parallel to serial conversion. So the last version of the code in #6 with the direction flipped is what you want.

- - - Updated - - -

BTW the one thing you do have "wrong" with your code is using the blocking assignment (=) instead of a non-blocking assignment (<=).

The basic rule to follow (if you don't want obscure simulation/synthesis issues) is to use non-blocking assignments in any code that is "clocked" and blocking assignments in any code that is purely combinational. There are a number of papers and many discussions you can look up about this if you are interested.

 

[rogger201]

Hello,

Thank you so much for your help. I will keep that in mind. I am new to Verilog so this really helped.
Another small issue I am facing is that I want my "Shifter" to be variable width; as in the number of 0's being appended to shift_reg is variable (parameterized) .

Like

shift_reg = {{bit_width{1'b0}}, input_data[input_width:bit_width]}

this works for non-0 values. When I give bit_width as 0, I get this error
Zero multiple concatenation multiplier, treated as zero width by enclosing concatenation.
It works smoothly with non-0 values but has an issue with 0; clearly because of the generalized way in which it is defined. Is there some way to make it parameterized and yet make it work with 0 width as well?

 

[rogger201]

There's no way this should work. Although I'm not a verilog guy, even I can see that every clock you are reloading shift_reg with 0,input_data(7:1). If input_data doesn't change, neither will shift_reg. Just look at post #6. ADS did your work for you.

It does work because I can see it in my simulation. It just takes the previous appended value and appends more 0's to it. I need to do it this way because I cannot add the 'load' signal. I cant have additional inputs so I just load the data on reset itself.

 

[ads-ee]

Hello,

Thank you so much for your help. I will keep that in mind. I am new to Verilog so this really helped.
Another small issue I am facing is that I want my "Shifter" to be variable width; as in the number of 0's being appended to shift_reg is variable (parameterized) .

Like

shift_reg = {{bit_width{1'b0}}, input_data[input_width:bit_width]}

this works for non-0 values. When I give bit_width as 0, I get this error
Zero multiple concatenation multiplier, treated as zero width by enclosing concatenation.
It works smoothly with non-0 values but has an issue with 0; clearly because of the generalized way in which it is defined. Is there some way to make it parameterized and yet make it work with 0 width as well?

This is NOT shift register code as it written it is purely an assignment with a sliced bit vector. See the code I wrote in #6, where the assignment uses the same signal name on the LHS and the RHS. Do you know what a hardware shift register looks like? The code I provided describes such hardware...i.e. a chain of flip-flops with Q outputs routed into the next FFs D input.

It does work because I can see it in my simulation. It just takes the previous appended value and appends more 0's to it. I need to do it this way because I cannot add the 'load' signal. I cant have additional inputs so I just load the data on reset itself.

If you know when to reset how is that different than a load signal? Reset usually means everything is set to an initial power on state.

The way you wrote the shift register code won't work unless there is other code which you neglected to show that takes shift_reg and feeds it back into the input_data. Unless you show everything to do with the code all the way back to inputs and outputs of the module, we can only comment on the code you provide with your question.

 

[barry]

It does work because I can see it in my simulation. It just takes the previous appended value and appends more 0's to it. I need to do it this way because I cannot add the 'load' signal. I cant have additional inputs so I just load the data on reset itself.

Impossible. This is not a shift register no matter what you think your simulation is showing.