cancel
Showing results for 
Show  only  | Search instead for 
Did you mean: 
adrialex
Visitor
Visitor
3,950 Views
Registered: ‎08-05-2009

multiplier code problem

My code below has an undefined output. Could anyone please tell me what the problem is. Also, I would like to ask for advices for my code to be synthesizable in xilinx when you post-simulate it.Thank You.

 

 

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

entity multiplier is
    Port ( clk : in  STD_LOGIC;
           rst : in  STD_LOGIC;
           en : in  STD_LOGIC;
           opA : in  STD_LOGIC_VECTOR (3 downto 0);
           opB : in  STD_LOGIC_VECTOR (3 downto 0);
           res : out  STD_LOGIC_VECTOR (7 downto 0);
           done : out  STD_LOGIC);
end multiplier;

architecture Behavioral of multiplier is

    SIGNAL tempA,tempB,number : STD_LOGIC_VECTOR(3 downto 0);
    SIGNAL answer : STD_LOGIC_VECTOR(7 downto 0);
   
    COMPONENT cla_adder
    PORT ( opA : IN  std_logic_vector (3 downto 0);
           opB : IN  std_logic_vector (3 downto 0);
           carry_in : IN  STD_LOGIC;
           sum : OUT  std_logic_vector (3 downto 0);
           carry_out : OUT  STD_LOGIC
            );
    END COMPONENT;
   
    COMPONENT shifter
     PORT ( rst : IN std_logic;
             op1 : IN std_logic_vector(3 downto 0);
              op2 : IN std_logic_vector(3 downto 0);
              din : IN std_logic;         
              out1 : OUT std_logic_vector(3 downto 0);
              out2 : OUT std_logic_vector(3 downto 0)
            );
    END COMPONENT;
   
    --Inputs
    SIGNAL op_add1 :  std_logic_vector(3 downto 0);
    SIGNAL op_add2 :  std_logic_vector(3 downto 0);
   
    --Outputs
    SIGNAL sum_add :  std_logic_vector(3 downto 0);
    SIGNAL carry_out_add :  std_logic;
    SIGNAL carry_out2 :  std_logic;
    SIGNAL out_shift1 :  std_logic_vector(3 downto 0);
   SIGNAL out_shift2 :  std_logic_vector(3 downto 0);
 
   
begin
    adding: cla_adder PORT MAP(
        opA => op_add1,
        opB => op_add2,
        carry_in => '0',
        sum => sum_add,
        carry_out => carry_out_add
    );
   
    shifting: shifter PORT MAP(
        rst => rst,
        op1 => sum_add,
        op2 => tempB,
        out1 => out_shift1,
        out2 => out_shift2,
        din => carry_out_add
    );
   

process (rst,clk,en,tempB,number,out_shift1,out_shift2,op_add1,op_add2,carry_out_add)
begin
    if clk'event and clk = '1' then
        if rst = '1' then
            res <= "00000000";
            op_add1 <= "0000";
            op_add2 <= "0000";
            number <= "0000";
            done <= '0';
       
        elsif en = '1' then
        tempA <= opA;
        tempB <= opB;
         
        elsif tempB(0) = '1' and number = "0000" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
            number <= "0001";
            elsif tempB(0) = '1' and number = "0001" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
            number <= "0010";
            elsif tempB(0) = '1' and number = "0010" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
            number <= "0100";
            elsif tempB(0) = '1' and number = "0100" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
                number <= "1000";
            elsif tempB(0) = '1' and number = "1000" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                answer <= sum_add & tempB;
                done <= '1';
            res <= answer;
                number <= "1111";
               
            elsif tempB(0) = '0' and number = "0000" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
            number <= "0001";
            elsif tempB(0) = '0' and number = "0001" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
            number <= "0001";
            elsif tempB(0) = '0' and number = "0010" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
            number <= "0100";
            elsif tempB(0) = '0' and number = "0100" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                res <= sum_add & tempB;
            number <= "1000";
            elsif tempB(0) = '0' and number = "1000" then
            op_add2 <= tempA;
            op_add1 <= out_shift1;
            tempB <= out_shift2;
                answer <= sum_add & tempB;
                done <= '1';
            res <= answer;
                number <= "1111";
            else
                res <= answer;
        end if;
    end if;  
