Subtraction with numeric_Std doesn't work

[Altera_Forum]

Hello 

 What am i doing in wrong in the code below. subtraction doesn't work! I am using numeric_std and the numbers are in twos complement format.  // mattgust  library ieee; use ieee.std_logic_1164.all; use ieee.numeric_std.all; use work.OF_DET.all; entity Arith is port( op_code : in std_logic_vector(1 downto 0); a_in : in std_logic_vector(7 downto 0); b_in : in std_logic_vector(7 downto 0); zero_det : out std_logic; overflow : out std_logic; alu_out : out std_logic_vector(7 downto 0) ); end entity Arith; architecture ALU of Arith is signal Tmp_a : signed(a_in'range):=(others=>'0'); signal Tmp_b : signed(b_in'range):=(others=>'0'); signal Tmp_out : signed(alu_out'range):=(others=>'0'); begin Tmp_a <= signed(a_in); Tmp_b <= signed(b_in); ALU:process(Tmp_out, Tmp_a, Tmp_b) begin case op_code is when "00" => Tmp_out <= Tmp_a and Tmp_b; when "01" => Tmp_out <= Tmp_a or Tmp_b; when "10" => Tmp_out <= Tmp_a + Tmp_b; overflow <= detect_overflow(Tmp_out, Tmp_a, Tmp_b); zero_det <= detect_zero(Tmp_out); when "11" => Tmp_out <= Tmp_a - Tmp_b; overflow <= detect_overflow(Tmp_out, Tmp_a, Tmp_b); zero_det <= detect_zero(Tmp_out); when others => null; end case; alu_out <= std_logic_vector(Tmp_out); end process; end architecture ALU;  -- Copyright (C) 1991-2012 Altera Corporation -- Your use of Altera Corporation's design tools, logic functions -- and other software and tools, and its AMPP partner logic -- functions, and any output files from any of the foregoing -- (including device programming or simulation files), and any -- associated documentation or information are expressly subject -- to the terms and conditions of the Altera Program License -- Subscription Agreement, Altera MegaCore Function License -- Agreement, or other applicable license agreement, including, -- without limitation, that your use is for the sole purpose of -- programming logic devices manufactured by Altera and sold by -- Altera or its authorized distributors. Please refer to the -- applicable agreement for further details. -- *************************************************************************** -- This file contains a Vhdl test bench template that is freely editable to -- suit user's needs .Comments are provided in each section to help the user -- fill out necessary details. -- *************************************************************************** -- Generated on "11/17/2012 12:17:48" -- Vhdl Test Bench template for design : Arith -- -- Simulation tool : ModelSim-Altera (VHDL) -- LIBRARY ieee; USE ieee.std_logic_1164.all; ENTITY Arith_vhd_tst IS END Arith_vhd_tst; ARCHITECTURE Arith_arch OF Arith_vhd_tst IS -- constants -- signals SIGNAL a_in : STD_LOGIC_VECTOR(7 DOWNTO 0); SIGNAL b_in : STD_LOGIC_VECTOR(7 DOWNTO 0); SIGNAL alu_out : STD_LOGIC_VECTOR(7 DOWNTO 0); SIGNAL op_code : STD_LOGIC_VECTOR(1 DOWNTO 0); SIGNAL overflow : STD_LOGIC; SIGNAL zero_det : STD_LOGIC; COMPONENT Arith PORT ( a_in : IN STD_LOGIC_VECTOR(7 DOWNTO 0); alu_out : OUT STD_LOGIC_VECTOR(7 DOWNTO 0); b_in : IN STD_LOGIC_VECTOR(7 DOWNTO 0); op_code : IN STD_LOGIC_VECTOR(1 DOWNTO 0); overflow : OUT STD_LOGIC; zero_det : OUT STD_LOGIC ); END COMPONENT; BEGIN i1 : Arith PORT MAP ( -- list connections between master ports and signals a_in => a_in, alu_out => alu_out, b_in => b_in, op_code => op_code, overflow => overflow, zero_det => zero_det ); process begin op_code <="10"; wait for 300 ns; op_code <="11"; wait for 300 ns; end process; process begin a_in <= "01111110"; b_in <= "00000010"; wait for 50 ns; a_in <= "00000010"; b_in <= "00000010"; wait for 50 ns; a_in <= "01111111"; b_in <= "00000000"; wait for 50 ns; a_in <= "01111111"; b_in <= "10000000"; wait for 50 ns; a_in <= "00011110"; b_in <= "11011000"; wait for 50 ns; a_in <= "00000010"; b_in <= "11111110"; wait for 50 ns; a_in <= "10000000"; -- Subtraktion härifrån b_in <= "00000010"; wait for 50 ns; a_in <= "01111111"; b_in <= "11111110"; wait for 50 ns; a_in <= "00001000"; b_in <= "00001000"; wait for 50 ns; a_in <= "01111111"; b_in <= "10000000"; wait for 50 ns; a_in <= "00011110"; b_in <= "11011000"; wait for 50 ns; a_in <= "00000010"; b_in <= "11111110"; wait for 50 ns; end process; END Arith;

