Skip to content

Commit 457aef0

Browse files
alvherrealvherre
committed
Revert attempts to use POPCNT etc instructions
This reverts commits fc6c727, 109de05, d0b4663 and 711bab1. Somebody will have to try harder before submitting this patch again. I've spent entirely too much time on it already, and the #ifdef maze yet to be written in order for it to build at all got on my nerves. The amount of work needed to get a platform-specific performance improvement that's barely above the noise level is not worth it.
1 parent e89f14e commit 457aef0

14 files changed

+169
-709
lines changed

‎config/c-compiler.m4

-52
Original file line numberDiff line numberDiff line change
@@ -378,58 +378,6 @@ fi])# PGAC_C_BUILTIN_OP_OVERFLOW
378378

379379

380380

381-
# PGAC_C_BUILTIN_POPCOUNT
382-
# -------------------------
383-
AC_DEFUN([PGAC_C_BUILTIN_POPCOUNT],
384-
[AC_CACHE_CHECK([for __builtin_popcount], pgac_cv__builtin_popcount,
385-
[AC_COMPILE_IFELSE([AC_LANG_SOURCE(
386-
[static int x = __builtin_popcount(255);]
387-
)],
388-
[pgac_cv__builtin_popcount=yes],
389-
[pgac_cv__builtin_popcount=no])])
390-
if test x"$pgac_cv__builtin_popcount" = x"yes"; then
391-
AC_DEFINE(HAVE__BUILTIN_POPCOUNT, 1,
392-
[Define to 1 if your compiler understands __builtin_popcount.])
393-
fi])# PGAC_C_BUILTIN_POPCOUNT
394-
395-
396-
397-
# PGAC_C_BUILTIN_CTZ
398-
# -------------------------
399-
# Check if the C compiler understands __builtin_ctz(),
400-
# and define HAVE__BUILTIN_CTZ if so.
401-
AC_DEFUN([PGAC_C_BUILTIN_CTZ],
402-
[AC_CACHE_CHECK(for __builtin_ctz, pgac_cv__builtin_ctz,
403-
[AC_COMPILE_IFELSE([AC_LANG_SOURCE(
404-
[static int x = __builtin_ctz(256);]
405-
)],
406-
[pgac_cv__builtin_ctz=yes],
407-
[pgac_cv__builtin_ctz=no])])
408-
if test x"$pgac_cv__builtin_ctz" = xyes ; then
409-
AC_DEFINE(HAVE__BUILTIN_CTZ, 1,
410-
[Define to 1 if your compiler understands __builtin_ctz.])
411-
fi])# PGAC_C_BUILTIN_CTZ
412-
413-
414-
415-
# PGAC_C_BUILTIN_CLZ
416-
# -------------------------
417-
# Check if the C compiler understands __builtin_clz(),
418-
# and define HAVE__BUILTIN_CLZ if so.
419-
AC_DEFUN([PGAC_C_BUILTIN_CLZ],
420-
[AC_CACHE_CHECK(for __builtin_clz, pgac_cv__builtin_clz,
421-
[AC_COMPILE_IFELSE([AC_LANG_SOURCE(
422-
[static int x = __builtin_clz(256);]
423-
)],
424-
[pgac_cv__builtin_clz=yes],
425-
[pgac_cv__builtin_clz=no])])
426-
if test x"$pgac_cv__builtin_clz" = xyes ; then
427-
AC_DEFINE(HAVE__BUILTIN_CLZ, 1,
428-
[Define to 1 if your compiler understands __builtin_clz.])
429-
fi])# PGAC_C_BUILTIN_CLZ
430-
431-
432-
433381
# PGAC_C_BUILTIN_UNREACHABLE
434382
# --------------------------
435383
# Check if the C compiler understands __builtin_unreachable(),

‎configure

