Forum Discussion

Honored Contributor

12 years ago

Addition problem in Quartus

So, after break the head for some minutes trying to find out why an simple addition didnt work, i went to see the RTL and found an weird thing:

https://dl.dropboxusercontent.com/u/38797606/Untitled.png

so, beyond putting an '0' on the last bit, it magically merges an vector of 32 bits into one of 31 bits (see big arrow on the image)?

the bigger vectors (of 46 bits) also have this issue.


library IEEE;use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
entity COUNTEUR_XYR_HIRES is Port ( 
    CODEUR1 : IN  STD_LOGIC_VECTOR (1  downto 0);
    CODEUR2 : IN  STD_LOGIC_VECTOR (1  downto 0);
    RELATION: IN  STD_LOGIC_VECTOR (23 downto 0);
    POSX    : OUT STD_LOGIC_VECTOR (31 downto 0);
    POSY    : OUT STD_LOGIC_VECTOR (31 downto 0);
    ROT     : OUT STD_LOGIC_VECTOR (15 downto 0);  --0-360 degrees+7 bits
    RESET   : IN  STD_LOGIC;
    H       : IN  STD_LOGIC
);
end COUNTEUR_XYR_HIRES;
architecture Behavioral of COUNTEUR_XYR_HIRES is
signal intPosX : Integer; --pas (32 bit) * 14 bits => 46 bits
signal intPosY : Integer; --pas (32 bit) * 14 bits => 46 bits
signal buffPosX : std_logic_vector(45 downto 0); 
signal buffPosY : std_logic_vector(45 downto 0); 
signal buffRot :  std_logic_vector(31 downto 0); 
signal subSubRot, subIncRot : std_logic_vector(31 downto 0);
signal nextSubRot, nextIncRot : std_logic_vector(31 downto 0);
--signal lastCod1, lastCod2: std_logic_vector(1 downto 0);
signal SINUS, COSINUS: STD_LOGIC_VECTOR(15 downto 0);
component TABLE_SINUS is Port (  
    H       : in  STD_LOGIC;
    ANGLE   : in  STD_LOGIC_VECTOR(15 downto 0);
    SINUS   : out STD_LOGIC_VECTOR(15 downto 0);
    COSINUS : out STD_LOGIC_VECTOR(15 downto 0)
);
end component;
begin
    POSX <= buffPosX(45 downto 14);
    POSY <= buffPosY(45 downto 14);
    ROT  <=  buffRot(31 downto 16);
    
    buffPosX <= std_logic_vector(to_unsigned(intPosX,46));
    buffPosY <= std_logic_vector(to_unsigned(intPosY,46));
    --buffRot  <= std_logic_vector(to_unsigned(intRot,32)) ;
    
    subSubRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) - TO_INTEGER(UNSIGNED(RELATION)),32));
    subIncRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) + TO_INTEGER(UNSIGNED(RELATION)),32));
    nextIncRot(23 downto 0) <= subIncRot(23 downto 0);
    nextSubRot(23 downto 0) <= subSubRot(23 downto 0);
    
    async : process(subIncRot, subSubRot)
    begin
        if(subIncRot(31 downto 24)=x"B4")then
            nextIncRot(31 downto 24) <= x"00";
        else
            nextIncRot(31 downto 24) <= subIncRot(31 downto 24);
        end if;
        if(subSubRot(31 downto 24)=x"FF")then
            nextSubRot(31 downto 24) <= x"B3";
        else
            nextSubRot(31 downto 24) <= subSubRot(31 downto 24);
        end if;
    end process;
    
    sin:TABLE_SINUS PORT MAP(
        H => H,
        ANGLE => buffRot(31 downto 16),
        SINUS => SINUS,
        COSINUS => COSINUS
    );
  
  onHorloge : process (H)
  begin
        if(H'event and H='1')then
            if(RESET='1')then
                intPosX<=0;
                intPosY<=0;
                buffRot <= x"00000000";
            else
                if(CODEUR1(0)='1')then
                    buffRot <= nextIncRot;
                    if(COSINUS(15)='0')then --cosinus>0
                        intPosX <= intPosX + TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    else
                        intPosX <= intPosX - TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    end if;
                    if(SINUS(15)='0')then
                        intPosY <= intPosY - TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    else
                        intPosY <= intPosY + TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    end if;
                elsif(CODEUR1(1)='1')then
                    buffRot <= nextSubRot;
                    if(COSINUS(15)='0')then --cosinus>0
                        intPosX <= intPosX - TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    else
                        intPosX <= intPosX + TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    end if;
                    if(SINUS(15)='0')then
                        intPosY <= intPosY + TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    else
                        intPosY <= intPosY - TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    end if;
                elsif(CODEUR2(0)='1')then
                    buffRot <= nextIncRot;
                    if(COSINUS(15)='0')then --cosinus>0, mais ici on subtraient
                        intPosX <= intPosX - TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    else
                        intPosX <= intPosX + TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    end if;
                    if(SINUS(15)='0')then
                        intPosY <= intPosY + TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    else
                        intPosY <= intPosY - TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    end if;
                elsif(CODEUR2(1)='1')then
                    buffRot <= nextSubRot;
                    if(COSINUS(15)='0')then --cosinus>0
                        intPosX <= intPosX + TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    else
                        intPosX <= intPosX - TO_INTEGER(UNSIGNED(COSINUS(14 downto 0)));
                    end if;
                    if(SINUS(15)='0')then
                        intPosY <= intPosY - TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    else
                        intPosY <= intPosY + TO_INTEGER(UNSIGNED(SINUS(14 downto 0)));
                    end if;
                end if;
            end if;
        end if;
    end process;
end Behavioral;

I contourned the problem by shifting 1 bit all the vectors relative (thus, lost 1 bit reolution). but still, why cant i make a "simple" (32 bit) addition?

20 Replies

Altera_Forum
Honored Contributor
12 years ago
and do you get wrong results.

adder0 in your post is the subtractor. adder1 is the adder.

I expected 32 bits as you did. it is indeed 32 bits on adder1 but 33 bits on adder0.
It looks like the adder0 is converted to subtractor by converting one input to signed so 7f is added on the msbs + sign bit.
Altera_Forum
Honored Contributor
12 years ago
until i made this last change (shift all the logic one bit to the right), the counter was not working as wanted. its supposed to be a degree counter (with fixed point values) so, it should go from 359."999" (actually "111...1"b) to 0 and vice versa, but it was going from 255 to 0 and from 0 to 103(which is 359 without the MSB).

but the weirdest is: why in the RTL an 32 bit is put in a 31 bit entrance?
Altera_Forum
Honored Contributor
12 years ago
--- Quote Start ---
It looks like the adder0 is converted to subtractor by converting one input to signed so 7f is added on the msbs + sign bit.
--- Quote End ---

Indeed, it is an ADDer and a SUBer as you can see in this part:

subSubRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) - TO_INTEGER(UNSIGNED(RELATION)),32));
subIncRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) + TO_INTEGER(UNSIGNED(RELATION)),32));
Altera_Forum
Honored Contributor
12 years ago
you need to look at subtractor result to verify if it works or not and not further in your design.
The subtractor design on the adder is something I haven't seen before but looking at it now. To convert 23:0 bits unsigned to negative they have to invert each bit then add 1. They are apparently doing something clever in this addition of 1. I am still looking at it. I can see 23:0 is mapped to 24:1 implying multiply by 2 then a 1 is added at bit 0 (implying half) then another 1 is added to bit0 of other input?? implying add another 1 and so 2 is added then possibly one lsb at output is discarded implying 2/2 = 1, thus 1 is added this way. But I am still not sure completely. So you check this adder and tell us.
Altera_Forum
Honored Contributor
12 years ago
Why are you using integers in your code?

