Help for VHDL code, wrong answer

[Adnan86]

Hi
i want to implement , multiplication of 2 matrix. A[MxN]*B[Nxk]=C[MxK] .
I want this for implementation on FPGA with xilinx , before all i have no error with the code (on MODELSIM) .
but my output, get wrong answer , i don't where is my code wrong , so need help , plzzz
here my code :

LIBRARY IEEE;
--USE ieee.std_logic_1164.all;
USE ieee.numeric_std.all;
PACKAGE matrix_pkg IS
type matrix_08 is array (integer range <>, integer range <>) of signed( 7 downto 0);
type matrix_16 is array (integer range <>, integer range <>) of signed(15 downto 0);
type matrix_32 is array (integer range <>, integer range <>) of signed(31 downto 0);
type matrix_64 is array (integer range <>, integer range <>) of signed(63 downto 0);
END PACKAGE ;
    
LIBRARY IEEE;
USE work.matrix_pkg.all ;
USE ieee.std_logic_1164.all;
USE ieee.numeric_std.all;

ENTITY MAC_matrix IS
   
   GENERIC (M : integer:=3;
            K : integer:=1;
            N : integer:=2);
   
   PORT (
      a: IN matrix_16(1 to M, 1 to N); -- MxN matrix ;
      b: IN matrix_16(1 to N, 1 to K); -- NxK matrix ;
      clk: IN STD_LOGIC;
      reset: IN STD_LOGIC;
      --flag : IN std_logic := '0' ;
      c: OUT  matrix_32(1 to M, 1 to K)
   ) ;
END MAC_matrix;
ARCHITECTURE rtl OF MAC_matrix IS

   SIGNAL x_a:  matrix_16(1 to M, 1 to N);
   SIGNAL x_b:  matrix_16(1 to N, 1 to K);
   SIGNAL x_c, x_reg:  matrix_32(1 to M, 1 to K);
   SIGNAL flag : std_logic := '0' ;
   SIGNAL ii,jj,kk : integer := 1 ;
   SIGNAL sum : signed(31 downto 0) ;
   
