Re: [RFC] decimal float point patch based on libdecnumber: gdb patch

[Wu Zhou <woodzltc at cn dot ibm dot com>]

Hi Daniel,

Thanks for reviewing this patch.

Quoting Daniel Jacobowitz <drow@false.org>:

> On Thu, Jun 22, 2006 at 07:36:17AM +0800, Wu Zhou wrote:
> > Appended is the patch.  I had tested it with the latest gcc-4.2 cvs tree
> > on x86, with option "--enable-decimal-float". All the 121 tests passed.
> > Please review and comment.  Thanks a lot.
>
> Sometimes you use DECDBL and sometimes you use DECDOUBLE and sometimes
> you use DECFLOAT and sometimes you use DECFLT.  Let's pick one or the
> other and use it everywhere.  This makes it easier to search for
> code that knows about decimal float.  "decfloat" might be more
> readable?

I was using these different names because I wa simulating what  
float/double/long double are using.

DECDBL is not used anywhere, but decdbl is. :-)
It is only used in write_exp_elt_decdblcst and its reference.  That is  
to simulate write_exp_elt_dblcst. I am not very sure why gdb named it  
dbl though.

DECDOUBLE is used in OP_DECDOUBLE and value_from_decdouble.  This is  
to simulate OP_DOUBLE and value_from_double. All three kinds of float  
constants and variables are handled by them.  So I am using them for  
DECDOUBLE too.

DECFLT is only used in TYPE_CODE_DECFLT.  This is to simulate  
TYPE_CODE_FLT, which is used to refer to types of float, double, long  
double at the same time. The length can differentiate them.

DECFLOAT is used in parsing and fundamental type representation  
dwarf2read.c and C language handling.  such as FT_DECFLOAT,  
FT_DBL_PREC_DECFLOAT  and FT_EXT_PREC_DECFLOAT.  This is the same what  
FLOAT is used in these scanario.

My point is to keep these usage so as they are consistent with how  
flaot/double is handled.  What is your idea on this?

> Why is reverse_dfp necessary?  It says GDB uses the opposite endianness
> from decNumber, but I don't see why.  This might be a related problem,
> but are you accomodating target endianness when you read values from
> the target?  If not, a cross debugger will do the wrong thing.

We are using an array of gdb_byte to represent decimal values.  The  
first byte is the most significant. This might be not the same as the  
underlying byte order of the architecuture.  In this kind of  
situation, reverse_dfp is needed to do conversion.

But in big-endian machine, this might be not needed. I will try a test  
on ppc64.

I am now also thinking that the place where reverse_dfp is defined is  
not very good. Maybe we can put them into decimal_to_string /  
decimal_from_string. Then other gdb code don't need to know the  
existence of that.

BTW, I am not very sure what you means by saying "are you accomodating  
target endianness when you read values from the target?". I guess you  
mean that I _need_ to accommodate the endianness when reading values  
from the target, right? Then putting some code into  
decimal_from_string / decimal_to_string to convert the byte order if  
needed is right what you want to see, right?  Any advices?

> > Index: expression.h
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/expression.h,v
> > retrieving revision 1.18
> > diff -u -r1.18 expression.h
> > --- expression.h	17 Dec 2005 22:33:59 -0000	1.18
> > +++ expression.h	21 Jun 2006 23:08:51 -0000
> > @@ -327,6 +327,11 @@
> >      /* A F90 array range operator (for "exp:exp", "exp:", ":exp"   
> > and ":").  */
> >      OP_F90_RANGE,
> >
> > +    /* OP_DECDOUBLE is followed by a type pointer in the next exp_element
> > +       and a dec long constant value in the following exp_element.
> > +       Then comes another OP_DECDOUBLE.  */
> > +    OP_DECDOUBLE,
> > +
>
> A comment on the format of the data would be nice here.  Is it in
> target byte order?

The first byte is the most significant byte.  I don't have code to  
make it into the target byte order yet.  reverse_dfp can partially do  
that.  But it need to be more general. And I am planning to add some  
code into decimal_to_string / decimal_from_string to test the target  
byte order and determine if the conversion is needed. What is your  
thought?

> > Index: c-valprint.c
> > ===================================================================
> > RCS file: /cvs/src/src/gdb/c-valprint.c,v
> > retrieving revision 1.39
> > diff -u -r1.39 c-valprint.c
> > --- c-valprint.c	18 Jan 2006 21:24:19 -0000	1.39
> > +++ c-valprint.c	21 Jun 2006 23:08:52 -0000
> > @@ -442,6 +442,17 @@
> >  	}
> >        break;
> >
> > +    case TYPE_CODE_DECFLT:
> > +      if (format)
> > +	{
> > +	  print_scalar_formatted (valaddr + embedded_offset, type,   
> > format, 0, stream);
> > +	}
> > +      else
> > +	{
> > +	  print_decimal_floating (valaddr + embedded_offset, type, stream);
> > +	}
> > +      break;
> > +
> >      case TYPE_CODE_METHOD:
> >        {
> >  	struct value *v = value_at (type, address);
>
> You don't need the braces in this; please omit them when you don't need
> to.  It saves a level of indentation, which generally makes them easier
> to read.

Thanks.  I will delete them when I update my patch.

Regards
- Wu Zhou