On Mon, Aug 31, 2026 at 05:46:22PM +0800, Ewan Young wrote: > 0001 is the conversion. Converted sites whose branch raises > ereport(ERROR) use the helper bare. numericvar_to_int64() keeps its > unlikely(), because its failure branch is a plain "return false" with > no cold marking -- and the adjacent pg_mul/pg_sub_s64_overflow() calls > a few lines up in the same function keep theirs, so dropping only the > new one would trade one inconsistency for another.
I was looking at this one with clang and gcc, and can spot what looks like regressions with new instructions for the following changes: int4abs int2abs int4lcm int8abs int8clm There are some cneg -> tbnz/tbz on arm64 and cmovnsl -> testl/jns on x86-64, with and without the BUILTIN flag. This is telling that my rewriting suggestion just sucks for the abs functions. Sorry. :) Also DecodeInterval(), where itm_in() changes slightly, increasing in activity. Not sure how to rewrite that, or if we should do it.. In all that, int4um, int4div, int42div, int8un, int8div, int84div, int82div, cash_div_int64, cash_in, numeric_to_int64 look cleaner overall. Without the builtin I get an identical result, and I am seeing a variance of 0~5 less instructions with the builtin. cash_in is showing much more reduction than the others. Note that I have kept the unlikely() in numeric.c, you are right that this matters with clang.. This first batch is done in the attached, as of v3 that I am planning to apply. We could always look at the rest later, that's still a good cut. -- Michael
From f07d474398e49647ff8aebfec494b447d8fc623a Mon Sep 17 00:00:00 2001 From: Michael Paquier <[email protected]> Date: Tue, 1 Sep 2026 08:42:53 +0900 Subject: [PATCH v3] Use pg_neg_s{16,32,64}_overflow() for some overflow checks Extracted from a larger patch by the same author; these ones have proved to reduce the number of instructions generated overall for clang, gcc, with and without the builtin versions. --- src/backend/utils/adt/cash.c | 9 +++++---- src/backend/utils/adt/int.c | 11 +++++------ src/backend/utils/adt/int8.c | 12 ++++-------- src/backend/utils/adt/numeric.c | 3 +-- 4 files changed, 15 insertions(+), 20 deletions(-) diff --git a/src/backend/utils/adt/cash.c b/src/backend/utils/adt/cash.c index 310b3bb3fca6..0820187b8e6a 100644 --- a/src/backend/utils/adt/cash.c +++ b/src/backend/utils/adt/cash.c @@ -156,6 +156,8 @@ cash_mul_int64(Cash c, int64 i) static inline Cash cash_div_int64(Cash c, int64 i) { + Cash res; + if (unlikely(i == 0)) ereport(ERROR, (errcode(ERRCODE_DIVISION_BY_ZERO), @@ -169,11 +171,11 @@ cash_div_int64(Cash c, int64 i) */ if (i == -1) { - if (unlikely(c == PG_INT64_MIN)) + if (pg_neg_s64_overflow(c, &res)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("money out of range"))); - return -c; + return res; } /* No overflow is possible */ @@ -379,12 +381,11 @@ cash_in(PG_FUNCTION_ARGS) */ if (sgn > 0) { - if (value == PG_INT64_MIN) + if (pg_neg_s64_overflow(value, &result)) ereturn(escontext, (Datum) 0, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("value \"%s\" is out of range for type %s", str, "money"))); - result = -value; } else result = value; diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c index 01608d8ca424..68fecbfe2ed3 100644 --- a/src/backend/utils/adt/int.c +++ b/src/backend/utils/adt/int.c @@ -800,12 +800,13 @@ Datum int4um(PG_FUNCTION_ARGS) { int32 arg = PG_GETARG_INT32(0); + int32 result; - if (unlikely(arg == PG_INT32_MIN)) + if (pg_neg_s32_overflow(arg, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); - PG_RETURN_INT32(-arg); + PG_RETURN_INT32(result); } Datum @@ -882,11 +883,10 @@ int4div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - if (unlikely(arg1 == PG_INT32_MIN)) + if (pg_neg_s32_overflow(arg1, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); - result = -arg1; PG_RETURN_INT32(result); } @@ -1140,11 +1140,10 @@ int42div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - if (unlikely(arg1 == PG_INT32_MIN)) + if (pg_neg_s32_overflow(arg1, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("integer out of range"))); - result = -arg1; PG_RETURN_INT32(result); } diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c index 1f59d831600f..1a8bddd6bb11 100644 --- a/src/backend/utils/adt/int8.c +++ b/src/backend/utils/adt/int8.c @@ -449,11 +449,10 @@ int8um(PG_FUNCTION_ARGS) int64 arg = PG_GETARG_INT64(0); int64 result; - if (unlikely(arg == PG_INT64_MIN)) + if (pg_neg_s64_overflow(arg, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); - result = -arg; PG_RETURN_INT64(result); } @@ -531,11 +530,10 @@ int8div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - if (unlikely(arg1 == PG_INT64_MIN)) + if (pg_neg_s64_overflow(arg1, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); - result = -arg1; PG_RETURN_INT64(result); } @@ -991,11 +989,10 @@ int84div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - if (unlikely(arg1 == PG_INT64_MIN)) + if (pg_neg_s64_overflow(arg1, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); - result = -arg1; PG_RETURN_INT64(result); } @@ -1133,11 +1130,10 @@ int82div(PG_FUNCTION_ARGS) */ if (arg2 == -1) { - if (unlikely(arg1 == PG_INT64_MIN)) + if (pg_neg_s64_overflow(arg1, &result)) ereport(ERROR, (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), errmsg("bigint out of range"))); - result = -arg1; PG_RETURN_INT64(result); } diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 523cd3cd608e..37f24e33857f 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -7870,9 +7870,8 @@ numericvar_to_int64(const NumericVar *var, int64 *result) if (!neg) { - if (unlikely(val == PG_INT64_MIN)) + if (unlikely(pg_neg_s64_overflow(val, &val))) return false; - val = -val; } *result = val; -- 2.55.0
signature.asc
Description: PGP signature
