Fu, Chao-Ying 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?
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.
+ /* 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?
I don't see any constant-folding code for fixed-point; is there no need
for that? Is it somewhere else in the codebase?
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.
+ fixed-point <- real,
Please write as "real to fixed point" to avoid ambiguity with the errors.
+DEF_RTL_EXPR(FIXED_ALL, "fixed_all", "e", RTX_UNARY)
Why FIXED_ALL? That doesn't seem mnemonic.
> + 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. :-)
Thanks,
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
