Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7518049

Browse files
committed
Prevent int128 from requiring more than MAXALIGN alignment.
Our initial work with int128 neglected alignment considerations, anoversight 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 chipssometimes enforce that.Raising MAXALIGN seems out of the question: the costs in wasted disk andmemory space would be significant, and there would also be an on-diskcompatibility break. Nor does it seem very practical to try to allow somedata structures to have more-than-MAXALIGN alignment requirement, as we'dhave to push knowledge of that throughout various code that copies datastructures around.The only way out of the box is to make type int128 conform to the system'salignment assumptions. Fortunately, gcc supports that via its__attribute__(aligned()) pragma; and since we don't currently supportint128 on non-gcc-workalike compilers, we shouldn't be losing any platformsupport this way.Although we could have just done pg_attribute_aligned(MAXIMUM_ALIGNOF) andcalled it a day, I did a little bit of extra work to make the code moreportable than that: it will also support int128 on compilers without__attribute__(aligned()), if the native alignment of their 128-bit-inttype is no more than that of int64.Add a regression test case that exercises the one known instance of theproblem, in parallel aggregation over a bigint column.This will need to be back-patched, along with the preparatory commit91aec93. But let's see what the buildfarm makes of it first.Discussion:https://postgr.es/m/20171110185747.31519.28038@wrigleys.postgresql.org
1 parent91aec93 commit7518049

File tree

8 files changed

+102
-12
lines changed

8 files changed

+102
-12
lines changed

‎config/c-compiler.m4

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,11 @@ undefine([Ac_cachevar])dnl
9696
# PGAC_TYPE_128BIT_INT
9797
# ---------------------
9898
# Check if __int128 is a working 128 bit integer type, and if so
99-
# define PG_INT128_TYPE to that typename. This currently only detects
100-
# a GCC/clang extension, but support for different environments may be
101-
# added in the future.
99+
# define PG_INT128_TYPE to that typename, and define ALIGNOF_PG_INT128_TYPE
100+
# as its alignment requirement.
101+
#
102+
# This currently only detects a GCC/clang extension, but support for other
103+
# environments may be added in the future.
102104
#
103105
# For the moment we only test for support for 128bit math; support for
104106
# 128bit literals and snprintf is not required.
@@ -128,6 +130,7 @@ return 1;
128130
[pgac_cv__128bit_int=no])])
129131
if test x"$pgac_cv__128bit_int" = xyes ; then
130132
AC_DEFINE(PG_INT128_TYPE,__int128,[Define to the name of a signed 128-bit integer type.])
133+
AC_CHECK_ALIGNOF(PG_INT128_TYPE)
131134
fi])# PGAC_TYPE_128BIT_INT
132135

133136

‎configure

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14864,7 +14864,10 @@ _ACEOF
1486414864

1486514865
# Compute maximum alignment of any basic type.
1486614866
# We assume long's alignment is at least as strong as char, short, or int;
14867-
# but we must check long long (if it exists) and double.
14867+
# but we must check long long (if it is being used for int64) and double.
14868+
# Note that we intentionally do not consider any types wider than 64 bits,
14869+
# as allowing MAXIMUM_ALIGNOF to exceed 8 would be too much of a penalty
14870+
# for disk and memory space.
1486814871

1486914872
MAX_ALIGNOF=$ac_cv_alignof_long
1487014873
iftest$MAX_ALIGNOF -lt$ac_cv_alignof_double;then
@@ -14924,7 +14927,7 @@ _ACEOF
1492414927
fi
1492514928

1492614929

14927-
#Check for extensions offering the integer scalar type __int128.
14930+
#Some compilers offer a 128-bit integer scalar type.
1492814931
{$as_echo"$as_me:${as_lineno-$LINENO}: checking for __int128">&5
1492914932
$as_echo_n"checking for __int128...">&6; }
1493014933
if${pgac_cv__128bit_int+:}false;then:
@@ -14974,6 +14977,41 @@ if test x"$pgac_cv__128bit_int" = xyes ; then
1497414977

1497514978
$as_echo"#define PG_INT128_TYPE __int128">>confdefs.h
1497614979

