Hi David, Thanks for the review!
On Wed, Aug 26, 2026 at 8:55 PM David Rowley <[email protected]> wrote: > > On Wed, 26 Aug 2026 at 00:33, Daniel Gustafsson <[email protected]> wrote: > > > > > On 25 Aug 2026, at 14:18, Dagfinn Ilmari Mannsåker <[email protected]> > > > wrote: > > > > > > Instead of open-coding this, how about about using pg_neg_s32_overflow? > > > > > > if (pg_neg_s32_overflow(n, &n)) > > > n = PG_INT32_MAX; > > > > Yes, that's indeed a good idea. > > I do see a few places where we do check for PG_INT32_MIN instead of > using pg_neg_s32_overflow(). The example in [1] does end up with less > code as a result of using pg_neg_s32_overflow(), so it might be worth > removing all applicable examples that use the other method from master > as a follow-up. It'd be worth verifying it doesn't make anything worse > for build systems that don't have __builtin_sub_overflow(). I verified that converting them is safe on platforms without __builtin_sub_overflow() -- there, pg_neg_s{16,32,64}_overflow() inline to exactly the same "if (a == PG_INTnn_MIN) ...; *result = -a;" code that is being replaced, so the generated code is unchanged (I confirmed the fallback path is bit-for-bit equivalent to the manual form, including at the INT_MIN boundary). Rather than pile that cleanup onto this bug-fix thread, I'll send it as a separate follow-up so this fix can be considered on its own. Will post that shortly and CC you three. > > David > > [1] https://godbolt.org/z/sfxY847E4 -- Regards, Ewan Young