buffRot is std_logic_vector(31 downto 0), so UNSIGNED(buffRot) is a 32-bit unsigned value, and you cannot convert that to a 32-bit (signed) integer.

VHDL Specification (5.2.3.2 Predefined integer types):

"The only predefined integer type is the type INTEGER. The range of INTEGER is implementation dependent, but it is guaranteed to include the range –2147483647 to +2147483647. It is defined with an
ascending range."

i.e., integers have a guaranteed range of up to 0x7FFF_FFFF, which is a 31-bit unsigned number.

Your code:
subSubRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) - TO_INTEGER(UNSIGNED(RELATION)),32)); subIncRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) + TO_INTEGER(UNSIGNED(RELATION)),32));

should be re-written to use unsigned and/or signed types, the resize() operator, and then an appropriate rounding method.

Cheers,
Dave
Altera_Forum
Honored Contributor
12 years ago
--- Quote Start ---
Why are you using integers in your code?

buffRot is std_logic_vector(31 downto 0), so UNSIGNED(buffRot) is a 32-bit unsigned value, and you cannot convert that to a 32-bit (signed) integer.

VHDL Specification (5.2.3.2 Predefined integer types):

"The only predefined integer type is the type INTEGER. The range of INTEGER is implementation dependent, but it is guaranteed to include the range –2147483647 to +2147483647. It is defined with an
ascending range."

i.e., integers have a guaranteed range of up to 0x7FFF_FFFF, which is a 31-bit unsigned number.