-119
Original file line numberDiff line numberDiff line change
@@ -651,7 +651,6 @@ CFLAGS_ARMV8_CRC32C
651651
CFLAGS_SSE42
652652
have_win32_dbghelp
653653
LIBOBJS
654-
have__builtin_popcount
655654
UUID_LIBS
656655
LDAP_LIBS_BE
657656
LDAP_LIBS_FE
@@ -733,7 +732,6 @@ CPP
733732
BITCODE_CXXFLAGS
734733
BITCODE_CFLAGS
735734
CFLAGS_VECTOR
736-
CFLAGS_POPCNT
737735
PERMIT_DECLARATION_AFTER_STATEMENT
738736
LLVM_BINPATH
739737
LLVM_CXXFLAGS
@@ -6582,48 +6580,6 @@ fi
65826580

65836581
fi
65846582

6585-
# Optimization flags and options for bit-twiddling
6586-
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether ${CC} supports -mpopcnt, for CFLAGS_POPCNT" >&5
6587-
$as_echo_n "checking whether ${CC} supports -mpopcnt, for CFLAGS_POPCNT... " >&6; }
6588-
if ${pgac_cv_prog_CC_cflags__mpopcnt+:} false; then :
6589-
$as_echo_n "(cached) " >&6
6590-
else
6591-
pgac_save_CFLAGS=$CFLAGS
6592-
pgac_save_CC=$CC
6593-
CC=${CC}
6594-
CFLAGS="${CFLAGS_POPCNT} -mpopcnt"
6595-
ac_save_c_werror_flag=$ac_c_werror_flag
6596-
ac_c_werror_flag=yes
6597-
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
6598-
/* end confdefs.h. */
6599-
6600-
int
6601-
main ()
6602-
{
6603-
6604-
;
6605-
return 0;
6606-
}
6607-
_ACEOF
6608-
if ac_fn_c_try_compile "$LINENO"; then :
6609-
pgac_cv_prog_CC_cflags__mpopcnt=yes
6610-
else
6611-
pgac_cv_prog_CC_cflags__mpopcnt=no
6612-
fi
6613-
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
6614-
ac_c_werror_flag=$ac_save_c_werror_flag
6615-
CFLAGS="$pgac_save_CFLAGS"
6616-
CC="$pgac_save_CC"
6617-
fi
6618-
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_prog_CC_cflags__mpopcnt" >&5
6619-
$as_echo "$pgac_cv_prog_CC_cflags__mpopcnt" >&6; }
6620-
if test x"$pgac_cv_prog_CC_cflags__mpopcnt" = x"yes"; then
6621-
CFLAGS_POPCNT="${CFLAGS_POPCNT} -mpopcnt"
6622-
fi
6623-
6624-
6625-
6626-
66276583
CFLAGS_VECTOR=$CFLAGS_VECTOR
66286584

66296585

@@ -14076,30 +14032,6 @@ if test x"$pgac_cv__builtin_bswap64" = xyes ; then
1407614032

1407714033
$as_echo "#define HAVE__BUILTIN_BSWAP64 1" >>confdefs.h
1407814034

14079-
fi
14080-
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_clz" >&5
14081-
$as_echo_n "checking for __builtin_clz... " >&6; }
14082-
if ${pgac_cv__builtin_clz+:} false; then :
14083-
$as_echo_n "(cached) " >&6
14084-
else
14085-
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
14086-
/* end confdefs.h. */
14087-
static int x = __builtin_clz(256);
14088-
14089-
_ACEOF
14090-
if ac_fn_c_try_compile "$LINENO"; then :
14091-
pgac_cv__builtin_clz=yes
14092-
else
14093-
pgac_cv__builtin_clz=no
14094-
fi
14095-
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
14096-
fi
14097-
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_clz" >&5
14098-
$as_echo "$pgac_cv__builtin_clz" >&6; }
14099-
if test x"$pgac_cv__builtin_clz" = xyes ; then
14100-
14101-
$as_echo "#define HAVE__BUILTIN_CLZ 1" >>confdefs.h
14102-
1410314035
fi
1410414036
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_constant_p" >&5
1410514037
$as_echo_n "checking for __builtin_constant_p... " >&6; }
@@ -14127,54 +14059,6 @@ if test x"$pgac_cv__builtin_constant_p" = xyes ; then
1412714059

