Hi,
On 2017-10-24 03:39:54 -0700, Andres Freund wrote:
> Largely that's due to the overflow checks.
>
> For integers we currently do:
>
> #define SAMESIGN(a,b) (((a) < 0) == ((b) < 0))
>
> /*
> * Overflow check. If the inputs are of different signs then their sum
> * cannot overflow. If the inputs are of the same sign, their sum had
> * better be that sign too.
> */
> if (SAMESIGN(arg1, arg2) && !SAMESIGN(result, arg1))
> ereport(ERROR,
> (errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
> errmsg("integer out of range")));
>
> which means that we turn a single integer instruction into ~10,
> including a bunch of branches. All that despite the fact that most
> architectures have flag registers signalling integer overflow. It's just
> that C doesn't easily make that available.
>
> gcc exposes more efficient overflow detection via intrinsics:
> https://gcc.gnu.org/onlinedocs/gcc-7.1.0/gcc/Integer-Overflow-Builtins.html
>
> Using that turns the non-error path from int4pl from:
>
> 0x0000000000826ec0 <+0>: mov 0x20(%rdi),%rcx # arg1
> 0x0000000000826ec4 <+4>: mov 0x28(%rdi),%rdx # arg2
> 0x0000000000826ec8 <+8>: mov %ecx,%esi
> 0x0000000000826eca <+10>: lea (%rdx,%rcx,1),%eax # add
> # overflow check
> 0x0000000000826ecd <+13>: shr $0x1f,%edx
> 0x0000000000826ed0 <+16>: not %esi
> 0x0000000000826ed2 <+18>: shr $0x1f,%esi
> 0x0000000000826ed5 <+21>: cmp %dl,%sil
> 0x0000000000826ed8 <+24>: je 0x826f30 <int4pl+112>
> 0x0000000000826eda <+26>: mov %eax,%edx
> 0x0000000000826edc <+28>: shr $0x1f,%ecx
> 0x0000000000826edf <+31>: shr $0x1f,%edx
> 0x0000000000826ee2 <+34>: cmp %cl,%dl
> 0x0000000000826ee4 <+36>: je 0x826f30 <int4pl+112>
> /* overflow error code */
> 0x0000000000826f30 <+112>: retq
>
> into
>
> 0x0000000000826ec0 <+0>: mov 0x28(%rdi),%rax # arg2
> 0x0000000000826ec4 <+4>: add 0x20(%rdi),%eax # arg1 + arg2
> 0x0000000000826ec7 <+7>: jo 0x826ecc <int4pl+12> # jump if overflowed
> 0x0000000000826ec9 <+9>: mov %eax,%eax # clear high bits
> 0x0000000000826ecb <+11>: retq
>
> which, not that surprisingly, is faster. Not to speak of easier to read
> ;)
>
> Besides the fact that the code is faster, there's also the issue that
> the current way to do overflow checks is not actually correct C, and
> requires compiler flags like -fwrapv.
Attached is a series of patches that:
0001) Introduces pg_{add,sub,mul}{16,32,64}_overflow(a, b, *result)
These use compiler intrinsics on gcc/clang. If that's not
available, they cast to a wider type and to overflow checks. For
64bit there's a fallback for the case 128bit math is not
available (here I stole from an old patch of Greg's).
These fallbacks are, as far as I can tell, C free of overflow
related undefined behaviour.
Perhaps it should rather be pg_add_s32_overflow, or a similar
naming scheme?
0002) Converts int.c, int8.c and a smattering of other functions to use
the new facilities. This removes a fair amount of code.
It might make sense to split this up further, but right now that's
the set of functions that either are affected performancewise by
previous overflow checks, and/or relied on wraparound
overflow. There's probably more places, but this is what I found
by visual inspection and compiler warnings.
0003) Removes -fwrapv. I'm *NOT* suggesting we apply this right now, but
it seems like an important test for the new facilities. Without
0002, tests would fail after this, after it all tests run
successfully.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers