Unexpected results when multiplying in VHDL

Question

I'm trying to make a simple BCD --> binary conversion operation work in an ALU I'm coding. All the other operations work perfectly fine, just this last operation doesn't work for some reason.

I've tried numerous approaches and nothing seems to make sense.

My inputs are A and B which are both 8-bit vectors. The outputs are X and Y.

So the formula goes Y:X = BCD2BIN(B:A).

In my case, in my testbench I have B = 0x40 and A = 0x46, so the output I'm expecting is 0xFCE, which is 4046.

Just for those who are unaware, the simplest way to perform this is to divide each byte into 2 nibbles, and multiply by a constant and sum the terms.

So:

B:A = 0x4046 = (4 * 1000) + (0 * 100) + (4 * 10) * (6 * 1)

Signals I'm Using

B = 0x40 --These are the inputs of the ALU, so it isn't actually coded this way.
A = 0x46 --However these are the values.
signal sig1000 : STD_LOGIC_VECTOR(15 downto 0) := x"03E8"; --1000
signal sig100 : STD_LOGIC_VECTOR(15 downto 0) := x"0064"; --100
signal sig10 : STD_LOGIC_VECTOR(15 downto 0) := x"000A"; --10
signal dig1 : STD_LOGIC_VECTOR(3 downto 0);
signal dig2 : STD_LOGIC_VECTOR(3 downto 0);
signal dig3 : STD_LOGIC_VECTOR(3 downto 0);
signal dig4 : STD_LOGIC_VECTOR(3 downto 0);

After the BEGIN

dig1 <= B(7 downto 4);
dig2 <= B(3 downto 0);
dig3 <= A(7 downto 4);
dig4 <= A(3 downto 0);

Actual Operation

opF <= STD_LOGIC_VECTOR((unsigned(dig1) * unsigned(sig1000)) + (unsigned(dig2) * unsigned(sig100)) + (unsigned(dig3) * unsigned(sig10)) + (unsigned(dig4)));

And later the opF gets split into Y and X.

So even verifying the signals in simulation, they are correct, yet the output is not.

The sig10001, sig100 and sig10 are the correct values, and dig1 = 4, dig2 = 0, dig3 = 4 and dig4 = 6 with unsigned decimal radix.

Yet the result is 252

I've even tried this using constants, instead of vectors, with even less success.

For example

