VHDL why does this overflow

[alank2]

Defined as:

   signal   param1: integer range 0 to 127 :=0;
   signal   param2: integer range 0 to 127 :=0;
   signal   param3: integer range 0 to 127 :=0;
   signal   param4: integer range 0 to 127 :=0;

Being loaded here:

                  if paramCount=1 then -- ESC[{param1}
                     param1 <= (param1 * 10) + (to_integer(unsigned(dispByteLatch))-48);
                  elsif paramCount=2 then -- ESC[{param1};{param2}
                     param2 <= (param2 * 10) + (to_integer(unsigned(dispByteLatch))-48);
                     --param2<=21;
                  elsif paramCount=3 then -- ESC[{param1};{param2};{param3}
                     param3 <= (param3 * 10) + (to_integer(unsigned(dispByteLatch))-48);
                  elsif paramCount=4 then -- ESC[{param1};{param2};{param3};{param4}
                     param4 <= (param4 * 10) + (to_integer(unsigned(dispByteLatch))-48);
                  end if;

When paramX are only 7 bit, they fail, but at 8 bit they calculate properly.  When 7 bit if I sent a "20" it would end up 2 or 3 for some reason in param2.

[Sal Ammoniac]

Seems like you're going to overflow a 7 bit integer if paramX is greater than 12 (and even less if dispByteLatch is larger than 48. In fact, paramX is guaranteed to overflow any range if you multiply it by ten.

[hamster_nz]

Is what you are implementing an ASCII encoded decimal to binary conversion?

There is most likely better ways to achieve it... or at least code it. For example, if you know the range of your inputs are valid ASCII digits, then you don't need to subtract 48. Just use the low four bits.

[alank2]

Yes, this is someone else's VHDL that does ANSI escape sequence handling.

If the first digit comes along is a 2 and it begins at a 0, then it would be 0*10+(50-48)=2
Then when the 0 comes along, 2*10+(48-48)=20
Right?

[hamster_nz]

Yes, this is someone else's VHDL that does ANSI escape sequence handling.

If the first digit comes along is a 2 and it begins at a 0, then it would be 0*10+(50-48)=2
Then when the 0 comes along, 2*10+(48-48)=20
Right?

That's how it reads to me... but if '@' (ASCII code 0x40) comes along rather than a '0', you get different results if you use:

   param2 <= (param2 * 10) + (to_integer(unsigned(dispByteLatch))-48);

vs the more efficient:

  param2 <= (param2 * 10) + to_integer(unsigned(dispByteLatch(3 downto 0));

The benefit of the second is that you don't have the problem that "(to_integer(unsigned(dispByteLatch))-48)" is an 8-bit number, and that the '-' underflow, and generate an intermediate value > 127.

Hey... try this (note the moving of the 2nd to last bracket):
   param2 <= (param2 * 10) + (to_integer(unsigned(dispByteLatch)-48));

...where you are subtracting 48 from the 8-bit unsigned value, rather than from the (32-bit?) integer.

This somewhat reinforces my biases against using INTEGER types... this would work perfectly well if everything was just UNSIGNED...

      signal param2 : unsigned(6 downto 0);
      param2 <= (param2 * 10) + unsigned(dispByteLatch(3 downto 0); -- just use the low bits

Or if you are being defensive you can validate it to be an ASCII char before you use it:

   signal param2 : unsigned(6 downto 0);
   if dispByteLatch(7 downto 4) = x"3" and dispByteLatch(3 downto 0) < x"A" then
     ....
      param2 <= (param2 * 10) + unsigned(dispByteLatch(3 downto 0); -- just use the low
      ....
   end if;

[alank2]

>  param2 <= (param2 * 10) + unsigned(dispByteLatch(3 downto 0); -- just use the low bits

I get an error expression has 14 elements, but must have 7 elements on this.

WHY are there to different methods/types?  "integer range 0 to 127" vs "unsigned(6 downto 0)".  Or what about "std_logic_vector(6 downto 0)".  Is there a reason to use one type over another?  Do each have advantages?  Is there a penalty when you have to convert one type to another?

[jbb]

Hi there

I tend to use std_logic_vector(X downto 0), for two reasons.

• The std_logic supports extra values such as X (unknown) and U (uninitialised), which can be very helpful in simulation. (Also others - std_logic is very helpful).  Of course, in the silicon it's going to be 1 or 0, but in simulation you can easily pickup undefined values or weird behaviours.
• I like the (X downto 0) arrangement because then bit X is the most significant bit.

Cheers
jbb

[alank2]

The dispByteLatch is tested to be a valid number '0'(48)-'9'(57).

dispByteLatch is:
signal   dispByteLatch: std_logic_vector(7 DOWNTO 0);

param1 is:
signal   param1: integer range 0 to 127 :=0;

The formula is:

param1 <= param1 * 10 + (to_integer(unsigned(dispByteLatch))-48); --FAIL #1
param1 <= param1 * 10 + (to_integer(unsigned(dispByteLatch)-48)); --FAIL #2
param1 <= param1 * 10 + to_integer(unsigned(dispByteLatch(3 downto 0))); --SUCCESS #3

Changing param1 to 0 to 255 will also fix it with the first original formula.

Question # 1 - What I am trying to understand is why #1 and #2 fail, but #3 works.  If unsigned(dispByteLatch) is always >=48 and <=57, then subtracting 48 from it should be ok, right?

Question # 2 - which is more efficient, using integers or just avoiding them?  I found out that avoiding them adds a bit of complication in that when you multiply unsigned values, you end up with more bits than are necessary and have to use resize to take the right X number of bits that you want.

[hamster_nz]

>  param2 <= (param2 * 10) + unsigned(dispByteLatch(3 downto 0); -- just use the low bits

I get an error expression has 14 elements, but must have 7 elements on this.

WHY are there to different methods/types?  "integer range 0 to 127" vs "unsigned(6 downto 0)".  Or what about "std_logic_vector(6 downto 0)".  Is there a reason to use one type over another?  Do each have advantages?  Is there a penalty when you have to convert one type to another?

Now get into the meat of the problem.

INTEGERS are conceptually numbers - you can't ask for the 2nd binary digit of an integer directly. They support the range of a signed 32-bit number.

An individual STD_LOGIC signals must have one of nine different values:

'U': uninitialized. This signal hasn't been set yet.
'X': unknown. Impossible to determine this value/result.
'0': logic 0
'1': logic 1
'Z': High Impedance
'W': Weak signal, can't tell if it should be 0 or 1.
'L': Weak signal that should probably go to 0
'H': Weak signal that should probably go to 1
'-': Don't care.

A STD_LOGIC_VECTOR is an array of STD_LOGIC. This allows you to address individual 'slices' of the array, or manipulate individual bits with logical functions. However you can't perform math on them - they are just a list of STD_LOGIC values. And as an array, they have a length (and other) attributes that an INTEGER doesn't have.

UNSIGNED and SIGNED is an extension of STD_LOGIC_VECTOR, the difference being that you can do math with with them. When you do math, the elements in the vector take on the a place value of 2^n, except for SIGNED when the highest bit has a value of -2^n. (where n is zero for the leftmost bit).

There is no penalty when you convert from one type to the other (at least when running in hardware). The more explicit you are about the data type you want, the better the chance that you will get what you actually intended and not what the tools are forced to chose for you.

The error you are getting is because you are throwing away the most significant bits when you store the result back into param2 - when you multiply by 10, you are adding at least 4 more bits of data (for example, 127*10 = 1027, which needs 11 bits). 

It is the language being useful and telling you that you are throwing away useful information. It doesn't know you are limiting the input to two digits of ASCII, so only values of "0000000" to "1100011" are expected.

[alank2]

But, if an overflow never occurs, why wouldn't these work:

param1 <= param1 * 10 + (to_integer(unsigned(dispByteLatch))-48); --FAIL #1

param1 <= param1 * 10 + (to_integer(unsigned(dispByteLatch)-48)); --FAIL #2

Expanding param1 to 8 bits does also fix the issue.

[alank2]

Also, wouldn't integers be more efficient since they only have two states vs. the 8 or 9 stages of the signed/unsigned type for each bit?

edit : param1 <= param1 * 10 + to_integer(unsigned(dispByteLatch))-48;

ALSO WORKS.

Taking out the parenthesis before to_integer and after 48 also fixes it.

Combining those two, the to_integer and the -48 - is it because dispByteLatch is 8 bits and could be 0-255 that this is an issue even though it won't ever be lower than 48?

I've love to understand the logic of this...

[hamster_nz]

Question # 2 - which is more efficient, using integers or just avoiding them?  I found out that avoiding them adds a bit of complication in that when you multiply unsigned values, you end up with more bits than are necessary and have to use resize to take the right X number of bits that you want.

When you use integers you usually also end up with more bits than are necessary in your design (as it has to produce the correct output for any input), it's just that you don't see it explicitly in your code. This is most apparent when dealing with signed values, that need to be sign-extended before they are used.

If I was writing the code you are having trouble with, I would most likely say something like:

   var v : unsigned(x'high+3 downto 0);
  ....
   v := x*10 + y(3 downto 0); -- this has three more bits than 'x'
   x <= v(x'range);                   -- drop excess bits.
   ...
 
(not that I've tested it)

I'll ponder the other points tonight, but it looks to be something funky in the type conversions...

[Ice-Tea]

In addition, keep in mind multiplications in code like that suck FPGA resources down the drain. Works fine for simulation, but...

[alank2]

So guys, help me understand something - is it possible for VHDL code to work in some conditions and not others?  Let me explain - this code I've been talking about generates a VGA display and it can process characters and ANSI sequences properly.  I have seen it work even with the statement that fails above, then I've seen it work with the statement tweaked as above, then I've seen that statement fail in other conditions.  I am using Grant Searle's multicomp and one of the warning messages when I compile in Quartus II is something about the timing constraints not being specified.  Could timing have something to do with why it works sometimes and not at others?

[alank2]

Any idea on fitter options in Quartus that I can tweak to make it more stable? I've started with Grant's project, it works.  I turned it into the CP/M version and it breaks.  Then I changed the way the CP/M version disables the rom and it works again.  Then I put in the modified SD card module, completely different from the vga module, and it breaks again!

[Ice-Tea]

Off coarse timing constraints have to be specificatied. If you have a 100 mc clock and you didn't specify it, you fitter will dump everythingin the device, have a 23 mc max and call it a day.

[alank2]

I'm trying to read about timing constraints right now.  The device has a 50 Mhz clock.  All the others are generated from it.

create_clock -name "clk" -period 20.000ns [get_ports {clk}]

I can't this to be accepted:

create_generated_clock \
    -divide_by 50 \
    -source [get_ports {clk}] \
    -name sdClock \
    [get_pins {DIV|q}]

what is q ?

[alank2]

This is the warning I am trying to eliminate:

Warning (332060): Node: cpuClock was determined to be a clock but was found without an associated clock assignment.

It runs at 1/5th the clk...

[alank2]

Problem is finally solved - it could not handle 50 MHz.  I fed a 25 MHz clock to a pin and redirected the clock pin to it and made adjustments for the clock speed change.  It works perfectly now.

[jbb]

Glad you found your problem.

Also, wouldn't integers be more efficient since they only have two states vs. the 8 or 9 stages of the signed/unsigned type for each bit?

This is a good question.  The answer is that in silicon (including FPGA) a given signal will turn out to be a 1 or a 0 (excepting IO ports which can do special stuff), so there shouldn't be an efficiency penalty.

The advantage comes during simulation time - you can detect weird states (especially Uninitialised (U)) that a basic integer type would hide from you, or X when you have contention.  Or you can assign X to a signal to show that you don't have a result this clock cycle, and then see if the X propagates to anything useful.

Cheers
jbb