BEGIN
   PROCESS (clk)
   BEGIN
      IF (clk'event and clk = '1') THEN
          
         IF (reset = '1' and flag = '0') then
          x_a <= (others=>(others => x"0000"));
          x_b <= (others=>(others => x"0000"));
          x_reg <= (others=>(others => x"00000000"));
          x_c <= (others=>(others => x"00000000"));
          sum <=  x"00000000";
         ELSIF (reset = '0' and flag = '0') THEN
          x_a <= a;
          x_b <= b;
          flag <= '1' ;
 --        END IF;
         
--     END IF ;
--  END process ;
  
--  PROCESS (clk)
--  BEGIN
--      IF (clk'event and clk = '1') THEN
          
         ELSIF (reset = '0' and flag = '1') then
             IF (ii <= M and jj <= N and kk <= K) then
                x_reg(ii,kk) <= x_a(ii,jj) * x_b(jj,kk);
                sum <= sum + x_reg(ii,kk);
                jj <= jj + 1 ;
                IF (jj > N) THEN
                x_c(ii,kk) <= sum ;
                END IF ;
             ELSIF (ii <= M and jj > N and kk <= K) then
                --ii <= ii + 1;
                kk <= kk + 1 ;
                jj <= 1 ;
                sum <= x"00000000" ;
             ELSIF (ii <= M and jj > N and kk > K) then
                ii <= ii + 1;
                kk <= 1 ;
                jj <= 1 ;
                sum <= x"00000000" ;
             ELSE
                c <= x_c ;
             END IF ;
         END IF ;
         
      END IF;
      
   END process;
   c <= x_c ;
END rtl;

and here my testbanch

LIBRARY  IEEE;
USE IEEE.std_logic_1164.all;
USE IEEE.numeric_std.all ;
USE work.matrix_pkg.all ;

ENTITY Mac_matrix_tb IS
END  Mac_matrix_tb;

ARCHITECTURE test_mac_behav OF  Mac_matrix_tb IS
 
 constant M : integer := 3;
 constant K : integer := 1;
 constant N : integer := 2;
 COMPONENT MAC_matrix IS 
  --   GENERIC (M : integer:=M;
  --            K : integer:=K;
  --            N : integer:=N);
   
   PORT (
      a: IN matrix_16(1 to M, 1 to N); -- MxN matrix ;
      b: IN matrix_16(1 to N, 1 to K); -- NxK matrix ;
      clk: IN STD_LOGIC;
      reset: IN STD_LOGIC;
     -- ready: IN STD_LOGIC;
      c: OUT  matrix_32(1 to M, 1 to K)
   ) ;
 END COMPONENT;
 
SIGNAL a:  matrix_16(1 to M, 1 to N); -- MxN matrix ;
SIGNAL b:  matrix_16(1 to N, 1 to K); -- NxK matrix ;
SIGNAL clk   : std_logic := '0' ;
--SIGNAL ready   : std_logic := '0' ;
SIGNAL reset:  STD_LOGIC := '1';
SIGNAL c:   matrix_32(1 to M, 1 to K);
       
BEGIN
	UUT : MAC_matrix
	--generic map(M=>M,
	--            N=>N,
	--            K=>K);
	PORT MAP (
		 a => a,
		 b => b,
		 clk=>clk,
		 reset => reset,
		-- ready => ready,
		 c => c
		  );
   clk <= NOT clk AFTER 10 NS ;
   reset <= '0' AFTER 15 NS ;
--END ;

PROCESS --(clk)
BEGIN
WAIT FOR 20 NS;
a(1,1) <=x"0001";
a(1,2) <=x"0001";

a(2,1) <=x"0010";
a(2,2) <=x"0011";

a(3,1) <=x"0010";
a(3,2) <=x"0001";

b(1,1) <=x"0010";
b(2,1) <=x"0001";

WAIT FOR 10000 NS;
a(1,1) <=x"0001";
a(1,2) <=x"0001";

a(2,1) <=x"0010";
a(2,2) <=x"0011";

a(3,1) <=x"0010";
a(3,2) <=x"0001";

b(1,1) <=x"0010";
b(2,1) <=x"0001";


WAIT;

END PROCESS;
END ;

output always wrong , i don't know why ?
I'll be so appreciate for help me .
thanks

 

[ads-ee]

Hi
i want to implement , multiplication of 2 matrix. A[MxN]*B[Nxk]=C[MxK] .
I want this for implementation on FPGA with xilinx , before all i have no error with the code (on MODELSIM) .
but my output, get wrong answer , i don't where is my code wrong , so need help , plzzz
here my code :

output always wrong , i don't know why ?
I'll be so appreciate for help me .
thanks

What do you mean by no error with the code on Modelsim? Do you mean it compiles clean?

If you are getting the wrong answer then put all the signals in the UUT up in a modelsim wave window and start debugging where the calculations go wrong. You probably have some issue with the indexing i.e. ii, jj, and kk. Seeing that you are doing some of the checks with > and some with <= and you initialize the indexes with 1.

Your coding of flag as part of the the reset is really weird. I would rethink that logic and generate the flag outside the matrix multiplication and use that version of flag as an enable in this process. You should also switch to using if rising_edge (clk) then. One of the good reasons for it is it's more robust for simulating and doesn't have issues with going from X or U to '1'.

Regards

 

[Adnan86]

yes compile clean ;
flag just a name , for start of main part of program ;
i used( if rising_edge (clk) then ) too but nothing change ;
for some days i changed every indexing i.e. ii, jj, and kk and > and some with <= and , but still got nothing.
i really got confused .
whatever Thanks for help and Thanks for your time that gave me to look my code .

 

[ads-ee]

I told you to add the entire UUT to the Modelsim wave window (add wave *) and debug the code, by following the calculation through the simulation. I'm certainly not going to do it for you. Arbitrarily changing the index compares isn't going to fix the problem you need to know where in the calculation pipeline you are and when the indexes need to be updated. Perhaps you need to draw up a pipeline diagram of the calculation.

Debugging is a skill you'll need to develop, now is a good a time as any to start.


Regards

 

[Adnan86]

I did that (add the entire UUT to the Modelsim wave window) and my code just work for one time , it means just for a(1,1)*b(1,1) , that give me answer 16), ( in this case,accordin my testbanch ), after that all answer will be 0 , and at the end all matrix of C are zero .
i checked all wave

 

[ads-ee]

You need to trace through the waveform and see why you aren't getting the next value.

You do understand the calculation you have is pipelined and won't run as if it was a software program? If you were expecting everything to be calculated in a single clock cycle, that isn't what's going to happen.

- - - Updated - - -

I just compiled it and ran your testbench, you've got issues with the indexing as I suspected. For one thing the jj never increments after the first pass because kk increments past K after the first pass through.

 

[Adnan86]

no i checked it , for example in this case K = 1 ; after the first pass of jj ; kk will be increment, but after that because of kk > K , this 2 if statement will be passed :

IF (ii <= M and jj <= N and kk <= K) then
                x_reg(ii,kk) <= x_a(ii,jj) * x_b(jj,kk);
                sum <= sum + x_reg(ii,kk);
                jj <= jj + 1 ;
                IF (jj > N) THEN
                x_c(ii,kk) <= sum ;
                END IF ;
             ELSIF (ii <= M and jj > N and kk < K) then
                --ii <= ii + 1;
                kk <= kk + 1 ;
                jj <= 1 ;
                sum <= x"00000000" ;

till kk <= 1 again , as this part of code will be processed :

ELSIF (ii <= M and jj > N and kk > K) then
                ii <= ii + 1;
                kk <= 1 ;
                jj <= 1 ;
                sum <= x"00000000" ;

Am i wright ?

 

[ads-ee]

I think logically this is what you want to do for the indexing (though you should carefully check that it's correctly going through each step).

Given the matrices:
MxN = 3x2
NxK = 2x1

initialize nn = 1, mm = 1, and kk = 1
increment nn until nn == N then rollover to 1
increment kk when nn == N and kk < K
increment mm when nn == N and kk == K and stop calculation when mm == M and nn == N and kk == K

I suggest using nn & N mm & M and kk & K instead of ii, jj, kk makes it easer to pair the index and the dimension.

 

[Adnan86]

yes you wright
thank you

- - - Updated - - -

thank you again , i checked it again , yee i know what should i do as your guidance in previews post .
Thank you so much .

 

[ads-ee]

no i checked it , for example in this case K = 1 ; after the first pass of jj ; kk will be increment, but after that because of kk > K
Am i wright ?

It doesn't work because you don't increment and reset the indexes when you should.

e.g.
ii: 1 1 2 2 3 3
jj: 1 2 1 2 1 2
kk 1 1 1 1 1 1

Your sim has ii stuck at 1 and jj increments to 3!

you need to DESIGN this first then write the code for it.

I've given you a starting point in #8

 

[Adnan86]

I wrote #7 before saw #8 , after that i realized that i'm wrong .
thanks for give your time

- - - Updated - - -

i almost solved my problem , just have some problem that i can fix it . thank you for helping .
just as a last question if i want use this program (code ) as COMPONENT , in another program , but need twice , it means one time AxB=C ,
and then use the answer of previews for second time such CxD=E ,how can i delay for second time till answer (C) be complete and then used it for second time .
thank you again , i really for 2 days ,couldn't find my problem but your guidance gave me clue to fix my problem .

 

[ads-ee]

just as a last question if i want use this program (code ) as COMPONENT , in another program , but need twice , it means one time AxB=C ,
and then use the answer of previews for second time such CxD=E ,how can i delay for second time till answer (C) be complete and then used it for second time .
thank you again , i really for 2 days ,couldn't find my problem but your guidance gave me clue to fix my problem .

Don't think of your VHDL code as a program. VHDL doesn't execute on a microprocessor it doesn't execute in an FPGA or ASIC. It gets synthesized into gates and flip-flops (i.e. hardware).

To use the output of the first matrix multiplier for a second add a signal from the first that says the output of the C array is valid. That would load the input registers that feed the C and D inputs of the CXD=E matrix multiplier. Other options are add an enable signal to the matrix multiplier, make a top level FSM/sequencer that counts out clock cycles and enables (removes reset) from the matrix multipiers

This is why I originally commented on the flag logic...It's not a good way to disable the matrix multiplier or start it up.

You should have a reset (that's all it does is reset the design not for holding off startup) and you should have an enable/start signal along with a valid data output signal (which can be the start signal for the next module).

Regards

 

[Adnan86]

Don't think of your VHDL code as a program. VHDL doesn't execute on a microprocessor it doesn't execute in an FPGA or ASIC. It gets synthesized into gates and flip-flops (i.e. hardware).

To use the output of the first matrix multiplier for a second add a signal from the first that says the output of the C array is valid. That would load the input registers that feed the C and D inputs of the CXD=E matrix multiplier. Other options are add an enable signal to the matrix multiplier, make a top level FSM/sequencer that counts out clock cycles and enables (removes reset) from the matrix multipiers

This is why I originally commented on the flag logic...It's not a good way to disable the matrix multiplier or start it up.

You should have a reset (that's all it does is reset the design not for holding off startup) and you should have an enable/start signal along with a valid data output signal (which can be the start signal for the next module).

Regards

ok thank you ...
you said "Don't think of your VHDL code as a program. VHDL doesn't execute on a microprocessor it doesn't execute in an FPGA or ASIC. It gets synthesized into gates and flip-flops (i.e. hardware)." , it means if i used this code in xilins ISE , for implementation on FPGA , this code couldn't work , because i want use it and some another code together for implementation on FPGA .

 

[ads-ee]

The code you wrote will produce sequential logic, there's nothing inherently wrong with it. The indexing if it grows too large will be an issue as you are using registers (flip-flops) to implement the arrays. If you want more efficient code that can deal with much larger matrices then you need to implement this using a RAM.

Regards.

 

[Adnan86]

The code you wrote will produce sequential logic, there's nothing inherently wrong with it. The indexing if it grows too large will be an issue as you are using registers (flip-flops) to implement the arrays. If you want more efficient code that can deal with much larger matrices then you need to implement this using a RAM.

Regards.

Understood , i really appreciate for giving your time , thanks so much

 

[Adnan86]

Don't think of your VHDL code as a program. VHDL doesn't execute on a microprocessor it doesn't execute in an FPGA or ASIC. It gets synthesized into gates and flip-flops (i.e. hardware).

To use the output of the first matrix multiplier for a second add a signal from the first that says the output of the C array is valid. That would load the input registers that feed the C and D inputs of the CXD=E matrix multiplier. Other options are add an enable signal to the matrix multiplier, make a top level FSM/sequencer that counts out clock cycles and enables (removes reset) from the matrix multipiers

This is why I originally commented on the flag logic...It's not a good way to disable the matrix multiplier or start it up.

You should have a reset (that's all it does is reset the design not for holding off startup) and you should have an enable/start signal along with a valid data output signal (which can be the start signal for the next module).

Regards

I did all you said and worked , but just one problem, as you see in my code for multiplication A,B as input are 16 bit and C as a output 32 bit .
here my problem i f i used this code as a component , for first time i have no problem but for second times as i want CxD = E, C is 32 bit but in main code it's 16 bit , so what can i do for second ?
Thanks

 

[ads-ee]

either make the widths generics and increase the bit widths or scale the values at the output.

 

[Adnan86]

either make the widths generics and increase the bit widths or scale the values at the output.

I really search for your suggestions , but not matrics , see i f it's not matrics i easyly could used this for resize :

x_a(15) <= C(31);
 x_a(14 downto 0) <= C(14 downto 0);

but as you know C and x_a it's matrics , and we show any part of matrics with this for example C(mm,nn) , and if i used C(14 downto 0) , give me error because C it's 2D . for using Generic , i don't know how used it in PACKAGE !!!!
sorry for asking too many questions ...
I have limited time for finish of this project and not expert in vhdl for that asked too many question.

- - - Updated - - -

 

[ads-ee]

I really search for your suggestions , but not matrics , see i f it's not matrics i easyly could used this for resize :

x_a(15) <= C(31);
 x_a(14 downto 0) <= C(14 downto 0);

but as you know C and x_a it's matrics , and we show any part of matrics with this for example C(mm,nn) , and if i used C(14 downto 0) , give me error because C it's 2D . for using Generic , i don't know how used it in PACKAGE !!!!
sorry for asking too many questions ...
I have limited time for finish of this project and not expert in vhdl for that asked too many question.

- - - Updated - - -

I'm not really sure how to add a generic vector width to a 2D array that is defined in a package. I'm not a big fan of VHDL as I find I'm usually always trying to find a way around some seemingly restrictive aspect of the language. I feel Verilog is much more straight forward in how you code something.

Regards

- - - Updated - - -

I saw this on another post but I'm not sure it will work with Xilinx. I suspect they haven't added enough support for even this part of VHDL 2008
https://www.edaboard.com/threads/266446/

- - - Updated - - -

BTW Verilog doesn't support arrays passed through ports so I typically just make my ports like this input [mm*nn*width-1:0] a_port, and later in the code I use a for loop to assign the values to an actual array variable. It seems to me the added value (headaches) of having VHDL support for array in ports is not worth it.

 

[Adnan86]

I'm not really sure how to add a generic vector width to a 2D array that is defined in a package. I'm not a big fan of VHDL as I find I'm usually always trying to find a way around some seemingly restrictive aspect of the language. I feel Verilog is much more straight forward in how you code something.

Regards

- - - Updated - - -

I saw this on another post but I'm not sure it will work with Xilinx. I suspect they haven't added enough support for even this part of VHDL 2008
https://www.edaboard.com/threads/266446/

- - - Updated - - -

BTW Verilog doesn't support arrays passed through ports so I typically just make my ports like this input [mm*nn*width-1:0] a_port, and later in the code I use a for loop to assign the values to an actual array variable. It seems to me the added value (headaches) of having VHDL support for array in ports is not worth it.

let's see what happen , i'll try to solve it .
Thanks for your help .

- - - Updated - - -

let's see what happen , i'll try to solve it .
Thanks for your help .

My problem solved Thank you ads-ee .