Avoid a performance regression in float overflow/underflow detection.

Commit 6bf0bc842 replaced float.c's CHECKFLOATVAL() macro with static
inline subroutines, but that wasn't too well thought out.  In the original
coding, the unlikely condition (isinf(result) or result == 0) was checked
first, and the inf_is_valid or zero_is_valid condition only afterwards.
The inline-subroutine coding caused that to be swapped around, which is
pretty horrid for performance because (a) in common cases the is_valid
condition is twice as expensive to evaluate (e.g., requiring two isinf()
calls not one) and (b) in common cases the is_valid condition is false,
requiring us to perform the unlikely-condition check anyway.  Net result
is that one isinf() call becomes two or three, resulting in visible
performance loss as reported by Keisuke Kuroda.

The original fix proposal was to revert the replacement of the macro,
but on second thought, that macro was just a bad idea from the beginning:
if anything it's a net negative for readability of the code.  So instead,
let's just open-code all the overflow/underflow tests, being careful to
test the unlikely condition first (and mark it unlikely() to help the
compiler get the point).

Also, rather than having N copies of the actual ereport() calls, collapse
those into out-of-line error subroutines to save some code space.  This
does mean that the error file/line numbers won't be very helpful for
figuring out where the issue really is --- but we'd already burned that
bridge by putting the ereports into static inlines.

In HEAD, check_float[48]_val() are gone altogether.  In v12, leave them
present in float.h but unused in the core code, just in case some
extension is depending on them.

Emre Hasegeli, with some kibitzing from me and Andres Freund

Discussion: https://postgr.es/m/CANDwggLe1Gc1OrRqvPfGE=kM9K0FSfia0hbeFCEmwabhLz95AA@mail.gmail.com

diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c
index a90d4db21504f41a67f5510b87fc49c0ede6fc32..012c3252f3c7c5cd0077b9ee702efed3e3611e48 100644 (file)
--- a/src/backend/utils/adt/float.c
+++ b/src/backend/utils/adt/float.c
@@ -86,6 +86,38 @@ static double cbrt(double x);
 #endif                                                 /* HAVE_CBRT */
 
 