end process;

end Behavioral;

 

This code below is my carry look ahead adder

 

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

---- Uncomment the following library declaration if instantiating
---- any Xilinx primitives in this code.
--library UNISIM;
--use UNISIM.VComponents.all;

entity cla_adder is
    Port ( opA : in  STD_LOGIC_VECTOR (3 downto 0);
           opB : in  STD_LOGIC_VECTOR (3 downto 0);
           carry_in : in  STD_LOGIC;
           sum : out  STD_LOGIC_VECTOR (3 downto 0);
           carry_out : out  STD_LOGIC);
end cla_adder;

architecture Behavioral of cla_adder is
    SIGNAL s0,s1,s2,s3 : STD_LOGIC;
    SIGNAL g0,g1,g2,g3 : STD_LOGIC;
    SIGNAL p0,p1,p2,p3 : STD_LOGIC;
    SIGNAL c0,c1,c2,c3 : STD_LOGIC;
begin
    s0 <= opA(0) XOR opB(0) XOR carry_in;
    g0 <= opA(0) AND opB(0);
    p0 <= opA(0) OR opB(0);
    c0 <= g0 OR (p0 AND carry_in);
   
    s1 <= opA(1) XOR opB(1) XOR c0;
    g1 <= opA(1) AND opB(1);
    p1 <= opA(1) OR opB(1);
    c1 <= g1 OR (p1 AND c0);
   
    s2 <= opA(2) XOR opB(2) XOR c1;
    g2 <= opA(2) AND opB(2);
    p2 <= opA(2) OR opB(2);
    c2 <= g2 OR (p2 AND c1);
   
    s3 <= opA(3) XOR opB(3) XOR c2;
    g3 <= opA(3) AND opB(3);
    p3 <= opA(3) OR opB(3);
    c3 <= g3 OR (p3 AND c2);
   
    sum <= s3 & s2 & s1 & s0;
    carry_out <= c3;
   
end Behavioral;

----this code below is my shifter

 

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

---- Uncomment the following library declaration if instantiating
---- any Xilinx primitives in this code.
--library UNISIM;
--use UNISIM.VComponents.all;

entity shifter is
    Port ( rst : in  STD_LOGIC;
           op1 : in  STD_LOGIC_VECTOR (3 downto 0);
           op2 : in  STD_LOGIC_VECTOR (3 downto 0);
           out1 : out  STD_LOGIC_VECTOR (3 downto 0);
           out2 : out  STD_LOGIC_VECTOR (3 downto 0);
           din : in  STD_LOGIC);
end shifter;

architecture Behavioral of shifter is
    signal temp1 : STD_LOGIC_VECTOR (3 downto 0);
    signal temp2 : STD_LOGIC_VECTOR (3 downto 0);
begin
    process(din,rst)
    begin
        if din = '0' then
            temp1 <= op1;
            temp2 <= op2;
            temp2(0) <= op2(1); temp2(1) <= op2(2); temp2(2) <= op2(3); temp2(3) <= op1(0); 
            temp1(0) <= op1(1); temp1(1) <= op1(2); temp1(2) <= op1(3); temp1(3) <= '0';
            out2 <= temp2;
            out1 <= temp1;
        else
            temp2(0) <= op2(1); temp2(1) <= op2(2); temp2(2) <= op2(3); temp2(3) <= op1(0); 
            temp1(0) <= op1(1); temp1(1) <= op1(2); temp1(2) <= op1(3); temp1(3) <= '1';
            out2 <= temp2;
            out1 <= temp1;
        end if;
       
        if rst = '1' then
        out2 <= "0000";
        out1 <= "0000";
        end if;
    end process;
   