[Altera_Forum]

The testbench shows that the ALU works perfectly. The result is always right. But I'm sure the overflow detector is wrong. You didn't post the code. The problem is in the substraction. Look at the the first sub: -128 - ( +2) = -130. But you can't store -130 in 8 bits. So it's an overflow. Signal overflow should be '1'.

[Altera_Forum]

I'm sorry, i missed that! Here is the code for Overflow flag. 

I'm extending the in bytes and compare the sign bit of the extendend sum and the old sum.   library ieee; use ieee.std_logic_1164.all; use ieee.numeric_std.all; package OF_DET is function detect_overflow(Tmp, A, B : in signed(7 downto 0)) return std_logic; function detect_zero(Tmp : in signed(7 downto 0)) return std_logic; end; package body OF_DET is function detect_overflow(Tmp, A, B : in signed(7 downto 0)) return std_logic is variable Tmp_xtnd : signed(8 downto 0); variable A_xtnd : signed(8 downto 0); variable B_xtnd : signed(8 downto 0); variable same_sign : std_logic; variable plus_minus : std_logic; begin Tmp_xtnd := (Tmp(7) & Tmp); --resize(Tmp, 9); A_xtnd := (A(7) & A); --resize(A, 9); B_xtnd := (B(7) & B); --resize(A, 9); same_sign := (A(7) xor B(7)); if (same_sign = '0') then Tmp_xtnd := A_xtnd + B_xtnd; if (Tmp_xtnd(8) /= Tmp(7) ) then --Tmp_xtnd(7) return '1'; else return '0'; end if; else return '0'; end if; end detect_overflow; function detect_zero(Tmp : in signed(7 downto 0)) return std_logic is begin if (Tmp = 0) then return '1'; else return '0'; end if; end detect_zero; end OF_DET;

[Altera_Forum]

You need the opcode to generate overflow signal. I give you some rules: 

 a-If your adding and the operands have oposite signs an overflow NEVER occurs.  b-If your adding and the operands have the same sign and the result has the same sign too, there are NO overflow.  c-If your adding and the operands have the same sign and the result has oposite same sign, there ARE overflow.  e-If your substracting and the operands have the same sign an overflow NEVER occurs.  f-If your substracting and the operands have oposite sign, but the result has same sign as first operand, there are NO overflow.  g-If your substracting and the operands have oposite sign, but the result has oposite sign as first operand, there ARE overflow.

[Altera_Forum]

Thank you for your fast replies. I will rewrite my overflow detector with your guidelines.

[Altera_Forum]

Just curious, why use this complicated overflow detector. All you need just extend your addition or subtraction result by 1 bit then you can check if there is overflow or not in all cases if bit(8) /= bit(7). Or am I missing something?

[Altera_Forum]

Here is the working version. Following kaz advice. 

  function detect_overflow(Tmp, A, B : in signed(7 downto 0); op : in std_logic_vector(1 downto 0)) return std_logic is variable Tmp_xtnd : signed(8 downto 0); variable A_xtnd : signed(8 downto 0); variable B_xtnd : signed(8 downto 0); begin Tmp_xtnd := (Tmp(7) & Tmp); --resize(Tmp, 9); A_xtnd := (A(7) & A); --resize(A, 9); B_xtnd := (B(7) & B); --resize(A, 9); case op is when "10" => Tmp_xtnd := A_xtnd + B_xtnd; if (Tmp_xtnd(8) /= Tmp_xtnd(7) ) then return '1'; else return '0'; end if; when "11" => Tmp_xtnd := A_xtnd - B_xtnd; if (Tmp_xtnd(8) /= Tmp_xtnd(7)) then return '1'; else return '0'; end if; when others => return '0'; end case; end detect_overflow;

