This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [fixed-point] Patch to parse _Fract, _Accum, _Sat
- From: "Chao-ying Fu" <fu at mips dot com>
- To: "Steven Bosscher" <stevenb dot gcc at gmail dot com>
- Cc: "Thekkath, Radhika" <radhika at mips dot com>, "Stephens, Nigel" <nigel at mips dot com>, <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 8 Nov 2006 14:38:52 -0800
- Subject: Re: [fixed-point] Patch to parse _Fract, _Accum, _Sat
- References: <3CB54817FDF733459B230DD27C690CEC012BB164@Exchange.mips.com> <200611080624.38998.steven@gcc.gnu.org>
- Reply-to: "Chao-ying Fu" <fu at mips dot com>
Hello,
Thanks for your review!
> > + unsigned saturating_flag : 1;
>
> You probably have noticed that the flags are grouped in sets of 8 bits,
> and that there are four groups of 8 bits. You've added another bit,
> and increased the size of tree_common by a full word. Is that change
> on purpose?
About saturating_flag, if I cannot add a new flag in either tree_common
or tree_type, I will need to use lang_flag_2 in c-tree.h
Ex:
#define C_TYPE_SATURATING(TYPE) TYPE_LANG_FLAG_2 (TYPE)
> > + TI_SAT_SFRACT_TYPE,
> > + TI_SAT_FRACT_TYPE,
> > + TI_SAT_LFRACT_TYPE,
> > + TI_SAT_LLFRACT_TYPE,
> > + TI_SAT_USFRACT_TYPE,
> > + TI_SAT_UFRACT_TYPE,
> > + TI_SAT_ULFRACT_TYPE,
> > + TI_SAT_ULLFRACT_TYPE,
>
> The usual practice in GCC is to use machine modes instead of C type
> names. I'm not sure what that would exactly look like in this case,
> but could you have e.g. TI_SAT_UFRACTSI_TYPE instead of
> TI_SAT_ULFRACT_TYPE?
These TI*s are for the standard named data types. Their machine modes
will be different on different backends (like the "long" type ).
However, I can change the names for the nameless data types.
Ex:
TI_QQ_TYPE -> TI_FRACTQQ_TYPE
TI_UQQ_TYPE -> TI_UFRACTQQ_TYPE
...
> > +static tree
> > +make_or_reuse_fract_type (unsigned size, int unsignedp, int satp)
>
> New function, needs a comment before it. Likewise for all other new
> functions.
Yes. I will add comments.
> > @@ -1523,7 +1532,12 @@
> > _Decimal32
> > _Decimal64
> > _Decimal128
> > + _Fract
> > + _Accum
> >
> > + type-qualifier:
> > + _Sat
> > +
>
> You've put _Sat under GNU extensions. I see that the dfp type specifiers
> are under that header too but I thought they come from some standard.
> Anyway, if _Sat comes from a standard, it deserves its own heading here
> (and perhaps you could point to the relevant standards in a comment
> everywhere else where you add things for fixed-point parsing??).
_Fract, _Accum and _Sat are from this DTR.
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1021.pdf
I will add this link to the comment.
> > + /* Fixed-point types. */
> > + sat_short_fract_type_node = make_sat_signed_fract_type
(SHORT_FRACT_TYPE_SIZE);
> > + sat_fract_type_node = make_sat_signed_fract_type (FRACT_TYPE_SIZE);
> > + sat_long_fract_type_node = make_sat_signed_fract_type
(LONG_FRACT_TYPE_SIZE);
> > + sat_long_long_fract_type_node = make_sat_signed_fract_type
(LONG_LONG_FRACT_TYPE
>
> Should this not be conditional on targetm.fixed_point_supported_p() ?
Yes. I will put the test.
Thanks a lot!
Regards,
Chao-ying