1412814060
$as_echo "#define HAVE__BUILTIN_CONSTANT_P 1" >>confdefs.h
1412914061

14130-
fi
14131-
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_ctz" >&5
14132-
$as_echo_n "checking for __builtin_ctz... " >&6; }
14133-
if ${pgac_cv__builtin_ctz+:} false; then :
14134-
$as_echo_n "(cached) " >&6
14135-
else
14136-
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
14137-
/* end confdefs.h. */
14138-
static int x = __builtin_ctz(256);
14139-
14140-
_ACEOF
14141-
if ac_fn_c_try_compile "$LINENO"; then :
14142-
pgac_cv__builtin_ctz=yes
14143-
else
14144-
pgac_cv__builtin_ctz=no
14145-
fi
14146-
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
14147-
fi
14148-
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_ctz" >&5
14149-
$as_echo "$pgac_cv__builtin_ctz" >&6; }
14150-
if test x"$pgac_cv__builtin_ctz" = xyes ; then
14151-
14152-
$as_echo "#define HAVE__BUILTIN_CTZ 1" >>confdefs.h
14153-
14154-
fi
14155-
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_popcount" >&5
14156-
$as_echo_n "checking for __builtin_popcount... " >&6; }
14157-
if ${pgac_cv__builtin_popcount+:} false; then :
14158-
$as_echo_n "(cached) " >&6
14159-
else
14160-
cat confdefs.h - <<_ACEOF >conftest.$ac_ext
14161-
/* end confdefs.h. */
14162-
static int x = __builtin_popcount(255);
14163-
14164-
_ACEOF
14165-
if ac_fn_c_try_compile "$LINENO"; then :
14166-
pgac_cv__builtin_popcount=yes
14167-
else
14168-
pgac_cv__builtin_popcount=no
14169-
fi
14170-
rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
14171-
fi
14172-
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv__builtin_popcount" >&5
14173-
$as_echo "$pgac_cv__builtin_popcount" >&6; }
14174-
if test x"$pgac_cv__builtin_popcount" = x"yes"; then
14175-
14176-
$as_echo "#define HAVE__BUILTIN_POPCOUNT 1" >>confdefs.h
14177-
1417814062
fi
1417914063
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for __builtin_unreachable" >&5
1418014064
$as_echo_n "checking for __builtin_unreachable... " >&6; }
@@ -14693,9 +14577,6 @@ $as_echo "#define LOCALE_T_IN_XLOCALE 1" >>confdefs.h
1469314577

1469414578
fi
1469514579

14696-
have__builtin_popcount=$pgac_cv__builtin_popcount
14697-
14698-
1469914580
# MSVC doesn't cope well with defining restrict to __restrict, the
1470014581
# spelling it understands, because it conflicts with
1470114582
# __declspec(restrict). Therefore we define pg_restrict to the

‎configure.in

-9
Original file line numberDiff line numberDiff line change
@@ -547,10 +547,6 @@ elif test "$PORTNAME" = "hpux"; then
547547
PGAC_PROG_CXX_CFLAGS_OPT([+Olibmerrno])
548548
fi
549549

550-
# Optimization flags and options for bit-twiddling
551-
PGAC_PROG_CC_VAR_OPT(CFLAGS_POPCNT, [-mpopcnt])
552-
AC_SUBST(CFLAGS_POPCNT)
553-
554550
AC_SUBST(CFLAGS_VECTOR, $CFLAGS_VECTOR)
555551

556552
# Determine flags used to emit bitcode for JIT inlining. Need to test
@@ -1492,10 +1488,7 @@ PGAC_C_TYPES_COMPATIBLE
14921488
PGAC_C_BUILTIN_BSWAP16
14931489
PGAC_C_BUILTIN_BSWAP32
14941490
PGAC_C_BUILTIN_BSWAP64
1495-
PGAC_C_BUILTIN_CLZ
14961491
PGAC_C_BUILTIN_CONSTANT_P
1497-
PGAC_C_BUILTIN_CTZ
1498-
PGAC_C_BUILTIN_POPCOUNT
14991492
PGAC_C_BUILTIN_UNREACHABLE
15001493
PGAC_C_COMPUTED_GOTO
15011494
PGAC_STRUCT_TIMEZONE
@@ -1510,8 +1503,6 @@ AC_TYPE_LONG_LONG_INT
15101503

