Tamar Christina <tamar.christ...@arm.com> writes: > Hi All, > > This patch adds an optimized route to the fpclassify builtin > for floating point numbers which are similar to IEEE-754 in format. > > [...]
I might be the least competent person on this list to review this patch
but nevertheless read it out of interest and stumbled over a comment
that I believe could be improved for clarity.
diff --git a/gcc/real.h b/gcc/real.h
index
59af580e78f2637be84f71b98b45ec6611053222..36ded57cf4db7c30c935bdb24219a167480f39c8
100644
--- a/gcc/real.h
+++ b/gcc/real.h
@@ -161,6 +161,15 @@ struct real_format
bool has_signed_zero;
bool qnan_msb_set;
bool canonical_nan_lsbs_set;
+
+ /* This flag indicates whether the format can be used in the optimized
+ code paths for the __builtin_fpclassify function and friends.
+ The format has to have the same NaN and INF representation as normal
+ IEEE floats (e.g. exp must have all bits set), most significant bit
must be
+ sign bit, followed by exp bits of at most 32 bits. Lastly the
floating
+ point number must be representable as an integer. The base of the
number
+ also must be base 2. */
+ bool is_binary_ieee_compatible;
const char *name;
};
My first issue is that
> The format has to have the same NaN and INF representation as normal
> IEEE floats
is kind of an oxymoron because NaNs and INFs are not "normal" IEEE
floats.
Second,
> the floating point number must be representable as an integer
is also somewhat misleading because it could be interpreted in the
(obviously nonsensical) way that the floating-point *values* have to be
integral. (I think it should be possible to *interpret* not *represent*
them as integers.)
So I would like to suggest the following rewording.
> This flag indicates whether the format is suitable for the optimized
> code paths for the __builtin_fpclassify function and friends. For
> this, the format must be a base 2 representation with the sign bit as
> the most-significant bit followed by (exp <= 32) exponent bits
> followed by the mantissa bits. It must be possible to interpret the
> bits of the floating-point representation as an integer. NaNs and
> INFs must be represented by the same schema used by IEEE 754. (NaNs
> must be represented by an exponent with all bits 1, any mantissa
> except all bits 0 and any sign bit. +INF and -INF must be represented
> by an exponent with all bits 1, a mantissa with all bits 0 and a sign
> bit of 0 and 1 respectively.)
I Hope this is clearer and still matches what the comment was supposed
to say.
--
OpenPGP:
Public Key: http://openpgp.klammler.eu
Fingerprint: 2732 DA32 C8D0 EEEC A081 BE9D CF6C 5166 F393 A9C0
signature.asc
Description: PGP signature
