Hi,
Back in commit 4f658dc8 we gained src/port/fls.c. As anticipated by
its commit message, we later finished up with something better in
src/include/port/pg_bitutils.h. fls() ("find last set") is an
off-by-one cousin of pg_leftmost_one_pos32(). I don't know why ffs()
("find first set", the rightmost variant) made it into POSIX while
fls() did not, other than perhaps its more amusing name. fls() is
present on *BSD, Macs and maybe more, but not everywhere, hence the
configure test. Let's just do it with pg_bitutils.h instead, and drop
some cruft? Open to better ideas on whether we need a new function,
or there is some way to use the existing facilities directly without
worrying about undefined behaviour for 0, etc.
Noticed while looking for configure stuff to cull. Mentioning
separately because this isn't a simple
no-longer-needed-crutch-for-prestandard-system case like the others in
a nearby thread.
From 0260f65533bfdff024c5af6c0b14692484305b06 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Tue, 19 Jul 2022 17:47:10 +1200
Subject: [PATCH] Remove replacement fls function.
Commit 4f658dc8 provided the traditional BSD fls() function so it could
be used in several places. Later we added a bunch of similar sorts of
things in pg_bitutils.h. Let's drop fls.c and the configure probe, and
reuse the newer code.
---
configure | 13 ------
configure.ac | 1 -
src/backend/access/hash/hashutil.c | 2 +-
src/backend/optimizer/path/allpaths.c | 4 +-
src/backend/optimizer/prep/prepunion.c | 2 +-
src/backend/utils/mmgr/dsa.c | 3 +-
src/include/pg_config.h.in | 3 --
src/include/port.h | 4 --
src/include/port/pg_bitutils.h | 15 ++++++
src/port/fls.c | 64 --------------------------
src/tools/msvc/Mkvcbuild.pm | 2 +-
src/tools/msvc/Solution.pm | 1 -
12 files changed, 22 insertions(+), 92 deletions(-)
delete mode 100644 src/port/fls.c
diff --git a/configure b/configure
index a4f4d321fb..4db38683e5 100755
--- a/configure
+++ b/configure
@@ -16771,19 +16771,6 @@ esac
fi
-ac_fn_c_check_func "$LINENO" "fls" "ac_cv_func_fls"
-if test "x$ac_cv_func_fls" = xyes; then :
- $as_echo "#define HAVE_FLS 1" >>confdefs.h
-
-else
- case " $LIBOBJS " in
- *" fls.$ac_objext "* ) ;;
- *) LIBOBJS="$LIBOBJS fls.$ac_objext"
- ;;
-esac
-
-fi
-
ac_fn_c_check_func "$LINENO" "getopt" "ac_cv_func_getopt"
if test "x$ac_cv_func_getopt" = xyes; then :
$as_echo "#define HAVE_GETOPT 1" >>confdefs.h
diff --git a/configure.ac b/configure.ac
index 5bd29a4d2f..b826823e50 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1911,7 +1911,6 @@ fi
AC_REPLACE_FUNCS(m4_normalize([
dlopen
explicit_bzero
- fls
getopt
getpeereid
getrusage
diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index fe37bc47cb..856462b641 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -436,7 +436,7 @@ _hash_get_oldblock_from_newbucket(Relation rel, Bucket new_bucket)
* started. Masking the most significant bit of new bucket would give us
* old bucket.
*/
- mask = (((uint32) 1) << (fls(new_bucket) - 1)) - 1;
+ mask = (((uint32) 1) << (pg_fls(new_bucket) - 1)) - 1;
old_bucket = new_bucket & mask;
metabuf = _hash_getbuf(rel, HASH_METAPAGE, HASH_READ, LH_META_PAGE);
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index 358bb2aed6..6f19721751 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -1491,7 +1491,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
if (enable_parallel_append)
{
parallel_workers = Max(parallel_workers,
- fls(list_length(live_childrels)));
+ pg_fls(list_length(live_childrels)));
parallel_workers = Min(parallel_workers,
max_parallel_workers_per_gather);
}
@@ -1542,7 +1542,7 @@ add_paths_to_append_rel(PlannerInfo *root, RelOptInfo *rel,
* the planned number of parallel workers.
*/
parallel_workers = Max(parallel_workers,
- fls(list_length(live_childrels)));
+ pg_fls(list_length(live_childrels)));
parallel_workers = Min(parallel_workers,
max_parallel_workers_per_gather);
Assert(parallel_workers > 0);
diff --git a/src/backend/optimizer/prep/prepunion.c b/src/backend/optimizer/prep/prepunion.c
index f004fad1d9..3af7b55e7d 100644
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -668,7 +668,7 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
if (enable_parallel_append)
{
parallel_workers = Max(parallel_workers,
- fls(list_length(partial_pathlist)));
+ pg_fls(list_length(partial_pathlist)));
parallel_workers = Min(parallel_workers,
max_parallel_workers_per_gather);
}
diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c
index b6cb8fa13d..80e92d80f3 100644
--- a/src/backend/utils/mmgr/dsa.c
+++ b/src/backend/utils/mmgr/dsa.c
@@ -51,6 +51,7 @@
#include "postgres.h"
#include "port/atomics.h"
+#include "port/pg_bitutils.h"
#include "storage/dsm.h"
#include "storage/ipc.h"
#include "storage/lwlock.h"
@@ -137,7 +138,7 @@ typedef size_t dsa_segment_index;
* free pages? There is no point in looking in segments in lower bins; they
* definitely can't service a request for n free pages.
*/
-#define contiguous_pages_to_segment_bin(n) Min(fls(n), DSA_NUM_SEGMENT_BINS - 1)
+#define contiguous_pages_to_segment_bin(n) Min(pg_fls(n), DSA_NUM_SEGMENT_BINS - 1)
/* Macros for access to locks. */
#define DSA_AREA_LOCK(area) (&area->control->lock)
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 529fb84a86..86d0b1941b 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -191,9 +191,6 @@
/* Define to 1 if you have the `fdatasync' function. */
#undef HAVE_FDATASYNC
-/* Define to 1 if you have the `fls' function. */
-#undef HAVE_FLS
-
/* Define to 1 if fseeko (and presumably ftello) exists and is declared. */
#undef HAVE_FSEEKO
diff --git a/src/include/port.h b/src/include/port.h
index e647f62b77..d39b04141f 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -366,10 +366,6 @@ extern int gettimeofday(struct timeval *tp, struct timezone *tzp);
#define pgoff_t off_t
#endif
-#ifndef HAVE_FLS
-extern int fls(int mask);
-#endif
-
#ifndef HAVE_GETPEEREID
/* On Windows, Perl might have incompatible definitions of uid_t and gid_t. */
#ifndef PLPERL_HAVE_UID_GID
diff --git a/src/include/port/pg_bitutils.h b/src/include/port/pg_bitutils.h
index 04e58cd1c4..209e5f68d0 100644
--- a/src/include/port/pg_bitutils.h
+++ b/src/include/port/pg_bitutils.h
@@ -247,6 +247,21 @@ pg_ceil_log2_64(uint64 num)
return pg_leftmost_one_pos64(num - 1) + 1;
}
+/*
+ * Like pg_leftmost_one_pos32(), except that the least significant bit is
+ * called 1, not 0. This means that 0 can be accepted as an input, returning
+ * 0. This function is traditionally known as fls() on BSD-derived systems,
+ * but was not standardized.
+ */
+static inline int
+pg_fls(int num)
+{
+ if (num == 0)
+ return 0;
+ else
+ return pg_leftmost_one_pos32(num) + 1;
+}
+
/*
* With MSVC on x86_64 builds, try using native popcnt instructions via the
* __popcnt and __popcnt64 intrinsics. These don't work the same as GCC's
diff --git a/src/port/fls.c b/src/port/fls.c
deleted file mode 100644
index 19b4221826..0000000000
--- a/src/port/fls.c
+++ /dev/null
@@ -1,64 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * fls.c
- * finds the last (most significant) bit that is set
- *
- * Portions Copyright (c) 1996-2022, PostgreSQL Global Development Group
- *
- *
- * IDENTIFICATION
- * src/port/fls.c
- *
- * This file was taken from FreeBSD to provide an implementation of fls()
- * for platforms that lack it. Note that the operating system's version may
- * be substantially more efficient than ours, since some platforms have an
- * assembly instruction that does exactly this.
- *
- * The FreeBSD copyright terms follow.
- */
-
-/*-
- * Copyright (c) 1990, 1993
- * The Regents of the University of California. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in the
- * documentation and/or other materials provided with the distribution.
- * 4. Neither the name of the University nor the names of its contributors
- * may be used to endorse or promote products derived from this software
- * without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#include "c.h"
-
-/*
- * Find Last Set bit
- */
-int
-fls(int mask)
-{
- int bit;
-
- if (mask == 0)
- return (0);
- for (bit = 1; mask != 1; bit++)
- mask = (unsigned int) mask >> 1;
- return (bit);
-}
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index e4feda10fd..78adb9770a 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -99,7 +99,7 @@ sub mkvcbuild
$solution = CreateSolution($vsVersion, $config);
our @pgportfiles = qw(
- chklocale.c explicit_bzero.c fls.c getpeereid.c getrusage.c inet_aton.c
+ chklocale.c explicit_bzero.c getpeereid.c getrusage.c inet_aton.c
getaddrinfo.c gettimeofday.c inet_net_ntop.c kill.c open.c
snprintf.c strlcat.c strlcpy.c dirmod.c noblock.c path.c
dirent.c dlopen.c getopt.c getopt_long.c link.c
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 1e125aef94..1d23573d66 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -258,7 +258,6 @@ sub GenerateFiles
HAVE_EXECINFO_H => undef,
HAVE_EXPLICIT_BZERO => undef,
HAVE_FDATASYNC => undef,
- HAVE_FLS => undef,
HAVE_FSEEKO => 1,
HAVE_FUNCNAME__FUNC => undef,
HAVE_FUNCNAME__FUNCTION => 1,
--
2.36.1