15111504
PGAC_TYPE_LOCALE_T
15121505

1513-
AC_SUBST(have__builtin_popcount, $pgac_cv__builtin_popcount)
1514-
15151506
# MSVC doesn't cope well with defining restrict to __restrict, the
15161507
# spelling it understands, because it conflicts with
15171508
# __declspec(restrict). Therefore we define pg_restrict to the

‎src/Makefile.global.in

-4
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,6 @@ CXX = @CXX@
260260
CFLAGS = @CFLAGS@
261261
CFLAGS_VECTOR = @CFLAGS_VECTOR@
262262
CFLAGS_SSE42 = @CFLAGS_SSE42@
263-
CFLAGS_POPCNT = @CFLAGS_POPCNT@
264263
CFLAGS_ARMV8_CRC32C = @CFLAGS_ARMV8_CRC32C@
265264
PERMIT_DECLARATION_AFTER_STATEMENT = @PERMIT_DECLARATION_AFTER_STATEMENT@
266265
CXXFLAGS = @CXXFLAGS@
@@ -517,9 +516,6 @@ WIN32_STACK_RLIMIT=4194304
517516
# Set if we have a working win32 crashdump header
518517
have_win32_dbghelp = @have_win32_dbghelp@
519518

520-
# Set if __builtin_popcount() is supported by $(CC)
521-
have__builtin_popcount = @have__builtin_popcount@
522-
523519
# Pull in platform-specific magic
524520
include $(top_builddir)/src/Makefile.port
525521

‎src/backend/access/heap/visibilitymap.c

+48-25
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,12 @@
8989
#include "access/visibilitymap.h"
9090
#include "access/xlog.h"
9191
#include "miscadmin.h"
92-
#include "port/pg_bitutils.h"
9392
#include "storage/bufmgr.h"
9493
#include "storage/lmgr.h"
9594
#include "storage/smgr.h"
9695
#include "utils/inval.h"
9796

97+
9898
/*#define TRACE_VISIBILITYMAP */
9999