opF <= STD_LOGIC_VECTOR((unsigned(dig1) * 10#1000#) + (unsigned(dig2) * 10#100#) + (unsigned(dig3) * 10#10#) + (unsigned(dig4)));

Full file, as some have requested.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
entity ALU is
Port ( A : in STD_LOGIC_VECTOR (7 downto 0);
 B : in STD_LOGIC_VECTOR (7 downto 0);
 Cin : in STD_LOGIC;
 OPCODE : in STD_LOGIC_VECTOR (3 downto 0);
 Y : out STD_LOGIC_VECTOR (7 downto 0);
 X : out STD_LOGIC_VECTOR (7 downto 0);
 Z : out STD_LOGIC;
 Cout : out STD_LOGIC;
 V : out STD_LOGIC;
 F_active : out STD_LOGIC;
 X_bin_pal : out STD_LOGIC;
 X_prime : out STD_LOGIC;
 N : out STD_LOGIC);
end ALU;
architecture Behavioral of ALU is
 signal resultSignal : STD_LOGIC_VECTOR(15 downto 0);
 signal cin_vector : STD_LOGIC_VECTOR(0 downto 0);
 signal op0 : STD_LOGIC_VECTOR(15 downto 0);
 signal op1 : STD_LOGIC_VECTOR(15 downto 0);
 signal op2 : STD_LOGIC_VECTOR(15 downto 0);
 signal op3 : STD_LOGIC_VECTOR(15 downto 0);
 signal op4 : STD_LOGIC_VECTOR(15 downto 0);
 signal op5 : STD_LOGIC_VECTOR(15 downto 0);
 signal op6 : STD_LOGIC_VECTOR(15 downto 0);
 signal op7 : STD_LOGIC_VECTOR(15 downto 0);
 signal op8 : STD_LOGIC_VECTOR(15 downto 0);
 signal op9 : STD_LOGIC_VECTOR(15 downto 0);
 signal opA : STD_LOGIC_VECTOR(15 downto 0);
 signal opB : STD_LOGIC_VECTOR(15 downto 0);
 signal opC : STD_LOGIC_VECTOR(15 downto 0);
 signal opD : STD_LOGIC_VECTOR(15 downto 0);
 signal opE : STD_LOGIC_VECTOR(15 downto 0);
 signal opF : STD_LOGIC_VECTOR(15 downto 0);
 signal dig1 : STD_LOGIC_VECTOR(3 downto 0);
 signal dig2 : STD_LOGIC_VECTOR(3 downto 0);
 signal dig3 : STD_LOGIC_VECTOR(3 downto 0);
 signal dig4 : STD_LOGIC_VECTOR(3 downto 0);
 signal Xsig : STD_LOGIC_VECTOR(7 downto 0) := x"00";
 signal Xsig_reverse : STD_LOGIC_VECTOR(7 downto 0) := x"00";
 signal Ysig : STD_LOGIC_VECTOR(7 downto 0) := x"00";
 signal Zsig : STD_LOGIC := '0';
 signal Coutsig : STD_LOGIC := '0';
 signal Vsig : STD_LOGIC := '0';
 signal Nsig : STD_LOGIC := '0';
 signal sig1000 : STD_LOGIC_VECTOR(15 downto 0) := x"03E8";
 signal sig100 : STD_LOGIC_VECTOR(15 downto 0) := x"0064";
 signal sig10 : STD_LOGIC_VECTOR(15 downto 0) := x"000A";
begin
cin_vector(0) <= Cin;
dig1 <= B(7 downto 4);
dig2 <= B(3 downto 0);
dig3 <= A(7 downto 4);
dig4 <= A(3 downto 0);
op0 <= x"00" & (A AND B);
op1 <= x"00" & (A OR B);
op2 <= x"00" & (A XOR B);
op3 <= x"00" & (A XNOR B);
op4 <= STD_LOGIC_VECTOR(unsigned(x"00" & A) + unsigned(x"00" & B));
op5 <= x"00" & STD_LOGIC_VECTOR(signed(A) + signed(B));
op6 <= STD_LOGIC_VECTOR(unsigned(x"00" & A) + unsigned(x"00" & B) + unsigned(x"000" & "000" & cin_vector));
op7 <= STD_LOGIC_VECTOR(signed(A) * signed(B));
op8 <= STD_LOGIC_VECTOR(unsigned(A) * unsigned(B));
op9 <= STD_LOGIC_VECTOR(unsigned(x"00" & A) - unsigned(x"00" & B));
opA <= STD_LOGIC_VECTOR(ROTATE_LEFT(unsigned(A & x"00"), 1));
opB <= STD_LOGIC_VECTOR(ROTATE_LEFT(unsigned(Cin & A & "0000000"), 1));
opC <= STD_LOGIC_VECTOR(SHIFT_RIGHT(unsigned(A & x"00"), 1));
opD <= STD_LOGIC_VECTOR(SHIFT_RIGHT(signed(A & x"00"), 1));
opE <= STD_LOGIC_VECTOR(SHIFT_LEFT(unsigned(A & x"00"), 1));
opF <= STD_LOGIC_VECTOR((unsigned(dig1) * unsigned(sig1000)) + (unsigned(dig2) * unsigned(sig100)) + (unsigned(dig3) * unsigned(sig10)) + (unsigned(dig4)));
with OPCODE select resultSignal <=
 op0 when x"0",
 op1 when x"1",
 op2 when x"2",
 op3 when x"3",
 op4 when x"4",
 op5 when x"5",
 op6 when x"6",
 op7 when x"7",
 op8 when x"8",
 op9 when x"9",
 opA when x"A",
 opB when x"B",
 opC when x"C",
 opD when x"D",
 opE when x"E",
 opF when x"F";
Ysig <= resultSignal(15 downto 8) when OPCODE = "0111" or OPCODE = "1000" or OPCODE = "1111" else
 x"00";
Y <= Ysig;
Xsig <= resultSignal(15 downto 8) when OPCODE = "1100" or OPCODE = "1101" or OPCODE = "1110" else
 resultSignal(15 downto 9) & resultSignal(0) when OPCODE = "1010" else
 resultSignal(14 downto 8) & resultSignal(0) when OPCODE = "1011" else
 resultSignal(7 downto 0);
Xsig_reverse <= Xsig(0) & Xsig(1) & Xsig(2) & Xsig(3) & Xsig(4) & Xsig(5) & Xsig(6) & Xsig(7);
X <= Xsig;
Zsig <= '1' when resultSignal = x"0000" else
 '0';
Z <= Zsig;
Coutsig <= resultSignal(8) when OPCODE = "0100" or OPCODE = "0110" or OPCODE = "1001" or OPCODE = "1110" else
 resultSignal(15) when OPCODE = "1011" else
 resultSignal(7) when OPCODE = "1100" or OPCODE = "1101" else
 '1' when OPCODE = "1111" and Ysig /= x"00" else
 '0';
Cout <= Coutsig;
Vsig <= '1' when OPCODE = "0101" and resultSignal(7) = '0' and A(7) = '1' and B(7) = '1' else
 '1' when OPCODE = "0101" and resultSignal(7) = '1' and A(7) = '0' and B(7) = '0' else
 '1' when OPCODE = "1110" and resultSignal(15) /= A(7) else
 '0';
V <= Vsig;
F_active <= Zsig OR Coutsig OR Vsig;
X_bin_pal <= '1' when Xsig = Xsig_reverse else
 '0';
X_prime <= '1' when Xsig = x"02" or Xsig = x"03" or Xsig = x"05" or Xsig = x"07" or Xsig = x"0B" or Xsig = x"0D" or Xsig = x"11" or Xsig = x"13" else
 '1' when Xsig = x"17" or Xsig = x"1D" or Xsig = x"1F" or Xsig = x"25" or Xsig = x"29" or Xsig = x"2B" or Xsig = x"2F" or Xsig = x"35" else
 '1' when Xsig = x"3B" or Xsig = x"3D" or Xsig = x"43" or Xsig = x"47" or Xsig = x"49" or Xsig = x"4F" or Xsig = x"53" or Xsig = x"59" else
 '1' when Xsig = x"61" or Xsig = x"65" or Xsig = x"67" or Xsig = x"6B" or Xsig = x"6D" or Xsig = x"71" or Xsig = x"7F" or (Xsig = x"83" and Nsig = '0') else
 '1' when (Xsig = x"89" or Xsig = x"8B" or Xsig = x"95" or Xsig = x"97" or Xsig = x"9D" or Xsig = x"A3" or Xsig = x"A7" or Xsig = x"AD") and Nsig = '0' else
 '1' when (Xsig = x"B3" or Xsig = x"B5" or Xsig = x"BF" or Xsig = x"C1" or Xsig = x"C5" or Xsig = x"C7" or Xsig = x"D3" or Xsig = x"DF") and Nsig = '0' else
 '1' when (Xsig = x"E3" or Xsig = x"E5" or Xsig = x"E9" or Xsig = x"EF" or Xsig = x"F1" or Xsig = x"FB") and Nsig = '0' else
 '0';
Nsig <= '1' when OPCODE = "0101" and Xsig(7) = '1' else
 '1' when OPCODE = "0111" and Ysig(7) = '1' else
 '1' when OPCODE = "1101" and Xsig(7) = '1' else
 '1' when OPCODE = "1110" and Xsig(7) = '1' else
 '0';
N <= Nsig;
end Behavioral;

• Dave Tweed: VHDL is very context sensitive. Can you give us a complete, self-contained source file that demonstrates the problem, instead of fragments of source code that have been pulled from your project?
• Roger C.: My VHDL is very rusty, but I would say that the problem is related to the sizes of the vectors you're multiplying. For example, dig1 is 4 bits and sig1000 is 16 bits. So probably the implementation is truncating bits when performing this operation.
• Roger C.: 252 in binary is 11111100 while 4046 is 111111001110. You are getting the correct value except for the missing last 4 bits.
• Roger C.: Because 16 bits multiplied by 4 bits could become, at most, a 20 bits number, this is I guess the size implicitly created by VHDL for storing the result of the multiplication. And when this implicit 20 bits number is assigned to a 16 bits array, the last 4 bits are lost. Could you try to resize opF to a 20 bits size vector?
• Roger C.: Alternatively you could "downsize" sig1000, sig100 and sig10 to a 12 bits size array.

Answer 1

16 bits multiplied by 4 bits can become, at most, a 20 bits number, this is apparently the size implicitly created by VHDL for storing the intermediate result of the multiplication. When this implicit 20 bits number is assigned to a 16 bits array, the last 4 bits are lost. And that's why Brian Drummonds's VHDL simulator complained about bound array error.

Because your final result actually fits in a 16 bits number, the best workaround is to reduce the number of bits of sig1000, sig100, sig10 to a 12 bits array.

• user16324: Let's be clear : implicit behaviour is not what VHDL is about. And now we know why. So much time has been wasted on this question, thanks to a tool problem. Unfortunately this tool defect does VHDL's reputation no favours, though a decent tool would have flattened this mountain into a minor speed bump.
• Roger C.: @BrianDrummond, sure, a good tool should had pointed this out, supposedly VHDL is a strongly typed language!

Answer 2

The first thing to do, as always, is to write a testbench. Here's a minimal one (below). Note I used a VHDL-2008 "to_string" call to simplify it.

Now, analyze, elaborate and simulate...

ghdl -a --std=08 alu.vhd
ghdl -a --std=08 alu_tb.vhd
ghdl -e --std=08 ALU_TB
./alu_tb
./alu_tb:error: bound check failure at alu.vhd:89
./alu_tb:error: simulation failed

So the problem is an array bound violation in line 89 ... which is, of course, opF <= ...

Which simulator can't diagnose a simple bounds violation? Ditch it and get a better one. Or find out how to turn the bound checks on. (If it's Xilinx ISIM, there's an option that should never have been off in the first place).

But now that you know it's a simple array bound error it should be dead easy to find and fix. Start by running just one digit at a time through that big calculation.

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;
entity ALU_TB is
end ALU_TB;
architecture TB of ALU_TB is
constant A : STD_LOGIC_VECTOR (7 downto 0) := X"46";
constant B : STD_LOGIC_VECTOR (7 downto 0) := X"40";
constant Cin : STD_LOGIC := '0';
constant OPCODE : STD_LOGIC_VECTOR (3 downto 0) := X"F";
signal Y : STD_LOGIC_VECTOR (7 downto 0);
signal X : STD_LOGIC_VECTOR (7 downto 0);
begin
DUT : entity Work.ALU
Port Map( 
 A => A,
 B => B,
 Cin => Cin,
 OPCODE => OPCODE,
 Y => Y,
 X => X,
 Z => open,
 Cout => open,
 V => open,
 F_active => open,
 X_bin_pal => open,
 X_prime => open,
 N => open);
check : process is
begin
 wait for 1 us;
 report "X = " & to_string(X) severity NOTE;
 report "Y = " & to_string(Y) severity NOTE;
end process check;
end TB;