[Altera_Forum]

Your "overflow" detector appears to be detecting overflow and underflow, so calling the output "overflow" is a bit of a misconception. It should probably just be renamed error detect.

[Altera_Forum]

--- Quote Start ---  Your "overflow" detector appears to be detecting overflow and underflow, so calling the output "overflow" is a bit of a misconception. It should probably just be renamed error detect.  --- Quote End ---      If you say so, it sounds reasonable.

[Altera_Forum]

Fair enough. Using package and functions may be good exercise for study but you don't really need two adders and two subtractors for same operations. 

Thus, for practical design I will do the following(code not tested):   library ieee; use ieee.std_logic_1164.all; use ieee.numeric_std.all; entity Arith is port( op_code : in std_logic_vector(1 downto 0); a_in : in std_logic_vector(7 downto 0); b_in : in std_logic_vector(7 downto 0); zero_det : out std_logic; overflow : out std_logic; alu_out : out std_logic_vector(7 downto 0) ); end entity Arith; architecture ALU of Arith is signal Tmp_a : signed(8 downto 0) :=(others=>'0'); signal Tmp_b : signed8 downto 0) :=(others=>'0'); signal Tmp_out : signed(8 downto 0) :=(others=>'0'); begin Tmp_a <= resize(signed(a_in),9); Tmp_b <= resize(signed(b_in),9); ALU:process(op_code,Tmp_out, Tmp_a, Tmp_b) begin case op_code is when "00" => Tmp_out <= Tmp_a and Tmp_b; when "01" => Tmp_out <= Tmp_a or Tmp_b; when "10" => Tmp_out <= Tmp_a + Tmp_b; when "11" => Tmp_out <= Tmp_a - Tmp_b; when others => null; end case; alu_out <= std_logic_vector(Tmp_out(7 downto 0)); overflow <= Tmp_out(8) xor Tmp_out(7); zero_det <= '0'; if Tmp_out = "000000000" then zero_det <= '1'; end if; end process; end architecture ALU;   Also notice that your design is all combinatorial. Nothing wrong about that but once you integrate with registers then you may decide to add registers. Moreover you can reduce adder/subtractor to one adder only by negating the input (negation require invert and add 1, a small adder):  Tmp_out <= Tmp_a + Tmp; --before case ...  when "10" => Tmp <= Tmp_b; when "11" => Tmp <= -Tmp_b;

[Altera_Forum]

--- Quote Start ---    Also notice that your design is all combinatorial. Nothing wrong about that but once you integrate with registers then you may decide to add registers. Moreover you can reduce adder/subtractor to one adder only by negating the input (negation require invert and add 1, a small adder):    --- Quote End ---    You mean like this?:   ALU: process(clk) begin  if rising_edge(clk)  ....  end if; end process;

[Altera_Forum]

Hi all, 

 I have a subtraction part in my code:  x<=unsigned(DataSent); y<=N/2; y1<=(unsigned(x)-y); my variables datasent ,x,y,y1,all are integers. when i comment the statement  y1<=(unsigned(x)-y); the variable x rightly has the datasent value but when the above statement is not commented the code breaks at y1<=unsigned(x)-y  How can x loose its value? It shows the -214748368  how do i make x store the right value ?  say if datasent is 231 and y-N/2 is 6 for N=12   what correction has to be done to get y1= 231-6=225  Can someone verify why the processing stops at the line y1<=x-y?  Thanks in Advance

[Altera_Forum]

If x is indeed an integer, then this code:y1<=(unsigned(x)-y);Will not compile. unsigned() can be used to convert a std_logic_vector to unsigned, but if you are converting from an integer, then you need to use the to_unsigned() function, and specify the number of bits for the result. 

Besidesy1<=x-y;should be enough

[Altera_Forum]

CAn you post the full code. Your snippet has all sorts of type errors.

[Altera_Forum]