100100
/*
@@ -115,9 +115,43 @@
115115
#define HEAPBLK_TO_MAPBYTE(x) (((x) % HEAPBLOCKS_PER_PAGE) / HEAPBLOCKS_PER_BYTE)
116116
#define HEAPBLK_TO_OFFSET(x) (((x) % HEAPBLOCKS_PER_BYTE) * BITS_PER_HEAPBLOCK)
117117

118-
/* Masks for bit counting bits in the visibility map. */
119-
#define VISIBLE_MASK64 0x5555555555555555 /* The lower bit of each bit pair */
120-
#define FROZEN_MASK64 0xaaaaaaaaaaaaaaaa /* The upper bit of each bit pair */
118+
/* tables for fast counting of set bits for visible and frozen */
119+
static const uint8 number_of_ones_for_visible[256] = {
120+
0, 1, 0, 1, 1, 2, 1, 2, 0, 1, 0, 1, 1, 2, 1, 2,
121+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
122+
0, 1, 0, 1, 1, 2, 1, 2, 0, 1, 0, 1, 1, 2, 1, 2,
123+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
124+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
125+
2, 3, 2, 3, 3, 4, 3, 4, 2, 3, 2, 3, 3, 4, 3, 4,
126+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
127+
2, 3, 2, 3, 3, 4, 3, 4, 2, 3, 2, 3, 3, 4, 3, 4,
128+
0, 1, 0, 1, 1, 2, 1, 2, 0, 1, 0, 1, 1, 2, 1, 2,
129+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
130+
0, 1, 0, 1, 1, 2, 1, 2, 0, 1, 0, 1, 1, 2, 1, 2,
131+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
132+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
133+
2, 3, 2, 3, 3, 4, 3, 4, 2, 3, 2, 3, 3, 4, 3, 4,
134+
1, 2, 1, 2, 2, 3, 2, 3, 1, 2, 1, 2, 2, 3, 2, 3,
135+
2, 3, 2, 3, 3, 4, 3, 4, 2, 3, 2, 3, 3, 4, 3, 4
136+
};
137+
static const uint8 number_of_ones_for_frozen[256] = {
138+
0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 2, 2, 1, 1, 2, 2,
139+
0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 2, 2, 1, 1, 2, 2,
140+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
141+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
142+
0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 2, 2, 1, 1, 2, 2,
143+
0, 0, 1, 1, 0, 0, 1, 1, 1, 1, 2, 2, 1, 1, 2, 2,
144+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
145+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
146+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
147+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
148+
2, 2, 3, 3, 2, 2, 3, 3, 3, 3, 4, 4, 3, 3, 4, 4,
149+
2, 2, 3, 3, 2, 2, 3, 3, 3, 3, 4, 4, 3, 3, 4, 4,
150+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
151+
1, 1, 2, 2, 1, 1, 2, 2, 2, 2, 3, 3, 2, 2, 3, 3,
152+
2, 2, 3, 3, 2, 2, 3, 3, 3, 3, 4, 4, 3, 3, 4, 4,
153+
2, 2, 3, 3, 2, 2, 3, 3, 3, 3, 4, 4, 3, 3, 4, 4
154+
};
121155

122156
/* prototypes for internal routines */
123157
static Buffer vm_readbuf(Relation rel, BlockNumber blkno, bool extend);
@@ -374,16 +408,18 @@ void
374408
visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_frozen)
375409
{
376410
BlockNumber mapBlock;
377-
BlockNumber nvisible = 0;
378-
BlockNumber nfrozen = 0;
379411

380412
/* all_visible must be specified */
381413
Assert(all_visible);
382414

415+
*all_visible = 0;
416+
if (all_frozen)
417+
*all_frozen = 0;
418+
383419
for (mapBlock = 0;; mapBlock++)
384420
{
385421
Buffer mapBuffer;
386-
uint64 *map;
422+
unsigned char *map;
387423
int i;
388424

389425
/*
@@ -400,30 +436,17 @@ visibilitymap_count(Relation rel, BlockNumber *all_visible, BlockNumber *all_fro
400436
* immediately stale anyway if anyone is concurrently setting or
401437
* clearing bits, and we only really need an approximate value.
402438
*/
403-
map = (uint64 *) PageGetContents(BufferGetPage(mapBuffer));
439+
map = (unsigned char *) PageGetContents(BufferGetPage(mapBuffer));
404440

405-
StaticAssertStmt(MAPSIZE % sizeof(uint64) == 0,
406-
"unsupported MAPSIZE");
407-
if (all_frozen == NULL)
408-
{
409-
for (i = 0; i < MAPSIZE / sizeof(uint64); i++)
410-
nvisible += pg_popcount64(map[i] & VISIBLE_MASK64);
411-
}
412-
else
441+
for (i = 0; i < MAPSIZE; i++)
413442
{
414-
for (i = 0; i < MAPSIZE / sizeof(uint64); i++)
415-
{
416-
nvisible += pg_popcount64(map[i] & VISIBLE_MASK64);
417-
nfrozen += pg_popcount64(map[i] & FROZEN_MASK64);
418-
}
443+
*all_visible += number_of_ones_for_visible[map[i]];
444+
if (all_frozen)
445+
*all_frozen += number_of_ones_for_frozen[map[i]];
419446
}
420447

421448
ReleaseBuffer(mapBuffer);
422449
}
423-
424-
*all_visible = nvisible;
425-
if (all_frozen)
426-
*all_frozen = nfrozen;
427450
}
428451

429452
/*

0 commit comments

Comments
 (0)