7
\$\begingroup\$

I am trying to make a simple MACC to work, but it does unexpected things. The multiplication is not working. 00001 * 00001 outputs 00000

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.STD_LOGIC_ARITH.ALL;
use IEEE.STD_LOGIC_UNSIGNED.ALL;

entity macc is
    Port ( clk : in  STD_LOGIC;
           rst : in  STD_LOGIC;
           en : in  STD_LOGIC;
           A : in  STD_LOGIC_VECTOR (4 downto 0);
           B : in  STD_LOGIC_VECTOR (4 downto 0);
           P : out  STD_LOGIC_VECTOR (8 downto 0));
end macc;

architecture Behavioral of macc is
    signal product : STD_LOGIC_VECTOR (8 downto 0);
    signal acc_in : STD_LOGIC_VECTOR (8 downto 0);
    signal acc_out : STD_LOGIC_VECTOR (8 downto 0);
begin

    product <= A*B;
    acc_in <= acc_out + product;

        acc: process is
        begin
        wait until rising_edge(clk);
            if (rst = '1') then
                    acc_out <= (others => '0');     
            elsif (en = '1') then
                    acc_out <=  acc_in;
            end if;
        end process acc;

    P <= acc_out;

end Behavioral;

Multiplication mistake

\$\endgroup\$
  • \$\begingroup\$ Waveform added. \$\endgroup\$ – Arturs Vancans Feb 27 '12 at 14:08
  • \$\begingroup\$ I don't remember so much about VHDL, but product should be updated asynchronously right? In that case the problem is not acc, but the actual multiplication... \$\endgroup\$ – clabacchio Feb 27 '12 at 14:15
  • 1
    \$\begingroup\$ Well, that's what this posts is about. \$\endgroup\$ – Arturs Vancans Feb 27 '12 at 14:29
  • \$\begingroup\$ Yep, sorry, I was confused by the context... \$\endgroup\$ – clabacchio Feb 27 '12 at 14:31
11
\$\begingroup\$

If you multiply 2 5-bit numbers (A and B are both std_logic_vector(4 downto 0)) don't you need 10 bits (not 9) to store it in (so P should be std_logic_vector(9 downto 0)? (31*31 = 961: needs 10 bits)

But also - don't use std_logic_arith/_unsigned. Use ieee.numeric_std and then use the unsigned data type.

\$\endgroup\$
  • \$\begingroup\$ I am not allowed to change packages. Moreover I have to meet provided conventions to meet synthaseable code. Regarding the problem, the output vector length was the problem. Thank you. \$\endgroup\$ – Arturs Vancans Feb 28 '12 at 1:09
  • 1
    \$\begingroup\$ Why are you not allowed to change packages? Can you not at least get rid of std_logic_arith (assuming you are doing unsigned maths)? \$\endgroup\$ – Martin Thompson Feb 28 '12 at 10:43
  • 1
    \$\begingroup\$ If you are really synthesizing, then you may want to consider a simple pipelined vendor-specific core instead. \$\endgroup\$ – Aaron D. Marasco Mar 2 '12 at 1:49
  • 4
    \$\begingroup\$ @AaronD.Marasco: or just stick a * in with a few pipeline registers afterwards and let the synth tool figure out where to put them - much more portable. \$\endgroup\$ – Martin Thompson Mar 2 '12 at 13:26

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.