Remove fls(), use pg_leftmost_one_pos32() instead.

Commit 4f658dc8 provided the traditional BSD fls() function in
src/port/fls.c so it could be used in several places.  Later we added a
bunch of similar facilities in pg_bitutils.h, based on compiler
builtins that map to hardware instructions.  It's a bit confusing to
have both 1-based and 0-based variants of this operation in use in
different parts of the tree, and neither is blessed by a standard.
Let's drop fls.c and the configure probe, and reuse the newer code.

Reviewed-by: David Rowley <[email protected]>
Reviewed-by: Tom Lane <[email protected]>
Discussion: https://postgr.es/m/CA%2BhUKG%2B7dSX1XF8yFGmYk-%3D48dbjH2kmzZj16XvhbrWP-9BzRg%40mail.gmail.com

diff --git a/configure b/configure
index 59fa82b8d7aea5dcb8660a03fcbaddabf4b8477c..33a425562f147a668a810c1df61c28010259622f 100755 (executable)
--- 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 612dabf6988ffbaecf7e614a5871e4a41ae0df3d..5295cb58065ca5e24e50fdaaf7ef136d53b18cb6 100644 (file)
--- 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 fe37bc47cbb2e641672bca7c0199138a105935cf..32822dbb6b928b58cb28dc9fcee2b1695a876f1f 100644 (file)
--- 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_leftmost_one_pos32(new_bucket)) - 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 028d9e168080dc0fc462e52394862abee83f7b2b..63f0f6b79c7613bb62cfe4743b4abc51d582d430 100644 (file)
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -47,6 +47,7 @@
 #include "parser/parsetree.h"
 #include "partitioning/partbounds.h"
 #include "partitioning/partprune.h"
+#include "port/pg_bitutils.h"
 #include "rewrite/rewriteManip.h"
 #include "utils/lsyscache.h"
 
@@ -1491,7 +1492,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_leftmost_one_pos32(list_length(live_childrels)) + 1);
            parallel_workers = Min(parallel_workers,
                                   max_parallel_workers_per_gather);
        }
@@ -1542,7 +1543,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_leftmost_one_pos32(list_length(live_childrels)) + 1);
        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 2214920dea4d9f2d99f174081072740a2bfd7b23..043181b586bccc51f785798033e17d30134eecb2 100644 (file)
--- a/src/backend/optimizer/prep/prepunion.c
+++ b/src/backend/optimizer/prep/prepunion.c
@@ -675,7 +675,7 @@ generate_union_paths(SetOperationStmt *op, PlannerInfo *root,
        if (enable_parallel_append)
        {
            parallel_workers = Max(parallel_workers,
-                                  fls(list_length(partial_pathlist)));
+                                  pg_leftmost_one_pos32(list_length(partial_pathlist)) + 1);
            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 b6cb8fa13dc9b9aaa9cd7b085ed6de458d878223..82376fde2d98d9f98c70f0dfef72abf30363a54b 100644 (file)
--- 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,18 @@ 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)
+static inline size_t
+contiguous_pages_to_segment_bin(size_t n)
+{
+   size_t      bin;
+
+   if (n == 0)
+       bin = 0;
+   else
+       bin = pg_leftmost_one_pos_size_t(n) + 1;
+
+   return Min(bin, 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 529fb84a86c2e49966fafeff5bc68ce678197d00..86d0b1941bb23bc06e214519c163c59aedf252e9 100644 (file)
--- 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 e647f62b779d9b55ce99832a8c8697b224719070..d39b04141f994d5f0f61921a594314cf6b7ee47a 100644 (file)
--- 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/port/fls.c b/src/port/fls.c
deleted file mode 100644 (file)
index 19b4221..0000000
--- 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 cc7a908d10a982f576e1b142368bbe08d22ffd73..c935f776e51badd9281b875f2c4714421279a7c0 100644 (file)
--- 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 fdatasync.c
+     chklocale.c explicit_bzero.c fdatasync.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

diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 3ddcd024a78c11b71f50a336efbc64baff47bf4c..23296527aed50a8f691060792017bf48f658b4f8 100644 (file)
--- 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                              => 1,
-       HAVE_FLS                                    => undef,
        HAVE_FSEEKO                                 => 1,
        HAVE_FUNCNAME__FUNC                         => undef,
        HAVE_FUNCNAME__FUNCTION                     => 1,