Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Thiago Jung Bauermann <bauerman@br.ibm.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: gdb-patches@sourceware.org
Subject: Re: [patch 2/2] Wrap-up expression support for DFP.
Date: Thu, 27 Dec 2007 07:40:00 -0000	[thread overview]
Message-ID: <1198705277.12907.39.camel@localhost.localdomain> (raw)
In-Reply-To: <uzlw4hw5f.fsf@gnu.org>

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.
-- 
[]'s
Thiago Jung Bauermann
Software Engineer
IBM Linux Technology Center


  parent reply	other threads:[~2007-12-26 21:41 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-20  5:55 [patch 0/2] Complete expression support for Decimal Floating Point Thiago Jung Bauermann
2007-12-20  5:55 ` [patch 1/2] Recognize DFP types in casts Thiago Jung Bauermann
2007-12-20 15:24   ` Daniel Jacobowitz
2007-12-20 16:48     ` Thiago Jung Bauermann
2007-12-20 16:54       ` Thiago Jung Bauermann
2007-12-20 16:57       ` Daniel Jacobowitz
2007-12-20 17:21         ` Thiago Jung Bauermann
2007-12-20  8:03 ` [patch 2/2] Wrap-up expression support for DFP Thiago Jung Bauermann
2007-12-21 16:12   ` Eli Zaretskii
2007-12-21 16:30     ` Andreas Schwab
2007-12-21 16:38       ` Eli Zaretskii
2007-12-27  7:40     ` Thiago Jung Bauermann [this message]
2007-12-29 11:21       ` Eli Zaretskii
2008-01-02 20:07         ` Thiago Jung Bauermann
2008-01-05 11:30           ` Eli Zaretskii
2008-01-07 16:20             ` Thiago Jung Bauermann
2008-01-07 17:09               ` Thiago Jung Bauermann
2008-01-07 21:09               ` Eli Zaretskii
2008-01-07 22:39                 ` Thiago Jung Bauermann
2007-12-28  6:16     ` Thiago Jung Bauermann
2007-12-29 12:53       ` Eli Zaretskii
2008-01-02 16:42         ` Thiago Jung Bauermann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1198705277.12907.39.camel@localhost.localdomain \
    --to=bauerman@br.ibm.com \
    --cc=eliz@gnu.org \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox