Mark Mitchell wrote:
> > I attached a diff file for 14 files of the new structures
> > and documents. You and other maintainers are welcome to
> > check it. Thanks a lot!
>
> Thank you for posting this.
>
> Things about which I am clueless:
>
> What is the difference between a _Fract type and an _Accum type? I'm
> gathering that _Fract types have no integer part, i.e., they always
> represent a number between (-1,1), whereas _Accum types
> contain both an
> integer and a fractional part. Is there any other difference?
No other differences. For _Fract and _Accum, there are signed and unsigned
types. The unsigned _Fract and _Accum types are always >= 0.
>
> Substantive issues:
>
> + unsigned saturating_flag : 1; /* FIXME. This new flag
> increases the
> size of
> + tree_common by a full word. */
>
> We need to fix that, if it's true. Grab a bit out of
> tree_base::spare,
> if need be.
Yes. Will get one spare bit for saturating_flag.
>
> + /* Fixed-point types. */
> + if (targetm.fixed_point_supported_p ())
>
> I thought there was an emulation library for fixed-point operations?
> So, why would this be supported only on some targets? Is there a harm
> in supporting it everywhere? If there is, should it be enabled
> per-target, or via a configure-time option?
Right now, the fixed-point support is a configure-time option.
Each target can decide to support it or not. I think there is no
harm to enable for every target. But, each target needs to modify
tm.h tm.c tm-modes.def to set or change some target macros.
>
> I don't see any constant-folding code for fixed-point; is
> there no need
> for that? Is it somewhere else in the codebase?
I will post a diff file that has everything including
constant-folding code soon.
>
> Stylistic points:
>
> Every function needs a comment saying what it does, what
> every argument
> is, and what the return value is. You have some new
> functions that are
> missing comments.
Ok. Will fix it.
>
> + fixed-point <- real,
>
> Please write as "real to fixed point" to avoid ambiguity with
> the errors.
Ok. Will fix it.
>
> +DEF_RTL_EXPR(FIXED_ALL, "fixed_all", "e", RTX_UNARY)
>
> Why FIXED_ALL? That doesn't seem mnemonic.
It's hard to pick up a name. Maybe we will change it to
the following as Nigel suggested.
/* Conversions involving fractional fixed point types without
saturation, including:
fractional to fractional (of different precision),
signed integer to fractional,
fractional to signed integer,
floating point to fractional,
fractional to floating point. */
DEF_RTL_EXPR(FRACT_CONVERT, "fract_convert", "e", RTX_UNARY)
>
> > + 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_SIZE);
>
> Can we macroize some of this goo? There are several places
> in the patch
> where you have very long sequences of types, modes, etc. I would hope
> that by naming things consistently, we might be able to compact this a
> lot, and reduce the chance that we've got something wrong in just one
> place. Like:
>
> #define MAKE_FIXED_TYPE_NODE(KIND, WIDTH, SAT) \
> set_##WIDTH_##KIND_type_node = make_##SAT_...
>
> and then:
>
> #define MAKE_FIXED_TYPE_NODE_FAMILY(KIND) \
> MAKE_FIXED_TYPE_NODE(_short, KIND); \
> MAKE_FIXED_TYPE_NODE(_, KIND); \
> MAKE_FIXED_TYPE_NODE(_long, KIND); \
> MAKE_FIXED_TYPE_NODE(_long_long, KIND);
>
> and then:
>
> MAKE_FIXED_TYPE_NODE_FAMILY(sat, fract);
> MAKE_FIXED_TYPE_NODE_FAMILY(sat, accum);
>
> etc.
>
> That's not right, but you get the idea. :-)
Ok. We will try to shorten C code. Thanks!
Regards,
Chao-ying
