0
\$\begingroup\$

I want to make a simple processing element for calculating the absolute difference between two 8-bit words. However, this element may be used as part of an array in order to speed up the AD calculation, by making it in parallel on more data. So, to achieve that, except from the two 8-bit data inputs and the one 8-bit data output, it should have another two 8-bit data outputs (let's name them SHIFT_A and SHIFT_B) that will hold the value of the previous inputs (like a shift register).

I tried to make an implementation in VHDL, but it's not working because I can't figure out how to make the two SHIFT outputs having different values than the two data inputs. I've tried many things without success so, I'm giving here my source code:

LIBRARY ieee;
USE ieee.std_logic_1164.ALL;
--use ieee.std_logic_arith.ALL;
use IEEE.NUMERIC_STD.ALL;

entity pe_element is
  port(
    --Output signals
    AD_o : out std_logic_vector(7 downto 0);
    SHIFT_CB_o : out std_logic_vector(7 downto 0);
    SHIFT_RB_o : out std_logic_vector(7 downto 0);
    --Input signals
    cb_en_i : in std_logic;
    rb_en_i : in std_logic;
    diff_en_i: in std_logic;
    clk_i : in std_logic;
    rst_i : in std_logic;
    CB_i : in std_logic_vector(7 downto 0);
    RB_i : in std_logic_vector(7 downto 0) 
  );
end pe_element;

architecture pe_element of pe_element is

--Define FSM states
type statetype is (S0, S1, S2, S3);
signal state, nextstate: statetype;

--Signal definitions
signal adder_tmp: std_logic_vector(8 downto 0);

-- That particular hack is a pretty annoying one. Idiots. 
signal am: integer;
signal bm: integer;
signal CBreg: std_logic_vector(7 downto 0);
signal RBreg: std_logic_vector(7 downto 0);

begin

-- Current State Register

process(clk_i)
  begin
    if clk_i'event and clk_i = '1' then
      if rst_i = '1' then
        state <= S0;
      else 
        state <= nextstate;
      end if;
    end if;
end process;

-- Next State Logic

process(state, diff_en_i, cb_en_i, rb_en_i) begin
  case state is
    when S0 => if diff_en_i = '0' and cb_en_i = '1' then nextstate <= S1;
               else nextstate <= S0;
               end if;
    when S1 => if diff_en_i = '0' and rb_en_i = '1' then nextstate <= S2;
               else nextstate <= S1;
               end if;
    when S2 => if diff_en_i = '1' and rb_en_i = '0' and cb_en_i = '0' then nextstate <= S3;
               else nextstate <= S2;
               end if;
    when S3 => if diff_en_i = '1' then nextstate <= S3;
               else nextstate <= S1;
               end if;
    when others => nextstate <= S0;
  end case;
end process;

-- Output Logic
am <= to_integer(signed(CB_i));
bm <= to_integer(signed(RB_i));


CBreg <= CB_i when
        ((state = S1) and cb_en_i='1');

SHIFT_CB_o <= CBreg when
           ((state = S1) and cb_en_i = '1');

RBreg <= RB_i when
        ((state = S2) and rb_en_i='1');

SHIFT_RB_o <= RBreg when
           ((state = S2) and rb_en_i = '1');

adder_tmp <= std_logic_vector(to_signed(abs((am - bm)),9)) when
           ((state = S3) and rb_en_i = '0' and cb_en_i = '0' and diff_en_i = '1')
           else "000000000";

AD_o <= adder_tmp(7 downto 0) when
           ((state = S3) and rb_en_i = '0' and cb_en_i = '0' and diff_en_i = '1')
           else "00000000";

end pe_element; 

As you can see in the following simulation image, the SHIFT output holds only the new value each time, and not the previous one.

enter image description here

I'm pretty sure I've coded something wrong, but I can't find it. Do you have any ideas?

\$\endgroup\$
  • \$\begingroup\$ IMO, you are trying to do too much with your state machines. Seperate your state and shift registers into their own blocks. Give the shift registers a load signal and run those from your FSTM's \$\endgroup\$ – Voltage Spike Apr 18 '16 at 23:06
2
\$\begingroup\$

It appears that you are trying to create latches in the last section by using when in your assignment statements. This is not recommended design practice. Among other things, this allows the data to "flow through" as soon as the conditions are met, which is what the simulator is showing you.

If you want to create pipeline registers (edge-triggered, master-slave), you need to do those assignments within a clocked process.


EDIT: I'm really not following all of the complexity in your code. If I had to do what I think you're trying to do, I would code it more like this:

entity pe_element is
  port(
    --Output signals
    AD_o        : out std_logic_vector (7 downto 0);
    SHIFT_CB_o  : out std_logic_vector (7 downto 0);
    SHIFT_RB_o  : out std_logic_vector (7 downto 0);
    --Input signals
    cb_en_i     : in  std_logic;
    rb_en_i     : in  std_logic;
    diff_en_i   : in  std_logic;
    clk_i       : in  std_logic;
    rst_i       : in  std_logic;
    CB_i        : in  std_logic_vector (7 downto 0);
    RB_i        : in  std_logic_vector (7 downto 0) 
  );
end pe_element;

architecture pe_element of pe_element is

  --Signal definitions
  signal adder_tmp      : std_logic_vector (8 downto 0);

  -- That particular hack is a pretty annoying one. Idiots. 
  signal am             : integer;
  signal bm             : integer;

begin

  process (clk_i)
  begin
    if rising_edge(clk_i) then

      -- update CB pipeline
      if cb_en_i = '1' then
        SHIFT_CB_o <= CB_i;
      end if;

      -- update RB pipeline
      if rb_en_i = '1' then
        SHIFT_RB_o <= RB_i;
      end if;

      -- update output
      if diff_en_i = '1' then
        AD_o <= adder_tmp (7 downto 0);
      end if;

    end if;
  end process;

  -- Output Logic
  am <= to_integer(signed(CB_i));
  bm <= to_integer(signed(RB_i));
  adder_tmp <= std_logic_vector(to_signed(abs((am - bm)),9));

end pe_element; 
\$\endgroup\$
  • \$\begingroup\$ That's right, it's sort of a pipeline register. In that FSM model I used, is it "allowed" to use a clocked process in the output logic? How would you advice me to change my code in order to follow some good design practices? \$\endgroup\$ – Arkoudinos Apr 18 '16 at 22:54
  • \$\begingroup\$ I'm really not sure what you're trying to accomplish with all of that complexity. Why does a magnitude comparator need a state machine at all? But technically, the pipeline registers are not part of the state machine at all, although they are controlled by it. \$\endgroup\$ – Dave Tweed Apr 18 '16 at 23:12
  • \$\begingroup\$ Thank you very much for your time. I will test it and see what happens. \$\endgroup\$ – Arkoudinos Apr 18 '16 at 23:28

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.