end Behavioral;

 

0 Kudos
3 Replies
eilert
Teacher
Teacher
3,927 Views
Registered: ‎08-14-2007

Hi Adrialex,

to find out if your code is synthesizable, you can either give it a try with ise or if you simulate with modelsim  you can use the -check_synthesis option with vcom on your models.

 

Some question about your code: Why are you doing it so complicated?

 

e.g. the shifter:

If you really want to fix the outputs to "0000" using the res input, ok. But why? Its just a combinatorical circuit.

And  why are you always manipulating each bit separately and using so much temp signals?

 

 

useless assignment:            temp1 <= op1;
useless assignment:            temp2 <= op2;

because you are overwriting the temp driver in the next assignments:

            temp2(0) <= op2(1); temp2(1) <= op2(2); temp2(2) <= op2(3); temp2(3) <= op1(0); 
            temp1(0) <= op1(1); temp1(1) <= op1(2); temp1(2) <= op1(3); temp1(3) <= '0';
            out2 <= temp2;
            out1 <= temp1;

 

these two lines replace the whole first if construct:

out2 <= op1(0) & op2(3 downto 1);

out1 <=       din & op1(3 downto 1);

 

 Also your sensitivity list should cover all inputs, when it is a combinatorical circuit. 

 This means op1 and op2 as well as the temp signals in your example.

 

Something similar is valid for your adder.

CLA is a nice architecture for fast ASICs, but for FPGAs it renders almost useless.

Especially when your wordwidth is so small.

The Xilinx CLBs have fast carry chains.

These are much faster than your CLA logic that would use extra LUTs and routing, causing unneccessary delay.

 

 So for your adder you can use this simple construct:

 carryandsum := '0'&opA + '0'&opB + carry_in;

 sum <=  carryandsum(3 downto 0)

 carry_out <= carryandsum(4);

 

 

For your Multiplier: 

