Re: Using 128-bit integers for sum, avg and statistics aggregates

From: Peter Geoghegan <pg(at)heroku(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andreas Karlsson <andreas(at)proxel(dot)se>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Using 128-bit integers for sum, avg and statistics aggregates
Date: 2015-03-18 22:59:52
Message-ID: CAM3SWZTNDKe3SvBG1iBU32Tz3+7Z5xgW3AxHxst761208ZvxPg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Mar 18, 2015 at 3:16 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
>> For another, Andreas has chosen to lump together __int128 and unsigned
>> __int128 into one test, where the latter really doesn't receive
>> coverage.
>
> On my urging actually. It's pretty darn unlikely that only one variant
> will work.

I think that makes sense. Since we're targeting GCC here, and we're
comfortable with that, I guess that's okay after all.

> The reason we need a link test (vs just a compile test) is that gcc
> links to helper functions to do math - even if they're not present on
> the target platform. Compiling will succeed, but linking won't.

Okay. Attached revision has a few tweaks that reflect the status of
int128/uint128 as specialized types that are basically only useful for
this optimization, or other similar optimizations on compilers that
either are GCC, or aim to be compatible with it. I don't think
Andreas' V9 reflected that sufficiently.

Also, I now always use PolyNumAggState (the typedef), even for #define
HAVE_INT128 code, which I think is a bit clearer. Note that I have
generated a minimal diff, without the machine generated changes that
are ordinarily included in the final commit when autoconf tests are
added, mostly because I do not have the exact version of autoconf on
my development machine required to do this without creating irrelevant
cruft.

Marked "Ready for committer".

A committer that has the exact right version of autoconf (GNU Autoconf
2.69), or perhaps Andreas can build the machine generated parts.
Andres may wish to take another look at the autoconf changes.

--
Peter Geoghegan

Attachment Content-Type Size
int128-agg-v10.patch text/x-patch 36.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-03-18 23:07:40 Re: Using 128-bit integers for sum, avg and statistics aggregates
Previous Message Andres Freund 2015-03-18 22:16:14 Re: Using 128-bit integers for sum, avg and statistics aggregates