Your code:
subSubRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) - TO_INTEGER(UNSIGNED(RELATION)),32)); subIncRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) + TO_INTEGER(UNSIGNED(RELATION)),32));

should be re-written to use unsigned and/or signed types, the resize() operator, and then an appropriate rounding method.

Cheers,
Dave
--- Quote End ---

Hi Dave,

I think that he is not converting it to any signed but the compiler does and it does lose 1 bit eventually from this 32 bit input.
the input buffrot(30 : 0) is mapped(I believe) to 31:1(i.e. bit 31 dropped) and an extra fixed lsb of 1 is added.
the "relation" input is 23:0 mapped to 24:1 and extra 1 also added so overall a 2 enters adder with inputs then one lsb must be discarded at output so in effect 1 is added(2/2).

So in principle it should work unless the user enters really large value on buffrot.

Thanks
Altera_Forum
Honored Contributor
12 years ago
Be aware that, with ieee.numeric_std, 32bits_vector + 32bits_vector = 32bits_vector the 33th bit (which is carry) is simply forgotten I have already face to : http://www.alteraforum.com/forum/showthread.php?t=21027&p=180870#post180870

In standard package, Integers are numbers from –2147483648 to +2147483647 (Be carefull dwh@ovro.caltech.edu ;-))

Be aware in

--- Quote Start ---
async : process(subIncRot, subSubRot)
--- Quote End ---

Quartus will ignore the sensitivity list (= (subIncRot, subSubRot)), and take as sensitivity all "input" signals.
In your case, it doesn't change the behaviour.

--- Quote Start ---
UNSIGNED(buffRot) is a 32-bit unsigned value
--- Quote End ---

Wrong, UNSIGNED(buffRot) will give exactly the same vector, it is just interpreted as unsigned for operations (mostly comparisons, multiplications)
BUT "TO_INTEGER" gives a 32bits vector by the compiler.
Altera_Forum
Honored Contributor
12 years ago
--- Quote Start ---

In standard package, Integers are numbers from –2147483648 to +2147483647 (Be carefull dwh@ovro.caltech.edu ;-))

--- Quote End ---

My statement above was copied directly from the VHDL standard, which includes what is "guaranteed to include". So technically the value –2147483648 comes for "free" in some packages :)

Bottom-line is that if you are going to use integers for data types in code, you need to start to get worried when your bit-widths gets near 32-bits.

Cheers,
Dave

Altera_Forum

Honored Contributor

12 years ago

[SOLVED] [?]

Since the Integer type doesnt work at 32 bit (it uses the bit 31 as carry, as the RTL shows) I worked 2 strategies to solve it:

For the ROT counter, I shifted all the logic one bit to the right, so i actually use 31 bits


ROT  <=  buffRot(30 downto 15);
subDecRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) - TO_INTEGER(UNSIGNED(RELATION)),32));
subIncRot <= STD_LOGIC_VECTOR(TO_UNSIGNED(TO_INTEGER(UNSIGNED(buffRot)) + TO_INTEGER(UNSIGNED(RELATION)),32));

For the 46 bits variables, I have implemented the sum as another VHDL file (even knowing it could be done with an function)


library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
entity ADDER_46 is Port ( 
	A    : IN  STD_LOGIC_VECTOR (45 downto 0);
	B    : IN  STD_LOGIC_VECTOR (45 downto 0);
	C    : OUT STD_LOGIC_VECTOR (45 downto 0);
	COUT : OUT STD_LOGIC;
	CIN  : IN  STD_LOGIC
);
end ADDER_46;
architecture Behavioral of ADDER_46 is
signal Carry: STD_LOGIC_VECTOR (46 downto 0); 
begin
Carry(0) <= CIN;
COUT <= Carry(46);
Carry(46 downto 1) <= (A and B)or(Carry(45 downto 0) and (A or B));
C <= (A xor B)xor Carry(45 downto 0);
end Behavioral;

Still, I think the designers of Quartus should give a look at the image I have posted in the first post.

Altera_Forum
Honored Contributor
12 years ago
Why are you using to_integer though?

What is wrong with just letting the VHDL signed type perform modulo math?

subDecRot <= STD_LOGIC_VECTOR(SIGNED(buffRot)-resize(SIGNED(RELATION),32)); subIncRot <= STD_LOGIC_VECTOR(SIGNED(buffRot)+resize(SIGNED(RELATION),32));

Cheers,
Dave

Forum Discussion

Addition problem in Quartus

20 Replies

Recent Discussions

Interfacing Avalon Streaming FIFO IP with GTS Ethernet Hard IP

Invalid license key (inconsistent authentication code)

Regarding the issue of UFM not starting

ram retiming

Reset Release IP for Agilex needs Stratix 10 device files installed!