Prevent int128 from requiring more than MAXALIGN alignment.
We backported 128-bit integer support to speed up aggregates (commits 8122e143 and 959277a4) from upstream 9.6 into Greenplum (in commits 9b164486 and 325e6fcd). However, we forgot to also port a follow-up fix postgres/postgres@7518049980b, mostly because it's nuanced and hard to reproduce. There are two ways to tell the brokenness: 1. On a lucky day, tests would fail on my workstation, but not my laptop (or vice versa). 1. If you stare at the generated code for `int8_avg_combine` (and friends), you'll notice the compiler uses "aligned" instructions like `movaps` and `movdqa` (on AMD64). Today's my lucky day. Original commit message from postgres/postgres@7518049980b (by Tom Lane): > Our initial work with int128 neglected alignment considerations, an > oversight that came back to bite us in bug #14897 from Vincent Lachenal. > It is unsurprising that int128 might have a 16-byte alignment requirement; > what's slightly more surprising is that even notoriously lax Intel chips > sometimes enforce that. > Raising MAXALIGN seems out of the question: the costs in wasted disk and > memory space would be significant, and there would also be an on-disk > compatibility break. Nor does it seem very practical to try to allow some > data structures to have more-than-MAXALIGN alignment requirement, as we'd > have to push knowledge of that throughout various code that copies data > structures around. > The only way out of the box is to make type int128 conform to the system's > alignment assumptions. Fortunately, gcc supports that via its > __attribute__(aligned()) pragma; and since we don't currently support > int128 on non-gcc-workalike compilers, we shouldn't be losing any platform > support this way. > Although we could have just done pg_attribute_aligned(MAXIMUM_ALIGNOF) and > called it a day, I did a little bit of extra work to make the code more > portable than that: it will also support int128 on compilers without > __attribute__(aligned()), if the native alignment of their 128-bit-int > type is no more than that of int64. > Add a regression test case that exercises the one known instance of the > problem, in parallel aggregation over a bigint column. > This will need to be back-patched, along with the preparatory commit > 91aec93e. But let's see what the buildfarm makes of it first. > Discussion: https://postgr.es/m/20171110185747.31519.28038@wrigleys.postgresql.org (cherry picked from commit 75180499)
Showing
想要评论请 注册 或 登录