+/*
+ * We use these out-of-line ereport() calls to report float overflow,
+ * underflow, and zero-divide, because following our usual practice of
+ * repeating them at each call site would lead to a lot of code bloat.
+ *
+ * This does mean that you don't get a useful error location indicator.
+ */
+pg_noinline void
+float_overflow_error(void)
+{
+       ereport(ERROR,
+                       (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                        errmsg("value out of range: overflow")));
+}
+
+pg_noinline void
+float_underflow_error(void)
+{
+       ereport(ERROR,
+                       (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+                        errmsg("value out of range: underflow")));
+}
+
+pg_noinline void
+float_zero_divide_error(void)
+{
+       ereport(ERROR,
+                       (errcode(ERRCODE_DIVISION_BY_ZERO),
+                        errmsg("division by zero")));
+}
+
+
 /*
  * Returns -1 if 'val' represents negative infinity, 1 if 'val'
  * represents (positive) infinity, and 0 otherwise. On some platforms,
@@ -1190,10 +1222,15 @@ Datum
 dtof(PG_FUNCTION_ARGS)
 {
        float8          num = PG_GETARG_FLOAT8(0);
+       float4          result;
 
-       check_float4_val((float4) num, isinf(num), num == 0);
+       result = (float4) num;
+       if (unlikely(isinf(result)) && !isinf(num))
+               float_overflow_error();
+       if (unlikely(result == 0.0f) && num != 0.0)
+               float_underflow_error();
 
-       PG_RETURN_FLOAT4((float4) num);
+       PG_RETURN_FLOAT4(result);
 }
 
 
@@ -1444,8 +1481,11 @@ dsqrt(PG_FUNCTION_ARGS)
                                 errmsg("cannot take square root of a negative number")));
 
        result = sqrt(arg1);
+       if (unlikely(isinf(result)) && !isinf(arg1))
+               float_overflow_error();
+       if (unlikely(result == 0.0) && arg1 != 0.0)
+               float_underflow_error();
 
-       check_float8_val(result, isinf(arg1), arg1 == 0);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1460,7 +1500,11 @@ dcbrt(PG_FUNCTION_ARGS)
        float8          result;
 
        result = cbrt(arg1);
-       check_float8_val(result, isinf(arg1), arg1 == 0);
+       if (unlikely(isinf(result)) && !isinf(arg1))
+               float_overflow_error();
+       if (unlikely(result == 0.0) && arg1 != 0.0)
+               float_underflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1532,7 +1576,11 @@ dpow(PG_FUNCTION_ARGS)
        else if (errno == ERANGE && result != 0 && !isinf(result))
                result = get_float8_infinity();
 
-       check_float8_val(result, isinf(arg1) || isinf(arg2), arg1 == 0);
+       if (unlikely(isinf(result)) && !isinf(arg1) && !isinf(arg2))
+               float_overflow_error();
+       if (unlikely(result == 0.0) && arg1 != 0.0)
+               float_underflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1551,7 +1599,11 @@ dexp(PG_FUNCTION_ARGS)
        if (errno == ERANGE && result != 0 && !isinf(result))
                result = get_float8_infinity();
 
-       check_float8_val(result, isinf(arg1), false);
+       if (unlikely(isinf(result)) && !isinf(arg1))
+               float_overflow_error();
+       if (unlikely(result == 0.0))
+               float_underflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1579,8 +1631,11 @@ dlog1(PG_FUNCTION_ARGS)
                                 errmsg("cannot take logarithm of a negative number")));
 
        result = log(arg1);
+       if (unlikely(isinf(result)) && !isinf(arg1))
+               float_overflow_error();
+       if (unlikely(result == 0.0) && arg1 != 1.0)
+               float_underflow_error();
 
-       check_float8_val(result, isinf(arg1), arg1 == 1);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1609,8 +1664,11 @@ dlog10(PG_FUNCTION_ARGS)
                                 errmsg("cannot take logarithm of a negative number")));
 
        result = log10(arg1);
+       if (unlikely(isinf(result)) && !isinf(arg1))
+               float_overflow_error();
+       if (unlikely(result == 0.0) && arg1 != 1.0)
+               float_underflow_error();
 
-       check_float8_val(result, isinf(arg1), arg1 == 1);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1639,8 +1697,9 @@ dacos(PG_FUNCTION_ARGS)
                                 errmsg("input is out of range")));
 
        result = acos(arg1);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
 
-       check_float8_val(result, false, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1669,8 +1728,9 @@ dasin(PG_FUNCTION_ARGS)
                                 errmsg("input is out of range")));
 
        result = asin(arg1);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
 
-       check_float8_val(result, false, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1694,8 +1754,9 @@ datan(PG_FUNCTION_ARGS)
         * finite, even if the input is infinite.
         */
        result = atan(arg1);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
 
-       check_float8_val(result, false, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1719,8 +1780,9 @@ datan2(PG_FUNCTION_ARGS)
         * should always be finite, even if the inputs are infinite.
         */
        result = atan2(arg1, arg2);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
 
-       check_float8_val(result, false, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1759,8 +1821,9 @@ dcos(PG_FUNCTION_ARGS)
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("input is out of range")));
+       if (unlikely(isinf(result)))
+               float_overflow_error();
 
-       check_float8_val(result, false, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1787,7 +1850,8 @@ dcot(PG_FUNCTION_ARGS)
                                 errmsg("input is out of range")));
 
        result = 1.0 / result;
-       check_float8_val(result, true /* cot(0) == Inf */ , true);
+       /* Not checking for overflow because cot(0) == Inf */
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1812,8 +1876,9 @@ dsin(PG_FUNCTION_ARGS)
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("input is out of range")));
+       if (unlikely(isinf(result)))
+               float_overflow_error();
 
