Weird VHDL Multiplier Problem

--- Quote Start ---

Hello all,

I was recently given reign over a DE0-Nano board by my research adviser to turn into a interference fringe counter of sorts but I am having some problems programming multiplication in VHDL and cannot seem to find anyone with similar errors online (my adviser also seems to be befuddled as to what, exactly, is wrong). I am trying to multiply two 32 bit unsigned numbers, one of which is a constant and the other a signal from a button press(for testing purposes), and then later convert it to bcd and put it on a 7-seg display. I have successfully implemented a binary to bcd and bcd to 7seg display code and use a set of seven segmented displays to check the outputs for my multiplier. For certain constant values, such as "100...0" or "111...1" or "100...01", the multiplier seems to work properly. However, if the multiplier tries to multiply a 'sandwiched' zero (like the number 13 which has the binary representation 1101), it will fail to produce the correct output. For example if the constant is 13, the display reads the following solutions for each multiplication: 0x13=0; 1x13=13; 2x13=54; 3x13=39; 4x13=76; 5x13=65; etc... As you can see, some of the values are correct, however others are plain wrong. (NOTE: the pattern for 13 seems typical... the even multiples are off, although I do not know for sure if this is always the case).

Here's my code for the multiplier:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
entity mult_32 is
port (
  a: in  unsigned(31 downto 0);
  c   : out unsigned(63 downto 0);
  clear : in std_logic
);
constant b: unsigned(31 downto 0):=to_unsigned(13,32); 
end entity mult_32;
architecture rtl of mult_32 is
signal a_reg : unsigned(31 downto 0);
signal b_reg: unsigned(31 downto 0);
signal c_reg : unsigned(63 downto 0);
begin
    process (clear)
    begin
        if (clear ='1') then
            -- Reset all register data to 0
            a_reg <= (others => '0');
            b_reg <= (others => '0');
            c_reg <= (others => '0');
        else
            -- Store input and output values in registers
            a_reg <= a;
            b_reg <= b;
            c_reg <= a_reg * b_reg;    
        end if;
    end process;
    
c<=c_reg;
end rtl;

I am new to coding in VHDL and believe that I have likely made some sort of syntax error, type cast conversion error (although I have been careful to use unsigned for everything) or other newbie mistake. If it is relevant, I am writing and compiling my code in Quratus II version 13 (64 bit version).

Thanks for your help.

--- Quote End ---

You will first need to add clock to your system and see. Your design looks combinatorial and result will not be saved, the delay variations will cause trouble. Yoy have named your signal reg but it is not inferred as reg.

--- Quote Start ---

I am having some problems programming multiplication in VHDL

--- Quote End ---

As Kaz points out, you have not created a synchronous process. In addition, I would advise you to create a simulation of your logic, as then you would be able to state "It works in simulation, but not in hardware", and from that we can point out what can go wrong in hardware :)

Read through this tutorial on using FPGAs for doing signal processing, look at the slides, and download the code;

http://www.ovro.caltech.edu/~dwh/correlator/pdf/esc-104paper_hawkins.pdf

http://www.ovro.caltech.edu/~dwh/correlator/pdf/esc-104slides_hawkins.pdf

http://www.ovro.caltech.edu/~dwh/correlator/pdf/esc-104code_hawkins.zip

The tutorial was tested on the DE0-nano. One of the examples shows how you can reduce the width of your product, eg., you do not need to keep all 64-bits of the 32-bit x 32-bit product if you do not want to.

You'll also see how SignalTap II can be used in hardware to "see" what is going on; which can help with "it does not work in hardware" ... :)

Cheers,

Dave

Contrary to Kaz' and Dave's suggestions, you don't need to create a synchronous process.

For a combinatorial process Quartus II will complain about a, a_reg, b_reg not being in the sensitivity list but will produce a valid circuit. Modelsim will keep silent about the sensitivity list but may fail to simulate correctly (IMHO).

Is this the exact code of your project, or a write-up? It would be interesting to see all the code.

Yes it's a valid circuit and asynchronous. The error can be expected in the other part of the design.

In terms of general FPGA design methodology, it would be reasonable to switch to a synchronous design, but it's not necessarily required. If the total design is asynchronous and doesn't latch data at the wrong time, it should manage to show the result on a 7-segment display.

--- Quote Start ---

Yes it's a valid circuit and asynchronous. The error can be expected in the other part of the design.

In terms of general FPGA design methodology, it would be reasonable to switch to a synchronous design, but it's not necessarily required. If the total design is asynchronous and doesn't latch data at the wrong time, it should manage to show the result on a 7-segment display.

--- Quote End ---

In practice, you cannot design proper projects in asynchronous logic in FPGAs. For student level miniature designs you may but and I see it wrong advise beginners to have their tiny circuit in asynchronous logic. If it was possible all the fuss about fmax would have disappeared and TimeQuest thrown in the bin.

The fpga design is based on rtl when a register exist on either side of logic cloud.

Moreover for testing you get cleaner waveforms when you are sampling your outputs at clock edge only away from delay effect.

Hi guys,

All good points ... keep in mind the original comment

--- Quote Start ---

a signal from a button press

--- Quote End ---

A button should be debounced, and that would typically be done with logic that uses a clock, and hence it would be "good design practice" to also use a clock for the multiplier.

Cheers,

Dave

--- Quote Start ---

For example if the constant is 13, the display reads the following solutions for each multiplication: 0x13=0; 1x13=13; 2x13=54; 3x13=39; 4x13=76; 5x13=65; etc... As you can see, some of the values are correct, however others are plain wrong.

--- Quote End ---

The symptom nevertheless is that (so far we can see) the results for even inputs are incorrect. Not that the button is bouncy. I repeat that we very probably don't see the real code from dropneedle. That what he has shown is basically correct, and supporting Frank, the problem must lie elsewhere in his code.

Hi Josy,

--- Quote Start ---

the problem must lie elsewhere in his code

--- Quote End ---

Yeah, sure ...

The original post has the comment:

--- Quote Start ---

convert it to bcd and put it on a 7-seg display

--- Quote End ---

So that's the first place I'd recommend looking.

Of course if the code was written along with a testbench, then we probably wouldn't need to have this discussion :)

Cheers,

Dave