Subtractor Module VHDL generating wrong values

Question

I have a code as such below that is designed to do subtraction and addition. Basically, when Binv is set, it should subtract, and Binv is 0, it should add. Unfortunately, it seems to be adding when Binv is set sometimes, and subtracting when it isn't set sometimes. Here is a snapshot of the simulation:

entity ADD_SUB is
Port ( A : in  STD_LOGIC_VECTOR (31 downto 0);
       B : in  STD_LOGIC_VECTOR (31 downto 0);
     Binv : in  STD_LOGIC;
     C_in: in  STD_LOGIC;
       S : out  STD_LOGIC_VECTOR (31 downto 0);
     TEST : out  STD_LOGIC_VECTOR (31 downto 0);
       C_out : out  STD_LOGIC
       );
end ADD_SUB;

architecture ADD_SUB_ARCH of ADD_SUB is
signal S_wider : std_logic_vector(32 downto 0);
begin

process (A,B,C_in,Binv)
begin

if Binv = '0' then
    S_wider <= ( A(31) & A) + ( B(31) & B) + C_in;
elsif Binv = '1' then
    S_wider <= (A(31)& A) + ('1'& not B) + '1';
else
    S_wider <= std_logic_vector(to_signed(0,32));
end if;

S <= S_wider(31 downto 0);
C_out <= S_wider(32);

end process;

I am getting nonsensical results which make no sense. In the first case, you can see that I tried to do (50 - 30) (Binv is 1). I get 80 which is wrong. You can see however that it works on (30 - 50) where I get -20. Second problem is where I try to do (30 + (-50)), however it shows up as 20.

The results are completely off and I can't see where I am going wrong

Answer 1

Jim is absolutely correct.

There are a couple of points that may be worth making.

First, the + C_in or + not C_in implies two 32 bit adds, one of which gets optimized away during synthesis leaving just the carry in to the remaining add.

Second, you are really only manipulating B and C_in using Binv. Subtraction is the equivalent of adding the two's complement, for B the one's complement + X"00000001. Note that Jim inverts C_in with Binv which allows C_in to be used for daisy chain operations (e.g. a 64 bit add or subtract with a 32 bit ALU).

Both points are illustrated with the following code, which also only uses numeric_std.unsigned and and only needs the unsigned numeric_std."+":

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity add_sub is
    port ( 
        a:      in  std_logic_vector (31 downto 0);
        b:      in  std_logic_vector (31 downto 0);
        binv:   in  std_logic;
        c_in:   in  std_logic;
        s:      out std_logic_vector (31 downto 0);
        test:   out std_logic_vector (31 downto 0);
       c_out:   out std_logic
    );
end entity;

architecture foo of add_sub is

begin

UNLABELLED:
    process (a,b,c_in,binv)
        variable x,y,z: std_logic_vector (33 downto 0);
    begin
        x := a(31) & a & '1'; -- this '1' generates a true carry in to z(1)
                              -- z(0) is optimized away as unused it's carry
                              -- retained as carry in to the next MS bit.
        if binv = '0' then
            y := b(31) & b & c_in;
        elsif binv = '1' then
            y := not b(31) & not b & not c_in;
        else 
            y := (others => 'X');  -- 'X' on binv is propagated from b onto y
        end if;

        z := std_logic_vector( unsigned(x) + unsigned(y));  -- only one add

        c_out <= z(33);
        s <= z(32 downto 1);

    end process;
end architecture;

This above example connects C_in a bit more directly to the adder stage with the LS bits of A and B and gives:

(The image is can be clicked to open)

(Synthesis software is generally smart enough to do all this with using Jim's form modified to either add or subtract based on Binv and A and B extended to 33 bits without any direct bit or bitfield manipulation, our synthesis tools have had more than 25 years to get it right.)

The waveform was produced with the following test bench:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity tb_add_sub is
end entity;

architecture foo of tb_add_sub is
    signal a:       std_logic_vector (31 downto 0) := (others =>'0');
    signal b:       std_logic_vector (31 downto 0) := (others =>'0');
    signal binv:    std_logic := '0';
    signal c_in:    std_logic := '0';
    signal s:       std_logic_vector (31 downto 0);
    signal test:    std_logic_vector (31 downto 0);
    signal c_out:   std_logic;

begin

DUT:
    entity work.add_sub
        port map ( 
            a => a,
            b => b,
            binv => binv,
            c_in => c_in,
            s => s,
            test => test,
           c_out => c_out
        );

STIMULUS:
    process
    begin
        wait for 100 ns;
        a <= std_logic_vector(to_signed(50,a'length));
        b <= std_logic_vector(to_signed(30,b'length));
        wait for 100 ns;
        binv <= '1';
        wait for 100 ns;
        binv <= '0';
        a <= std_logic_vector(to_signed(30,a'length));
        b <= std_logic_vector(to_signed(-50,b'length));
        wait for 100 ns;
        binv <= '1';
        b <= std_logic_vector(to_signed(50,b'length));
        wait for 600 ns;
        wait;
    end process;
end architecture;