-       check_float8_val(result, false, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1838,8 +1903,8 @@ dtan(PG_FUNCTION_ARGS)
                ereport(ERROR,
                                (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
                                 errmsg("input is out of range")));
+       /* Not checking for overflow because tan(pi/2) == Inf */
 
-       check_float8_val(result, true /* tan(pi/2) == Inf */ , true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -1991,7 +2056,9 @@ dacosd(PG_FUNCTION_ARGS)
        else
                result = 90.0 + asind_q1(-arg1);
 
-       check_float8_val(result, false, true);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2026,7 +2093,9 @@ dasind(PG_FUNCTION_ARGS)
        else
                result = -asind_q1(-arg1);
 
-       check_float8_val(result, false, true);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2056,7 +2125,9 @@ datand(PG_FUNCTION_ARGS)
        atan_arg1 = atan(arg1);
        result = (atan_arg1 / atan_1_0) * 45.0;
 
-       check_float8_val(result, false, true);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2090,7 +2161,9 @@ datan2d(PG_FUNCTION_ARGS)
        atan2_arg1_arg2 = atan2(arg1, arg2);
        result = (atan2_arg1_arg2 / atan_1_0) * 45.0;
 
-       check_float8_val(result, false, true);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2211,7 +2284,9 @@ dcosd(PG_FUNCTION_ARGS)
 
        result = sign * cosd_q1(arg1);
 
-       check_float8_val(result, false, true);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2276,7 +2351,8 @@ dcotd(PG_FUNCTION_ARGS)
        if (result == 0.0)
                result = 0.0;
 
-       check_float8_val(result, true /* cotd(0) == Inf */ , true);
+       /* Not checking for overflow because cotd(0) == Inf */
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2330,7 +2406,9 @@ dsind(PG_FUNCTION_ARGS)
 
        result = sign * sind_q1(arg1);
 
-       check_float8_val(result, false, true);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2395,7 +2473,8 @@ dtand(PG_FUNCTION_ARGS)
        if (result == 0.0)
                result = 0.0;
 
-       check_float8_val(result, true /* tand(90) == Inf */ , true);
+       /* Not checking for overflow because tand(90) == Inf */
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2462,7 +2541,6 @@ dsinh(PG_FUNCTION_ARGS)
                        result = get_float8_infinity();
        }
 
-       check_float8_val(result, true, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2486,7 +2564,9 @@ dcosh(PG_FUNCTION_ARGS)
        if (errno == ERANGE)
                result = get_float8_infinity();
 
-       check_float8_val(result, true, false);
+       if (unlikely(result == 0.0))
+               float_underflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2504,7 +2584,9 @@ dtanh(PG_FUNCTION_ARGS)
         */
        result = tanh(arg1);
 
-       check_float8_val(result, false, true);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2522,7 +2604,6 @@ dasinh(PG_FUNCTION_ARGS)
         */
        result = asinh(arg1);
 
-       check_float8_val(result, true, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2548,7 +2629,6 @@ dacosh(PG_FUNCTION_ARGS)
 
        result = acosh(arg1);
 
-       check_float8_val(result, true, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2583,7 +2663,6 @@ datanh(PG_FUNCTION_ARGS)
        else
                result = atanh(arg1);
 
-       check_float8_val(result, true, true);
        PG_RETURN_FLOAT8(result);
 }
 
@@ -2784,7 +2863,8 @@ float8_combine(PG_FUNCTION_ARGS)
                Sx = float8_pl(Sx1, Sx2);
                tmp = Sx1 / N1 - Sx2 / N2;
                Sxx = Sxx1 + Sxx2 + N1 * N2 * tmp * tmp / N;
-               check_float8_val(Sxx, isinf(Sxx1) || isinf(Sxx2), true);
+               if (unlikely(isinf(Sxx)) && !isinf(Sxx1) && !isinf(Sxx2))
+                       float_overflow_error();
        }
 
        /*
@@ -2853,9 +2933,7 @@ float8_accum(PG_FUNCTION_ARGS)
                if (isinf(Sx) || isinf(Sxx))
                {
                        if (!isinf(transvalues[1]) && !isinf(newval))
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                                                errmsg("value out of range: overflow")));
+                               float_overflow_error();
 
                        Sxx = get_float8_nan();
                }
@@ -2929,9 +3007,7 @@ float4_accum(PG_FUNCTION_ARGS)
                if (isinf(Sx) || isinf(Sxx))
                {
                        if (!isinf(transvalues[1]) && !isinf(newval))
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                                                errmsg("value out of range: overflow")));
+                               float_overflow_error();
 
                        Sxx = get_float8_nan();
                }
@@ -3152,9 +3228,7 @@ float8_regr_accum(PG_FUNCTION_ARGS)
                                (isinf(Sxy) &&
                                 !isinf(transvalues[1]) && !isinf(newvalX) &&
                                 !isinf(transvalues[3]) && !isinf(newvalY)))
-                               ereport(ERROR,
-                                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                                                errmsg("value out of range: overflow")));
+                               float_overflow_error();
 
                        if (isinf(Sxx))
                                Sxx = get_float8_nan();
@@ -3294,13 +3368,16 @@ float8_regr_combine(PG_FUNCTION_ARGS)
                Sx = float8_pl(Sx1, Sx2);
                tmp1 = Sx1 / N1 - Sx2 / N2;
                Sxx = Sxx1 + Sxx2 + N1 * N2 * tmp1 * tmp1 / N;
-               check_float8_val(Sxx, isinf(Sxx1) || isinf(Sxx2), true);
+               if (unlikely(isinf(Sxx)) && !isinf(Sxx1) && !isinf(Sxx2))
+                       float_overflow_error();
                Sy = float8_pl(Sy1, Sy2);
                tmp2 = Sy1 / N1 - Sy2 / N2;
                Syy = Syy1 + Syy2 + N1 * N2 * tmp2 * tmp2 / N;
-               check_float8_val(Syy, isinf(Syy1) || isinf(Syy2), true);
+               if (unlikely(isinf(Syy)) && !isinf(Syy1) && !isinf(Syy2))
+                       float_overflow_error();
                Sxy = Sxy1 + Sxy2 + N1 * N2 * tmp1 * tmp2 / N;
-               check_float8_val(Sxy, isinf(Sxy1) || isinf(Sxy2), true);
+               if (unlikely(isinf(Sxy)) && !isinf(Sxy1) && !isinf(Sxy2))
+                       float_overflow_error();
        }
 
        /*

diff --git a/src/backend/utils/adt/geo_ops.c b/src/backend/utils/adt/geo_ops.c
index d5ded471c4d5082da11e17cb892364108837fc6c..71ee7f8f08e8bf6387024da92447c0748fc3d01e 100644 (file)
--- a/src/backend/utils/adt/geo_ops.c
+++ b/src/backend/utils/adt/geo_ops.c
@@ -5545,7 +5545,10 @@ pg_hypot(float8 x, float8 y)
        yx = y / x;
        result = x * sqrt(1.0 + (yx * yx));
 
-       check_float8_val(result, false, false);
+       if (unlikely(isinf(result)))
+               float_overflow_error();
+       if (unlikely(result == 0.0))
+               float_underflow_error();
 
        return result;
 }

diff --git a/src/include/utils/float.h b/src/include/utils/float.h
index e2c5dc0f571badcfa870638e9543bd3a14e38c91..8d4bbc51a654dfd8d9f4c481c2d1a778142beb22 100644 (file)
--- a/src/include/utils/float.h
+++ b/src/include/utils/float.h
@@ -37,6 +37,9 @@ extern PGDLLIMPORT int extra_float_digits;
 /*
  * Utility functions in float.c
  */
+extern void float_overflow_error(void) pg_attribute_noreturn();
+extern void float_underflow_error(void) pg_attribute_noreturn();
+extern void float_zero_divide_error(void) pg_attribute_noreturn();
 extern int     is_infinite(float8 val);
 extern float8 float8in_internal(char *num, char **endptr_p,
                                                                const char *type_name, const char *orig_string);
@@ -129,41 +132,7 @@ get_float8_nan(void)
 }
 
 /*
- * Checks to see if a float4/8 val has underflowed or overflowed
- */
-
-static inline void
-check_float4_val(const float4 val, const bool inf_is_valid,
-                                const bool zero_is_valid)
-{
-       if (!inf_is_valid && unlikely(isinf(val)))
-               ereport(ERROR,
-                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                                errmsg("value out of range: overflow")));
-
-       if (!zero_is_valid && unlikely(val == 0.0))
-               ereport(ERROR,
-                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                                errmsg("value out of range: underflow")));
-}
-
-static inline void
-check_float8_val(const float8 val, const bool inf_is_valid,
-                                const bool zero_is_valid)
-{
-       if (!inf_is_valid && unlikely(isinf(val)))
-               ereport(ERROR,
-                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                                errmsg("value out of range: overflow")));
-
-       if (!zero_is_valid && unlikely(val == 0.0))
-               ereport(ERROR,
-                               (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
-                                errmsg("value out of range: underflow")));
-}
-
-/*
- * Routines for operations with the checks above
+ * Floating-point arithmetic with overflow/underflow reported as errors
  *
  * There isn't any way to check for underflow of addition/subtraction
  * because numbers near the underflow value have already been rounded to
@@ -178,7 +147,8 @@ float4_pl(const float4 val1, const float4 val2)
        float4          result;
 
        result = val1 + val2;
-       check_float4_val(result, isinf(val1) || isinf(val2), true);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
 
        return result;
 }
@@ -189,7 +159,8 @@ float8_pl(const float8 val1, const float8 val2)
        float8          result;
 
        result = val1 + val2;
-       check_float8_val(result, isinf(val1) || isinf(val2), true);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
 
        return result;
 }
@@ -200,7 +171,8 @@ float4_mi(const float4 val1, const float4 val2)
        float4          result;
 
        result = val1 - val2;
-       check_float4_val(result, isinf(val1) || isinf(val2), true);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
 
        return result;
 }
@@ -211,7 +183,8 @@ float8_mi(const float8 val1, const float8 val2)
        float8          result;
 
        result = val1 - val2;
-       check_float8_val(result, isinf(val1) || isinf(val2), true);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
 
        return result;
 }
@@ -222,8 +195,10 @@ float4_mul(const float4 val1, const float4 val2)
        float4          result;
 
        result = val1 * val2;
-       check_float4_val(result, isinf(val1) || isinf(val2),
-                                        val1 == 0.0f || val2 == 0.0f);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
+       if (unlikely(result == 0.0f) && val1 != 0.0f && val2 != 0.0f)
+               float_underflow_error();
 
        return result;
 }
@@ -234,8 +209,10 @@ float8_mul(const float8 val1, const float8 val2)
        float8          result;
 
        result = val1 * val2;
-       check_float8_val(result, isinf(val1) || isinf(val2),
-                                        val1 == 0.0 || val2 == 0.0);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
+       if (unlikely(result == 0.0) && val1 != 0.0 && val2 != 0.0)
+               float_underflow_error();
 
        return result;
 }
@@ -245,13 +222,13 @@ float4_div(const float4 val1, const float4 val2)
 {
        float4          result;
 
-       if (val2 == 0.0f)
-               ereport(ERROR,
-                               (errcode(ERRCODE_DIVISION_BY_ZERO),
-                                errmsg("division by zero")));
-
+       if (unlikely(val2 == 0.0f))
+               float_zero_divide_error();
        result = val1 / val2;
-       check_float4_val(result, isinf(val1) || isinf(val2), val1 == 0.0f);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
+       if (unlikely(result == 0.0f) && val1 != 0.0f)
+               float_underflow_error();
 
        return result;
 }
@@ -261,13 +238,13 @@ float8_div(const float8 val1, const float8 val2)
 {
        float8          result;
 
-       if (val2 == 0.0)
-               ereport(ERROR,
-                               (errcode(ERRCODE_DIVISION_BY_ZERO),
-                                errmsg("division by zero")));
-
+       if (unlikely(val2 == 0.0))
+               float_zero_divide_error();
        result = val1 / val2;
-       check_float8_val(result, isinf(val1) || isinf(val2), val1 == 0.0);
+       if (unlikely(isinf(result)) && !isinf(val1) && !isinf(val2))
+               float_overflow_error();
+       if (unlikely(result == 0.0) && val1 != 0.0)
+               float_underflow_error();
 
        return result;
 }