--- Quote Start ---  CAn you post the full code. Your snippet has all sorts of type errors.  --- Quote End ---    yes below is the entire code  library ieee; use ieee.std_logic_1164.all; use ieee.std_logic_arith.all; use IEEE.math_real.all; -- for UNIFORM, TRUNC functions USE ieee.numeric_std.ALL; -- for TO_UNSIGNED function use ieee.STD_LOGIC_UNSIGNED.all; use ieee.std_logic_textio.all; entity ADwgn is port ( diginp:in std_logic_vector(11 downto 0); digiop:out std_logic_vector(11 downto 0);   x:inout integer; result:out std_logic_vector(11 downto 0); lo_val:out integer ); end;   architecture a of ADwgn is subtype unsigned is integer range 0 to 255;  signal y1,R,y: unsigned := 0;   signal rst_n: std_logic:='1'; signal N: integer:=12;   signal middle: integer:=0; signal c:real; signal rand_wave: std_logic_vector(11 downto 0); signal rand: integer:=0; signal mean: integer:=1; signal variance:integer:= 2;     begin    process(rst_n)    variable RandomVal : real ;  variable DataSent : integer ; -- Random integer value in range 0..256  -- Declare seeds and initialize  -- Uniform uses seeds as state information,  -- so initialize only once  variable DataSent_seed1 : positive := 7 ; -- Seed values for random generator  variable DataSent_seed2 : positive := 1 ;  VARIABLE stim: std_logic_vector(11 DOWNTO 0); -- Random 12-bit stimulus   begin  if rst_n='0' then  x<=0;  elsif rst_n='1' then    for i in 1 to 10 loop  -- Generate a value between 0.0 and 1.0 (non-inclusive)  uniform(DataSent_seed1, DataSent_seed2, RandomVal) ;  -- Convert to integer in range of 0 to 255  DataSent := integer(trunc(RandomVal*256.0)) ;  stim := std_logic_vector(to_unsigned(DataSent, stim'LENGTH)); -- convert to std_logic_vector  lo_val<=i; end loop;   x<=unsigned(DataSent);   --tmp_a<= x; y<=N/2;   y1<=(x-y); -- set mean to 0 PROBLEM WITH CODE AT THIS LINE  result<=stim;  rst_n<='0'; middle<= (12/N); c<=sqrt(real(middle)); --rand<=unsigned(real(x)*c); -- PROBLEM ARISES HERE ALSO  --R<=integer(rand); -- rand_wave<=std_logic_vector(to_unsigned(R,rand_wave'LENGTH)) ; -- adjust variance to 1    -- rand_wave<=std_logic_vector(to_unsigned(x, rand_wave'LENGTH))  end if;    digiop<=diginp xor stim;  --x' <= mean + (sqrt(real(variance)) * x)         end process;   end;   --When the algorithm finishes, X will be our unit normal random.  --X can be further modified to have a particular mean and variance, e.g.:        Thanks

[Altera_Forum]

THe problem comes as you are missing several signals from the sensitivity list, namely x,y, y1, N, middle. But theres a couple of other issues: 

The values only get updated when rst_n is '1'. It starts as '1', but will imediatly goes to '0'.  You declared your own subtype called unsigned. This is a bad idea, as numeric_std and std_logic_arith also declare unsigned types. You're going to have problems if you stick with these. I suggest deleting your custom unsigned subtype and the non-standard std_logic_arith library.  Also: I assume you know this code is not synthesisable (ie. You cannot put it on an FPGA) - I guess this is some kind of simulation model?

[Altera_Forum]

--- Quote Start ---  THe problem comes as you are missing several signals from the sensitivity list, namely x,y, y1, N, middle. But theres a couple of other issues: The values only get updated when rst_n is '1'. It starts as '1', but will imediatly goes to '0'.  You declared your own subtype called unsigned. This is a bad idea, as numeric_std and std_logic_arith also declare unsigned types. You're going to have problems if you stick with these. I suggest deleting your custom unsigned subtype and the non-standard std_logic_arith library.  Also: I assume you know this code is not synthesisable (ie. You cannot put it on an FPGA) - I guess this is some kind of simulation model?  --- Quote End ---     Hi ,  Yes this is only a simulation model.   i simply specify y1<=integer(x-y ) where all are declared as integer .Though x and y hold there corresponding values say 231 and 6, y1 displays only its maximum range i.e -214748368.  And when i remove the subtype declaration it says unsigned is not directly visible.  Thanks in advance!

[Altera_Forum]

--- Quote Start ---    i simply specify y1<=integer(x-y ) where all are declared as integer .Though x and y hold there corresponding values say 231 and 6, y1 displays only its maximum range 214748368.   --- Quote End ---    With the code as it is, this is impossible, as that would be out of range of the subtype and would cause an error.   And when i remove the subtype declaration it says unsigned is not directly visible.   This is because numeric_std and std_logic_arith both declare unsigned. And so it doesnt know which one to use. So Delete the non-standard std_logic_arith library like I said.