2

I'm a newbie and I'm trying to implement in VHDL a shift register for a divisor component. The shift register has to take a 15 bit input and shift it to the right every clock cycle, while chaining a '0' on the most significant bit. I,ve wrote this code


------- SHIFT RIGHT ----------------------------------

> library IEEE; use IEEE.STD_LOGIC_1164.ALL; use ieee.numeric_std.all; > > entity shift_right is port( in_shift_right: in std_logic_vector(14 > downto 0); > clk_shift_right, rst_shift_right : in std_logic; > out_shift_right : out std_logic_vector (14 downto 0) ); end shift_right; > > architecture behavioral of shift_right is begin > process(clk_shift_right, rst_shift_right, in_shift_right ) variable > tmp : std_logic_vector(14 downto 0); begin > if rising_edge (clk_shift_right) then elsif rst_shift_right = '1' then > tmp := (others => '0'); > elsif rising_edge(clk_shift_right) then > tmp:='0'&in_shift_right(14 downto 1); > end if; > out_shift_right<=tmp; end process; end behavioral; 

This is the result on the waveform tested with Quartus' grafic tool:

enter image description here

This in not what i'm looking for, in fact in only works for the first clock cycle. Although i've found the following code online

> library IEEE; > use IEEE.STD_LOGIC_1164.ALL; > > entity shift is > port( > clk:in std_logic; > rst:in std_logic; > sin:in std_logic; > pin:in std_logic_vector(7 downto 0); > mode:in std_logic_vector(1 downto 0); > sout:out std_logic; > pout:out std_logic_vector(7 downto 0) > ); > end shift; > > > architecture Behavioral of shift is > signal temp:std_logic_vector(7 downto 0); > > > begin > process(rst,clk,sin,mode,pin) > > begin > > if rst='1' then > sout<='0'; > pout<="00000000"; > temp<="00000000"; > > > elsif(clk'event and clk='1')then > > case mode is > --SISO > when "00"=> > temp(6 downto 0)<= temp(7 downto 1); > temp(7)<=sin; > sout<=temp(0); > > --SIPO > when"01"=> > temp(6 downto 0)<= temp(7 downto 1); > temp(7)<=sin; > pout<=temp; > > --PIPO > when"10"=> > temp<=pin; > pout<=temp; > > > when others=> > null; > > end case; > > end if; > > end process; > > end Behavioral; 

IT actually works as i want, but, as you can see from the waveform, it only works when the mode chosen on "case" it's '01', but only if this comes after the '10' mode. I also can't understand why it works only after a clock cycle delay, that I highlighted in the picture:

https://ibb.co/m4W738v

Really thank to anyone could help me

1
  • 2
    Your code is very messy. I think if you just tidy up the indentation you will start to see some problems immediately. Commented Dec 30, 2019 at 8:50

1 Answer 1

1

If you tidy up the indentation of your code, I think you will start to see some problems immediately. For example, think about what this code will do:

if rising_edge (clk_shift_right) then elsif rst_shift_right = '1' then tmp := (others => '0'); elsif rising_edge(clk_shift_right) then ... 

The contents of the final elsif can never be reached.

A second comment is that you have put rst and in_shift_right in the sensitivity list. Do you want a synchronous or asynchronous reset? If synchronous, then rst doesn't need to be in the sensitivity list. Either way, I don't see any reason to put in_shift_right in the sensitivity list.

A third comment is that your use of a variable (tmp) is potentially worrying. Your usage is perfectly valid and syntactically correct, but seems a very strange choice in this context. If this is a deliberate stylistic choice, then that's fine. However, if you just don't understand the differences between a signal and a variable, then I recommend devoting a little time to that. Remember, this is not software; you are describing hardware.

A fourth comment is that your assignment tmp:='0'&in_shift_right(14 downto 1); always uses the upper 14 bits of the input in_shift_right. This is clearly not what you want to do (you want to keep shifting right). Try to think about what kind of hardware would be needed to do this.

You already include the ieee.numeric_std library, so there is actually a shift_right function you can use directly. You don't describe exactly what you want your block to do, but I guess it is something like this (assuming asynchronous reset):

library ieee; use ieee.std_logic_1164.all; use ieee.numeric_std.all; entity right_shifter is generic ( DATA_BITS : positive := 15 ); port ( clk : in std_logic; rst : in std_logic; -- Input in_valid : in std_logic; in_data : in std_logic_vector(DATA_BITS-1 downto 0); -- Control shift_enable : in std_logic; -- Output out_data : out std_logic_vector(DATA_BITS-1 downto 0) ); end right_shifter; architecture rtl of right_shifter is signal data : unsigned(DATA_BITS-1 downto 0); begin process(clk, rst) begin if rst = '1' then -- Reset data <= (others => '0'); elsif rising_edge(clk) then if in_valid = '1' then -- Load new input data <= unsigned(in_data); elsif shift_enable = '1' then -- Shift right data <= shift_right(data, 1); end if; end if; end process; out_data <= std_logic_vector(data); end rtl; 

Or, with a synchronous reset:

library ieee; use ieee.std_logic_1164.all; use ieee.numeric_std.all; entity right_shifter is generic ( DATA_BITS : positive := 15 ); port ( clk : in std_logic; rst : in std_logic; -- Input in_valid : in std_logic; in_data : in std_logic_vector(DATA_BITS-1 downto 0); -- Control shift_enable : in std_logic; -- Output out_data : out std_logic_vector(DATA_BITS-1 downto 0) ); end right_shifter; architecture rtl of right_shifter is signal data : unsigned(DATA_BITS-1 downto 0); begin process(clk) begin if rising_edge(clk) then if rst = '1' then -- Reset data <= (others => '0'); elsif in_valid = '1' then -- Load new input data <= unsigned(in_data); elsif shift_enable = '1' then -- Shift right data <= shift_right(data, 1); end if; end if; end process; out_data <= std_logic_vector(data); end rtl; 
Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.