1
\$\begingroup\$

The VHDL design below is supposed to extract the Nth bits from the four values x_0, x_1, x_2 and x_3 and create a new value at every clock but this is not happening.

Find below the design and the generated output for a few forced values for x_0 = 1100, x_1 = 0100, x_2 = 0010 and x_3 = 1011.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
entity gen_input is
    port (x_0,x_1,x_2,x_3 : in std_logic_vector(3 downto 0);
            y : out std_logic_vector(3 downto 0);
            clk : std_logic);
end gen_input;

architecture Behavioral of gen_input is
signal i : integer := 0;    
begin
    inst_process : process (clk,x_3,x_2,x_1,x_0)
                        begin
                        if(clk'event and clk='1') then
                            if(i < 4) then
                            y <= x_3(i)&x_2(i)&x_1(i)&x_0(i);
                            i <= i + 1;
                            else 
                            i <= 0;
                            end if;     
                        end if; 
                        end process inst_process;
end Behavioral;

The output generated: Output for assigned values of x_0, x_1, x_2 and x_3

\$\endgroup\$
  • \$\begingroup\$ (1) parentheses round expressions unnecessary (this isn't c), (2) 25 years ago, the rising_edge function simplified clock edge detection (3) no testbench so we can't try it ourselves (4) there isn't actually a question here and (5) code seems to be doing exactly what I'd expect. \$\endgroup\$ – Brian Drummond Apr 15 '18 at 11:17
  • 1
    \$\begingroup\$ @BrianDrummond, parentheses around expressions aren't required but are optional to an author. Personally, I find they make the test condition distinct and much clearer to the reader in a page of VHDL. Point 2's true. Can you expand point 5 into an explanation or answer. \$\endgroup\$ – TonyM Apr 15 '18 at 12:47
  • \$\begingroup\$ @TonyM They are optional, but just look like clutter to me. Perhaps it's different for younger readers raised on c. As for point 5, any explanation of signal assignment semantics and delta cycles should do; if they aren't completely clear perhaps the questioner needs to ask an actual question. \$\endgroup\$ – Brian Drummond Apr 15 '18 at 13:11
  • \$\begingroup\$ Welcome to the site. Please realise this is not a free design house, homework-answering service or an on-line technical encyclopedia, copied out to you on demand. People will help you take the next step if your question shows you've done as much as you possibly could on your own - which yours doesn't, I'm afraid. Please edit your question and greatly improve it. Ask a specific question, showing your work and findings so far in considerable detail with any schematic. The better the quality of question, the better the quality of the answers you will attract. Again, a warm welcome to the site. \$\endgroup\$ – TonyM Apr 15 '18 at 14:43
  • \$\begingroup\$ Brian has a valid point about the rising_edge function. A transition from 'U' to '1' on clk satisfies the condition clk'event and clk='1 while it would not satisfy rising_edge(clk) which requires a transition from '0' or 'L' to '1' or 'H' (filtering the parameter value and last value through function To_X01). Note the first non-U value on y_gen is the second bit value from X_3 - X_0. Please state a question. \$\endgroup\$ – user8352 Apr 15 '18 at 21:26
1
\$\begingroup\$

and create a new value at every clock but this is not happening

The reason this isn't happening, is because your code has a clock cycle where no output is assigned:

if(clk'event and clk='1') then
    if(i < 4) then
        y <= x_3(i)&x_2(i)&x_1(i)&x_0(i);
        i <= i + 1;
    else 
        i <= 0; -- here you have no assignment to y
    end if;     
end if;

Also, your code would be simpler if you declared i as a variable instead of a signal, which is probably what you mean to do anyway. See this for some discussions on the differences.

This code is probably what you're looking for (as a simulation):

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity gen_input is
end gen_input;

architecture Behavioral of gen_input is

    signal y   : std_logic_vector(3 downto 0);
    signal clk : std_logic := '0';

    constant x_0 : std_logic_vector(3 downto 0) := b"1100";
    constant x_1 : std_logic_vector(3 downto 0) := b"0100";
    constant x_2 : std_logic_vector(3 downto 0) := b"0010";
    constant x_3 : std_logic_vector(3 downto 0) := b"1011";

begin

    clkgen: process
    begin
        wait for 10 ns;
        clk <= not clk;
    end process clkgen;

    inst_process : process (clk)
        variable i : natural range 0 to 3 := 0;
    begin
        if rising_edge(clk) then
            y <= x_3(i) & x_2(i) & x_1(i) & x_0(i);
            i := i + 1;
            if i = 4 then
                i := 0;
            end if;
        end if;
    end process inst_process;
end Behavioral;

Note that, since i is declared as a variable within the process, you can increment i and then immediately check it to see if it should be reset to 0. You could also do this:

if i = 3 then
    i := 0;
else
    i := i + 1;
end if;

but it's more typing and not necessary. Either way, you will end up with the exact same RTL.

Here's the sim:

simulation

\$\endgroup\$
  • \$\begingroup\$ And actually, to argue with myself, I probably don’t need the if-statement at all, since variable i will be limited to just 2 bits (for range 0 to 3), and will just roll over naturally. But I couldn’t get the sim to work without explicitly resetting it. Probably this is just a sim setting in Vivado. \$\endgroup\$ – Blair Fonville Apr 19 '18 at 10:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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