Hi, On 2017-10-30 22:29:42 +0530, Robert Haas wrote: > On Mon, Oct 30, 2017 at 4:57 PM, Andres Freund <and...@anarazel.de> wrote: > > 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. > > Looks nice.
Thanks. > > Perhaps it should rather be pg_add_s32_overflow, or a similar > > naming scheme? > > Not sure what the s is supposed to be? Signed? Yes, signed. So we could add a u32 or something complementing the functions already in the patch. Even though overflow checks are a heck of a lot easier to write for unsigned ints, the intrinsics are still faster. I don't have any sort of strong feelings on the naming. > > 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. > > I lack the patience to review this tonight. Understandable ;) > > 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. > > I suggest that if we think we don't need -fwrapv any more, we ought to > remove it. Otherwise, we won't find out if we're wrong. I agree that we should do so at some point not too far away in the future. Not the least because we don't specify this kind of C dialect in a lot of other compilers. Additionally the flag causes some slowdown (because e.g. for loop variables are optimized less). But I'm fairly certain it needs a bit more care that I've invested as of now - should probably at least compile with -Wstrict-overflow=some-higher-level, and with ubsan. I'm fairly certain there's more bogus overflow checks around... 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