On Wed, 11 Sep 2019, Tejas Joshi wrote:
> diff --git a/gcc/builtins.def b/gcc/builtins.def
> index 8bb7027aac7..2df616c477e 100644
> --- a/gcc/builtins.def
> +++ b/gcc/builtins.def
> @@ -355,6 +355,9 @@ DEF_EXT_LIB_FLOATN_NX_BUILTINS (BUILT_IN_FABS, "fabs",
> FABS_TYPE, ATTR_CONST_NOT
> DEF_GCC_BUILTIN (BUILT_IN_FABSD32, "fabsd32",
> BT_FN_DFLOAT32_DFLOAT32, ATTR_CONST_NOTHROW_LEAF_LIST)
> DEF_GCC_BUILTIN (BUILT_IN_FABSD64, "fabsd64",
> BT_FN_DFLOAT64_DFLOAT64, ATTR_CONST_NOTHROW_LEAF_LIST)
> DEF_GCC_BUILTIN (BUILT_IN_FABSD128, "fabsd128",
> BT_FN_DFLOAT128_DFLOAT128, ATTR_CONST_NOTHROW_LEAF_LIST)
> +DEF_EXT_LIB_BUILTIN (BUILT_IN_FADD, "fadd", BT_FN_FLOAT_DOUBLE_DOUBLE,
> ATTR_CONST_NOTHROW_LEAF_LIST)
> +DEF_EXT_LIB_BUILTIN (BUILT_IN_FADDL, "faddl",
> BT_FN_FLOAT_LONGDOUBLE_LONGDOUBLE, ATTR_CONST_NOTHROW_LEAF_LIST)
> +DEF_EXT_LIB_BUILTIN (BUILT_IN_DADDL, "daddl",
> BT_FN_DOUBLE_LONGDOUBLE_LONGDOUBLE, ATTR_CONST_NOTHROW_LEAF_LIST)
I think these should all use ATTR_MATHFN_FPROUNDING_ERRNO instead of
ATTR_CONST_NOTHROW_LEAF_LIST. That way the attributes depend properly on
the command-line options.
> DEF_C99_BUILTIN (BUILT_IN_FDIM, "fdim", BT_FN_DOUBLE_DOUBLE_DOUBLE,
> ATTR_MATHFN_FPROUNDING_ERRNO)
> DEF_C99_BUILTIN (BUILT_IN_FDIMF, "fdimf", BT_FN_FLOAT_FLOAT_FLOAT,
> ATTR_MATHFN_FPROUNDING_ERRNO)
> DEF_C99_BUILTIN (BUILT_IN_FDIML, "fdiml",
> BT_FN_LONGDOUBLE_LONGDOUBLE_LONGDOUBLE, ATTR_MATHFN_FPROUNDING_ERRNO)
> diff --git a/gcc/fold-const-call.c b/gcc/fold-const-call.c
> index 3a14d2a41c1..f6b4508a101 100644
> --- a/gcc/fold-const-call.c
> +++ b/gcc/fold-const-call.c
> @@ -570,6 +570,44 @@ fold_const_nextafter (real_value *result, const
> real_value *arg0,
> return true;
> }
>
> +/* Try to evaluate:
> +
> + *RESULT = add (*ARG0, *ARG1)
> +
> + in format FORMAT. Return true on success. */
It's not specifically "add". It's an operation determined by ICODE.
> +
> +static bool
> +fold_const_narrow_binary (real_value *result, const real_value *arg0,
> + int icode, const real_value *arg1,
> + const real_format *format)
> +{
> + if (REAL_VALUE_ISSIGNALING_NAN (*arg0)
> + || REAL_VALUE_ISSIGNALING_NAN (*arg1))
> + return false;
> +
> + real_arithmetic (result, icode, arg0, arg1);
> + /* Underflow condition. */
> + if (flag_errno_math
> + && result->cl == rvc_normal
> + && REAL_EXP (result) < format->emin)
> + return false;
> +
> + if (!exact_real_truncate (format, result)
> + && (flag_rounding_math || flag_trapping_math))
> + return false;
> +
> + real_convert (result, format, result);
> + /* Overflow condition. */
> + if (!real_isfinite (result) && flag_errno_math)
> + return false;
For overflow, this should be checking for real_isinf, not for
!real_isfinite. And specifically for real_isinf with both arguments
finite, it's not an overflow to have an infinite result from an infinite
argument. (Strictly, infinite result with finite arguments could be
either overflow or divide-by-zero, depending on whether it's an exact or
inexact infinite result, but both those cases should be handled the same
here.)
> + if (REAL_VALUE_ISNAN (*result)
> + && (flag_errno_math || flag_trapping_math))
> + return false;
This last condition should only apply if neither argument is NaN; it's
fine to fold to a NaN result when one or both arguments is a quiet NaN
(and signaling NaNs were handled above).
There is one other case that also needs excluding in this function. If
both arguments are zero, and the operation is either adding two zeroes of
different sign, or subtracting two zeroes of the same sign, it's not valid
to fold if flag_rounding_math, because the sign of the result of 0 - 0
depends on the rounding mode.
> diff --git a/gcc/testsuite/gcc.dg/builtin-fadd-5.c
> b/gcc/testsuite/gcc.dg/builtin-fadd-5.c
> new file mode 100644
> index 00000000000..ac3b547724c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/builtin-fadd-5.c
> @@ -0,0 +1,26 @@
> +/* { dg-do link } */
> +/* { dg-options "-O2 -fno-trapping-math -fno-math-errno" } */
> +
> +extern int link_error (int);
> +
> +#define TEST(FN, VAL1, VAL2, RESULT) \
> + if (__builtin_##FN (VAL1, VAL2) != RESULT) link_error (__LINE__);
> +
> +int
> +main (void)
> +{
> + TEST(fadd, __DBL_MAX__, __DBL_MAX__, __builtin_inff());
> + TEST(fadd, __FLT_MAX__, __FLT_MAX__, __builtin_inff());
> + TEST(fadd, __DBL_MIN__, __DBL_MIN__, 0.0);
> +
> + TEST(faddl, __LDBL_MAX__, __LDBL_MAX__, __builtin_inff());
> + TEST(faddl, __FLT_MAX__, __FLT_MAX__, __builtin_inff());
> + TEST(faddl, __LDBL_MIN__, __LDBL_MIN__, 0.0);
> +
> + TEST(daddl, __LDBL_MAX__, __LDBL_MAX__, __builtin_inf());
> + TEST(daddl, __DBL_MAX__, __DBL_MAX__, __builtin_inf());
> + TEST(daddl, __LDBL_MIN__, __LDBL_MIN__, 0.0);
In the case where long double = double, this last test isn't correct, as
LDBL_MIN + LDBL_MIN == DBL_MIN + DBL_MIN and isn't zero.
The same also applies to the last test in builtin-fadd-6.c.
--
Joseph S. Myers
jos...@codesourcery.com