In your controlling process (I don't dare to call it a FSM) you need only the clk signal in your sensitivity list.

The reset is synchronous, and so no other signal change would trigger the first 'if' and you are just wasting simulation time.

 

 Have a nice synthesis

  Eilert

0 Kudos
adrialex
Visitor
Visitor
3,908 Views
Registered: ‎08-05-2009

I edited my multiplier code and had this code. I still have an undefined output. Is it because of my timing on assigning values or my components?

I didn't edit my components because I would change a great deal of code in my other programs including them. Is my code sound? Please tell me the problem. Thank You.

 

 

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

entity multiplier is
    Port ( clk : in  STD_LOGIC;
           rst : in  STD_LOGIC;
           en : in  STD_LOGIC;
           opA : in  STD_LOGIC_VECTOR (3 downto 0);
           opB : in  STD_LOGIC_VECTOR (3 downto 0);
           res : out  STD_LOGIC_VECTOR (7 downto 0);
           done : out  STD_LOGIC);
end multiplier;

architecture Behavioral of multiplier is

    SIGNAL tempA,tempB,tempC : STD_LOGIC_VECTOR(3 downto 0);
    SIGNAL cstate,nstate : STD_LOGIC_VECTOR(2 downto 0);
    SIGNAL answer : STD_LOGIC_VECTOR(7 downto 0);
   
    COMPONENT cla_adder
    PORT ( opA : IN  std_logic_vector (3 downto 0);
           opB : IN  std_logic_vector (3 downto 0);
           carry_in : IN  STD_LOGIC;
           sum : OUT  std_logic_vector (3 downto 0);
           carry_out : OUT  STD_LOGIC
            );
    END COMPONENT;
   
    COMPONENT shifter
     PORT ( rst : IN std_logic;
             op1 : IN std_logic_vector(3 downto 0);
              op2 : IN std_logic_vector(3 downto 0);
              din : IN std_logic;         
              out1 : OUT std_logic_vector(3 downto 0);
              out2 : OUT std_logic_vector(3 downto 0)
            );
    END COMPONENT;
   
    --Inputs
    SIGNAL op_add1 :  std_logic_vector(3 downto 0);
    SIGNAL op_add2 :  std_logic_vector(3 downto 0);
   
    --Outputs
    SIGNAL sum_add :  std_logic_vector(3 downto 0);
    SIGNAL carry_out_add :  std_logic;
    SIGNAL carry_out2 :  std_logic;
    SIGNAL out_shift1 :  std_logic_vector(3 downto 0);
   SIGNAL out_shift2 :  std_logic_vector(3 downto 0);
 
   
begin
    adding: cla_adder PORT MAP(
        opA => op_add1,
        opB => op_add2,
        carry_in => '0',
        sum => sum_add,
        carry_out => carry_out_add
    );
   
    shifting: shifter PORT MAP(
        rst => rst,
        op1 => sum_add,
        op2 => tempB,
        out1 => out_shift1,
        out2 => out_shift2,
        din => carry_out_add
    );
   
    changes : process(rst,clk,en)
    begin
        if rst = '1' then
            cstate <= "000";
            res <= "00000000";
            done <= '0';
            op_add1 <= "0000";
            op_add2 <= "0000";
        elsif en = '1' then
            tempA <= opA;
            tempB <= opB;
        else
            if clk'event and clk = '1' then
                cstate <= nstate;
            end if;
        end if;
    end process;

    instances : process(cstate,tempA,tempB,tempC,out_shift1,out_shift2)
    begin
        case cstate is
            when "000" =>
                if tempB(0) = '1' then
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "001";
                    res <= tempC & tempB;
                else
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "001";
                    res <= tempC & tempB;
                end if;
            when "001" =>
                if tempB(0) = '1' then
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "010";
                    res <= tempC & tempB;
                else
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "010";
                    res <= tempC & tempB;
                end if;
            when "010" =>
                if tempB(0) = '1' then
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "011";
                    res <= tempC & tempB;
                else
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "011";
                    res <= tempC & tempB;
                end if;
            when "011" =>
                if tempB(0) = '1' then
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "100";
                    res <= tempC & tempB;
                else
                    op_add2 <= tempA;
                    tempB <= out_shift2;
                    tempC <= out_shift1;
                    nstate <= "100";
                    res <= tempC & tempB;
                end if;
            when others =>
                res <= tempC & tempB;
            end case;
    end process;

end Behavioral;

0 Kudos
eilert
Teacher
Teacher
3,881 Views
Registered: ‎08-14-2007

Hi adrialex,

The FSM-Part looks much better now.

I just wonder why you are doing the same things for  tempB(0) = '1'and the else branch?

When you enter State 100 the case goes into the 'others' branch and stops there, so your multiplyer is only working once (you also didn't use the done signal).

 

If you are using EN as a enable for the multiplyer, it would block the sequential operation. (The clk'event branch won't run until EN becomes inactive. Unlogical.) 

 

You are assigning to signal res in two processes. Multiple driver problem. Won't synthesize!

Same for op_add2. op_add1 is always "0000".

 

The only clocked assignment in your state-register process is cstate, so it's useless (if not dangerous) to reset/preset other signals there. 

 Your cla-adder and shifter are combinatorical circuits and the case-process of your FSM too. So where will the intermediate results be stored?

No registers. You won't be happy if the tool creates latches or combinatorical loops.

 

So, your code looks nicer now, but is (sorry to say so) quite crappy.

Rethink your architecture. RTL Design means: You need registers somewhere in your datapath.

 

What's happening with the carry out signal of the adder? You are feeding it into your shifter as Din?

What is the lower part of your Shifter good for? (won't call it shift register, cause it has no clock)

You are always overwriting it with the adder result. There's no feedback of the shifters lower part.

 

What is your algorithm anyway? You are always adding and shifting. This seems wrong to me.

You should draw yourself a sketch of how the signals are flowing and what operations have to be done on each clock cycle.

 

Have a nice synthesis

  Eilert

 

 

 

0 Kudos