On Fri, 11 Nov 2016, Tamar Christina wrote:

> is_infinite and is_zero have been created. This patch also introduces two new
> intrinsics __builtin_iszero and __builtin_issubnormal.

And so the ChangeLog entry needs to include:

        PR middle-end/77925
        PR middle-end/77926

(in addition to PR references for whatever PRs for signaling NaN issues 
are partly addressed by the patch - those can't be closed however until 
fully fixed for all formats with signaling NaNs).

> NOTE: the old code for ISNORMAL, ISSUBNORMAL, ISNAN and ISINFINITE had a
>       special case for ibm_extended_format which dropped the second part
>       of the number (which was being represented as two numbers internally).
>       fpclassify did not have such a case. As such I have dropped it as I am
>       under the impression the format is deprecated? So the optimization isn't
>       as important? If this is wrong it would be easy to add that back in.

It's not simply an optimization when it involves comparison against the 
largest normal value (to determine whether something is finite / infinite) 
- in that case it's needed to avoid bad results on the true maximum value 
(mantissa 53 1s, 0, 53 1s) which can occur at runtime but GCC can't 
represent internally because it internally treats this format as having a 
fixed width of 106 bits.

> Should ISFINITE be change as well? Also should it be SUBNORMAL or DENORMAL?

It's subnormal.  (Denormal was the old name in IEEE 754-1985.)

> And what should I do about Documentation? I'm not sure how to document a new
> BUILTIN.

Document them in the "Other Builtins" node in extend.texi, like the 
existing classification built-in functions.

> diff --git a/gcc/builtins.def b/gcc/builtins.def
> index 
> 219feebd3aebefbd079bf37cc801453cd1965e00..e3d12eccfed528fd6df0570b65f8aef42494d675
>  100644
> --- a/gcc/builtins.def
> +++ b/gcc/builtins.def
> @@ -831,6 +831,8 @@ DEF_EXT_LIB_BUILTIN    (BUILT_IN_ISINFL, "isinfl", 
> BT_FN_INT_LONGDOUBLE, ATTR_CO
>  DEF_EXT_LIB_BUILTIN    (BUILT_IN_ISINFD32, "isinfd32", BT_FN_INT_DFLOAT32, 
> ATTR_CONST_NOTHROW_LEAF_LIST)
>  DEF_EXT_LIB_BUILTIN    (BUILT_IN_ISINFD64, "isinfd64", BT_FN_INT_DFLOAT64, 
> ATTR_CONST_NOTHROW_LEAF_LIST)
>  DEF_EXT_LIB_BUILTIN    (BUILT_IN_ISINFD128, "isinfd128", 
> BT_FN_INT_DFLOAT128, ATTR_CONST_NOTHROW_LEAF_LIST)
> +DEF_C99_C90RES_BUILTIN (BUILT_IN_ISZERO, "iszero", BT_FN_INT_VAR, 
> ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
> +DEF_C99_C90RES_BUILTIN (BUILT_IN_ISSUBNORMAL, "issubnormal", BT_FN_INT_VAR, 
> ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)

No, these should be DEF_GCC_BUILTIN, so that only the __builtin_* names 
exist.

> +static tree
> +is_zero (gimple_seq *seq, tree arg, location_t loc)
> +{
> +  tree type = TREE_TYPE (arg);
> +
> +  /* If not using optimized route then exit early.  */
> +  if (!use_ieee_int_mode (arg))
> +  {
> +    tree arg_p
> +      = emit_tree_and_return_var (seq, fold_build1_loc (loc, ABS_EXPR, type,
> +                                                     arg));
> +    tree res = fold_build2_loc (loc, EQ_EXPR, boolean_type_node, arg_p,
> +                             build_real (type, dconst0));

There is no need to take the absolute value before comparing with constant 
0.

I think tests for the new built-in functions should be added for the 
_Float* types (so add gcc.dg/torture/float-tg-4.h to test them and 
associated float*-tg-4.c files using that header to instantiate the tests 
for each type).

-- 
Joseph S. Myers
jos...@codesourcery.com

Reply via email to