14980+
# The cast to long int works around a bug in the HP C Compiler,
14981+
# see AC_CHECK_SIZEOF for more information.
14982+
{$as_echo"$as_me:${as_lineno-$LINENO}: checking alignment of PG_INT128_TYPE">&5
14983+
$as_echo_n"checking alignment of PG_INT128_TYPE...">&6; }
14984+
if${ac_cv_alignof_PG_INT128_TYPE+:}false;then:
14985+
$as_echo_n"(cached)">&6
14986+
else
14987+
if ac_fn_c_compute_int"$LINENO""(long int) offsetof (ac__type_alignof_, y)""ac_cv_alignof_PG_INT128_TYPE""$ac_includes_default
14988+
#ifndef offsetof
14989+
# define offsetof(type, member) ((char *) &((type *) 0)->member - (char *) 0)
14990+
#endif
14991+
typedef struct { char x; PG_INT128_TYPE y; } ac__type_alignof_;";then:
14992+
14993+
else
14994+
iftest"$ac_cv_type_PG_INT128_TYPE" = yes;then
14995+
{ {$as_echo"$as_me:${as_lineno-$LINENO}: error: in\`$ac_pwd':">&5
14996+
$as_echo"$as_me: error: in\`$ac_pwd':">&2;}
14997+
as_fn_error 77"cannot compute alignment of PG_INT128_TYPE
14998+
See\`config.log' for more details""$LINENO" 5; }
14999+
else
15000+
ac_cv_alignof_PG_INT128_TYPE=0
15001+
fi
15002+
fi
15003+
15004+
fi
15005+
{$as_echo"$as_me:${as_lineno-$LINENO}: result:$ac_cv_alignof_PG_INT128_TYPE">&5
15006+
$as_echo"$ac_cv_alignof_PG_INT128_TYPE">&6; }
15007+
15008+
15009+
15010+
cat>>confdefs.h<<_ACEOF
15011+
#define ALIGNOF_PG_INT128_TYPE$ac_cv_alignof_PG_INT128_TYPE
15012+
_ACEOF
15013+
15014+
1497715015
fi
1497815016

1497915017
# Check for various atomic operations now that we have checked how to declare

‎configure.in

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1820,7 +1820,10 @@ AC_CHECK_ALIGNOF(double)
18201820

18211821
# Compute maximum alignment of any basic type.
18221822
# We assume long's alignment is at least as strong as char, short, or int;
1823-
# but we must check long long (if it exists) and double.
1823+
# but we must check long long (if it is being used for int64) and double.
1824+
# Note that we intentionally do not consider any types wider than 64 bits,
1825+
# as allowing MAXIMUM_ALIGNOF to exceed 8 would be too much of a penalty
1826+
# for disk and memory space.
18241827

18251828
MAX_ALIGNOF=$ac_cv_alignof_long
18261829
if test $MAX_ALIGNOF -lt $ac_cv_alignof_double ; then
@@ -1837,7 +1840,7 @@ AC_DEFINE_UNQUOTED(MAXIMUM_ALIGNOF, $MAX_ALIGNOF, [Define as the maximum alignme
18371840
AC_CHECK_TYPES([int8, uint8, int64, uint64], [], [],
18381841
[#include <stdio.h>])
18391842

1840-
#Check for extensions offering the integer scalar type __int128.
1843+
#Some compilers offer a 128-bit integer scalar type.
18411844
PGAC_TYPE_128BIT_INT
18421845

18431846
# Check for various atomic operations now that we have checked how to declare

‎src/include/c.h

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -376,13 +376,29 @@ typedef unsigned long long int uint64;
376376

377377
/*
378378
* 128-bit signed and unsigned integers
379-
*There currently is only a limited support for the type. E.g. 128bit
380-
*literals and snprintf are not supported; but math is.
379+
*There currently is only limited support for such types.
380+
*E.g. 128bit literals and snprintf are not supported; but math is.
381+
*Also, because we exclude such types when choosing MAXIMUM_ALIGNOF,
382+
*it must be possible to coerce the compiler to allocate them on no
383+
*more than MAXALIGN boundaries.
381384
*/
382385
#if defined(PG_INT128_TYPE)
383-
#defineHAVE_INT128
384-
typedefPG_INT128_TYPEint128;
385-
typedefunsignedPG_INT128_TYPEuint128;
386+
#if defined(pg_attribute_aligned)||ALIGNOF_PG_INT128_TYPE <=MAXIMUM_ALIGNOF
387+
#defineHAVE_INT128 1
388+
389+
typedefPG_INT128_TYPEint128
390+
#if defined(pg_attribute_aligned)
391+
pg_attribute_aligned(MAXIMUM_ALIGNOF)
392+
#endif
393+
;
394+
395+
typedefunsignedPG_INT128_TYPEuint128
396+
#if defined(pg_attribute_aligned)
397+
pg_attribute_aligned(MAXIMUM_ALIGNOF)
398+
#endif
399+
;
400+
401+
#endif
386402
#endif
387403

388404
/*

‎src/include/pg_config.h.in

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727
/* The normal alignment of `long long int', in bytes. */
2828
#undef ALIGNOF_LONG_LONG_INT
2929

30+
/* The normal alignment of `PG_INT128_TYPE', in bytes. */
31+
#undef ALIGNOF_PG_INT128_TYPE
32+
3033
/* The normal alignment of `short', in bytes. */
3134
#undef ALIGNOF_SHORT
3235

‎src/include/pg_config.h.win32

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,9 @@
3434
/* The alignment requirement of a `long long int'. */
3535
#define ALIGNOF_LONG_LONG_INT 8
3636

37+
/* The normal alignment of `PG_INT128_TYPE', in bytes. */
38+
#undef ALIGNOF_PG_INT128_TYPE
39+
3740
/* The alignment requirement of a `short'. */
3841
#define ALIGNOF_SHORT 2
3942

‎src/test/regress/expected/select_parallel.out

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,24 @@ select * from
444444

445445
reset enable_material;
446446
reset enable_hashagg;
447+
-- check parallelized int8 aggregate (bug #14897)
448+
explain (costs off)
449+
select avg(unique1::int8) from tenk1;
450+
QUERY PLAN
451+
-------------------------------------------------------------------------
452+
Finalize Aggregate
453+
-> Gather
454+
Workers Planned: 4
455+
-> Partial Aggregate
456+
-> Parallel Index Only Scan using tenk1_unique1 on tenk1
457+
(5 rows)
458+
459+
select avg(unique1::int8) from tenk1;
460+
avg
461+
-----------------------
462+
4999.5000000000000000
463+
(1 row)
464+
447465
-- gather merge test with a LIMIT
448466
explain (costs off)
449467
select fivethous from tenk1 order by fivethous limit 4;

‎src/test/regress/sql/select_parallel.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,12 @@ reset enable_material;
175175

176176
reset enable_hashagg;
177177

178+
-- check parallelized int8 aggregate (bug #14897)
179+
explain (costs off)
180+
selectavg(unique1::int8)from tenk1;
181+
182+
selectavg(unique1::int8)from tenk1;
183+
178184
-- gather merge test with a LIMIT
179185
explain (costs off)
180186
select fivethousfrom tenk1order by fivethouslimit4;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp