Having trouble with a VHDL output for a 4 bit shift register
I am trying to implement a 4 bit shift register in VHDL. I feel like I have the implementation correct for the actual shifting, but my output isn't working. Every out for this is "0".
I have been playing around with placement, but I am fairly new to VHDL. Any ideas?
I'm almost positive that problem is coming from this section:
signal temp: std_logic_vector(3 downto 0):="0000"; -- initial value of output
begin
process (clock)
begin
O <= temp;
But I'm not sure how to fix it.
library ieee;
use ieee.std_logic_1164.all;
entity shift_reg is
port( I: in std_logic_vector (3 downto 0);
I_SHIFT_IN: in std_logic;
sel: in std_logic_vector(1 downto 0); -- 00:hold; 01: shift left; 10: shift right; 11: load
clock: in std_logic;
enable: in std_logic; -- 0: don't do anything; 1: shift_reg is enabled
O: out std_logic_vector(3 downto 0)
);
end shift_reg;
architecture behav of shift_reg is
signal temp: std_logic_vector(3 downto 0):="0000"; -- initial value of output
begin
process (clock)
begin
O <= temp;
if (enable = '1') then
if (clock='1') then --rising_edge(clock) and
case sel is
when "00" => -- hold
temp <= temp;
O <= temp;
when "01" => -- left shift
temp <= (temp(2 downto 0) & I_SHIFT_IN);
O <= (temp(2 downto 0) & I_SHIFT_IN);
when "10" => -- right shift
temp <= (I_SHIFT_IN & temp(3 downto 1));
O <= (I_SHIFT_IN & temp(3 downto 1));
when "11" => -- load
temp <= I;
O <= I;
when others => --other? exception handling? wouldn't compile without this
temp <= "1111";
O <= "1111";
end case;
end if;
--else null;
end if;
end process;
end behav;
I'm almost positive that problem is coming from this section:
signal temp: std_logic_vector(3 downto 0):="0000"; -- initial value of output begin process (clock) begin O <= temp;But I'm not sure how to fix it.
The problem is a bit more subtle. The O assignment isn't actually a problem. Either temp is the same as what or O will get assigned again later.
There's only one projected output waveform value any simulation time, two assignments will mean the latest one will supplant the earlier.
Also no signal update occurs while any process has yet to resume or not yet suspended. Signal updates with no relative delay (the equivalent of after 0 ns) don't occur during the current simulation cycle, rather at the beginning of the next. Variable assignments occur immediately.
So you want temp to be a variable declared in the process statement. You could also note that with VHDL -2008 you could evaluate O and wouldn't need temp.
There's also a couple of synthesis gotchas in your code. While it will simulate successfully using clock = '1' as an if statement condition detecting the rising edge of clock, the convention used for synthesis is to be explicit. for example you could use rising_edge(clock) defined in package std_logic_1164 which returns a boolean (for condition evaluation) when the input signal has transitioned from '0' to '1' (and not from 'X' to '1', etc.).
There's also the implication that surround the clock edge if statement with an if statement with an enable will cause a gated clock instead of inferring storage using an enable in synthesis. The cure for that is to swap the two if statement conditions around.
Cleaning up all these things and you could get something that looks like:
architecture behav of shift_reg is
-- signal temp: std_logic_vector(3 downto 0):="0000";
begin
process (clock)
variable temp: std_logic_vector(3 downto 0):="0000";
begin
-- O <= temp;
if rising_edge(clock) then
if enable = '1' then -- don't gate clock, enable
case sel is
when "00" => -- hold
temp := temp;
when "01" => -- left shift
temp := (temp(2 downto 0) & I_SHIFT_IN);
when "10" => -- right shift
temp := (I_SHIFT_IN & temp(3 downto 1));
when "11" => -- load
temp := I;
when others =>
-- temp <= "1111"; -- temp <= NULL; -- null statement
end case;
O <= temp;
end if;
end if;
end process;
end architecture behav;
You could note there is no statement under the others choice in the case statement. Each case statement alternative is comprised of a sequence of statements (IEEE Std 1076-2008 10.9 Case statement).
Under 10.1 we see that a sequence of statements can be empty or one or more sequential statements (see 1.3.2 Syntactic description f), items in braces appear zero or more times).
If you want to show explicitly that nothing occurs here you can use a null statement (10.14).
Because assignment to O occurs in every binary value for sel in the case statement we only need one assignment. If it's at the beginning of the process it would cause a half clock delay (until the next clock event).
To demonstrate either the problem or that the changed code works you can use a testbench, providing a Minimal, Complete, and Verifiable example:
library ieee;
use ieee.std_logic_1164.all;
entity shift_reg_tb is
end entity;
architecture foo of shift_reg_tb is
signal I: std_logic_vector (3 downto 0);
signal I_SHIFT_IN: std_logic;
signal sel: std_logic_vector(1 downto 0);
signal clock: std_logic := '0';
signal enable: std_logic;
signal O: std_logic_vector(3 downto 0);
type op is (HOLD, LEFT, RIGHT, LOAD);
signal shftop: op;
use ieee.numeric_std.all;
begin
shftop <= op'val(to_integer(unsigned(sel)) );
DUT:
entity work.shift_reg
port map (
I => I,
I_SHIFT_IN => I_SHIFT_IN,
sel => sel,
clock => clock,
enable => enable,
O => O
);
CLOCK_PROC:
process
begin
wait for 5 ns;
clock <= not clock;
if now > 160 ns then
wait;
end if;
end process;
STIMULI:
process
begin
wait until rising_edge(clock);
I <= x"C";
I_SHIFT_IN <= '0';
enable <= '0';
sel <= "00"; -- HOLD
wait until rising_edge(clock);
enable <= '1';
wait until rising_edge(clock);
sel <= "11"; -- LOAD
wait until rising_edge(clock);
sel <= "10"; -- shift right
wait until rising_edge(clock);
wait until rising_edge(clock);
I_SHIFT_IN <= '1';
wait until rising_edge(clock);
wait until rising_edge(clock);
I_SHIFT_IN <= '0';
sel <= "01"; -- shift left
wait until rising_edge(clock);
wait until rising_edge(clock);
I_SHIFT_IN <= '1';
enable <= '1';
wait until rising_edge(clock);
wait until rising_edge(clock);
wait until rising_edge(clock);
wait until rising_edge(clock);
wait until rising_edge(clock);
wait until rising_edge(clock);
enable <= '0';
wait;
end process;
end architecture;
Note the creation of an enumerated type to define the shift register operation, a signal (shftop) of that type that is assigned values of converted from sel values to make the waveform display easier to interpret:
The good news is the expressions in your case statement alternatives are functional.