Re: [patch 2/2] Wrap-up expression support for DFP.

Thiago Jung Bauermann
2007-12-26T21:41:31Z

Hi,

On Fri, 2007-12-21 at 18:04 +0200, Eli Zaretskii wrote:
> > Date: Thu, 20 Dec 2007 03:49:28 -0200
> > From: Thiago Jung Bauermann <bauerman@br.ibm.com>
> >
> > - doesn't support conversion of 64-bit integers to decimal float,
> > because of libdecnumber limitation;
> > - error checking in decimal float operations ignore underflow, overflow
> > and divide by zero to imitate binary float implementation;
>
> These limitations should be documented in the manual, I think: they
> will affect GDB users, right?

Actually, the second one is an implementation choice, not really a
limitation. But I agree that both should be in the manual. I will update
the patch to include a documentation addition.

> > - decimal_from_floating is not very nice because it uses sprintf, but
> > I couldn't think of a better way.
>
> Yuck! I don't know anything about libdecnumber, but is there _really_
> no other way? What about producing the two parts before and after the
> decimal point as integers, then combine them with arithmetic
> operations?

I took (quite) some time to implement something following those lines,
and came up with a naÃ¯ve function which was able to convert binary float
to decimal float using arithmetic operations. It's less precise than the
printf version, though:

(gdb) p (_Decimal128) 1.2
$1 = 1.1999999997206032276153564453125

Compared to the printf one:

(gdb) p (_Decimal128) 1.2
$1 = 1.19999999999999995559107901499

Besides, it has problems with very small numbers (needs better stop
criterion) or very large ones (right now it has a limit at INT_MAX).

It's possible that those limitations can be removed or attenuated, but
I'm not experienced with numerical algorithms so it would take me great
work, and I don't think it would pay off. Going through the string
representation is not elegant, but provides good results and solves the
problem at hand with no drawbacks since snprintf is provided by
libiberty (thanks for the pointer).

> > +static void
> > +set_decnumber_context (decContext *ctx, int len)
> > +{
> > + switch (len)
> > + {
> > + case 4:
> > + decContextDefault (ctx, DEC_INIT_DECIMAL32);
> > + break;
> > + case 8:
> > + decContextDefault (ctx, DEC_INIT_DECIMAL64);
> > + break;
> > + case 16:
> > + decContextDefault (ctx, DEC_INIT_DECIMAL128);
> > + break;
> > + }
>
> I don't think our coding style includes mixed-case symbols. Are these
> library functions?

Yes, they are from libdecnumber. The library is also used by GCC to
provide decimal floating point support.

> > +void
> > +decimal_from_floating (struct value *from, gdb_byte *to, int len)
> > +{
> > + /* The size of this buffer is a conservative guess: assumes an 128-bit
> > + long double could have 40 decimal digits, plus 4 for exponent, plus
> > + 3 for mantissa and exponent signs and 'E', plus '\0'. */
> > + char buffer[48];
>
> Are the sizes of the exponent, mantissa, etc. available as parameters
> from libdecnumber? If so, it would be better to use them explicitly
> in allocating buffer[] off the stack (e.g., with alloca). That way,
> if your assumptions will ever become incorrect, the code will adapt
> automatically.

Actually, these numbers refer to the double representation in the target
system, not to the decimal float representation used in libdecnumber. By
the way, since libiberty also has an implementation for asprintf, I
could do away with these estimates and just use this function instead.

> > + /* We cannot use snprintf here because it is defined only in C99.
>
> We have portable substitutes for snprintf, I think. Take a look at
> libiberty, for example.

Yes, it is there. I didn't find any .h, though, so I don't really know
the proper way to use them. Maybe the configure system will set things
up behind my back for the functions which are not provided by the host?

> > + sprintf (buffer, "%Lf", value_as_double (from));
>
> If we must live with going through the printed representation, at the
> very least please use "%.30Lf", so as not to lose precision due to the
> default number of significant digits produced under "%Lf", arbitrarily
> chosen by the libc implementation.

Done, thanks.

>
> > + /* This is an ugly way to do the conversion, but libdecnumber does
> > + not offer a direct way to do it. */
> > + decimal_to_string (from, len, buffer);
> > + return atof (buffer);
>
> Isn't strtold better here?

It's C99 and not in libiberty.