Re: Use pg_neg_s*_overflow() for open-coded negation overflow checks

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Ewan Young <kdbase(dot)hack(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, David Rowley <dgrowleyml(at)gmail(dot)com>, Dagfinn Ilmari Mannsåker <ilmari(at)ilmari(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Subject: Re: Use pg_neg_s*_overflow() for open-coded negation overflow checks
Date: 2026-08-31 23:45:51
Message-ID: apYSL1xlyglUMj9O@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v3-0001-Use-pg_neg_s-16-32-64-_overflow-for-some-overflow.patch text/plain 4.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Zsolt Parragi 2026-08-31 23:47:29 Re: Routed ON CONFLICT inserts broken by partition-local deferrable unique constraints in 19 and master
Previous Message Zsolt Parragi 2026-08-31 23:31:42 Re: Offline data checksum changes can cause incorrect checksum state on standbys