git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 1/3] wrapper: introduce `log2u()`

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 4, 2024, 08:53 UTC
Message-ID
<df8c5dffffed0edd9068406fcf39ac6608bc930f.1725439407.git.ps@pks.im>
In-Reply-To
<cover.1725439407.git.ps@pks.im>

We have an implementation of a function that computes the log2 for an integer. While we could instead use log2(3P), that involves floating point numbers and is thus needlessly complex and inefficient.

We're about to add a second caller that wants to compute log2 for a `size_t`. Let's thus move the function into "wrapper.h" such that it becomes generally available.

While at it, tweak the implementation a bit:
  - The parameter is converted from `int` to `uintmax_t`. This
    conversion is safe to do in "bisect.c" because we already check that
    the argument is positive.
  - The return value is an `unsigned`. It cannot ever be negative, so it
    is pointless for it to be a signed integer.
  - Loop until `!n` instead of requiring that `n > 1` and then subtract
    1 from the result and add a special case for `!sz`. This helps
    compilers to generate more efficient code.

Compilers recognize the pattern of this function and optimize accordingly. On GCC 14.2 x86_64:

    log2u(unsigned long):
            test    rdi, rdi
            je      .L3
            bsr     rax, rdi
            ret
    .L3:
            mov     eax, -1
            ret

Clang 18.1 does not yet recognize the pattern, but starts to do so on Clang trunk x86_64. The code isn't quite as efficient as the one generated by GCC, but still manages to optimize away the loop:

    log2u(unsigned long):
            test    rdi, rdi
            je      .LBB0_1
            shr     rdi
            bsr     rcx, rdi
            mov     eax, 127
            cmovne  rax, rcx
            xor     eax, -64
            add     eax, 65
            ret
    .LBB0_1:
            mov     eax, -1
            ret

The pattern is also recognized on other platforms like ARM64 GCC 14.2.0, where we end up using `clz`:

    log2u(unsigned long):
            clz     x2, x0
            cmp     x0, 0
            mov     w1, 63
            sub     w0, w1, w2
            csinv   w0, w0, wzr, ne
            ret

Note that we have a similar function `fastlog2()` in the reftable code. As that codebase is separate from the Git codebase we do not adapt it to use the new function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 bisect.c  | 12 +-----------
 wrapper.h | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/bisect.c b/bisect.c
index 4406fc44cf9..4713226fad4 100644
--- a/bisect.c
+++ b/bisect.c
@@ -1130,16 +1130,6 @@ enum bisect_error bisect_next_all(struct repository *r, const char *prefix)
 	return res;
 }
 
-static inline int log2i(int n)
-{
-	int log2 = 0;
-
-	for (; n > 1; n >>= 1)
-		log2++;
-
-	return log2;
-}
-
 static inline int exp2i(int n)
 {
 	return 1 << n;
@@ -1162,7 +1152,7 @@ int estimate_bisect_steps(int all)
 	if (all < 3)
 		return 0;
 
-	n = log2i(all);
+	n = log2u(all);
 	e = exp2i(n);
 	x = all - e;
 
diff --git a/wrapper.h b/wrapper.h
index 1b2b047ea06..a6b3e1f09ec 100644
--- a/wrapper.h
+++ b/wrapper.h
@@ -140,4 +140,22 @@ int csprng_bytes(void *buf, size_t len);
  */
 uint32_t git_rand(void);
 
+/* Provide log2 of the given `size_t`. */
+static inline unsigned log2u(uintmax_t sz)
+{
+	unsigned l = 0;
+
+	/*
+	 * Technically this isn't required, but it helps the compiler optimize
+	 * this to a `bsr` instruction.
+	 */
+	if (!sz)
+		return 0;
+
+	for (; sz; sz >>= 1)
+		l++;
+
+	return l - 1;
+}
+
 #endif /* WRAPPER_H */
-- 
2.46.0.519.g2e7b89e038.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 12 of 16 in “refs/files: use heuristic to decide whether to repack with `--auto`”
  1. 0/2 refs/files: use heuristic to decide whether to repack with `--auto`Patrick Steinhardt, Sep 2, 2024
  2. 1/2 t0601: merge tests for auto-packing of refsPatrick Steinhardt, Sep 2, 2024
  3. 2/2 refs/files: use heuristic to decide whether to repack with `--auto`Patrick Steinhardt, Sep 2, 2024
  4. karthik nayakSep 3, 2024
  5. Patrick SteinhardtSep 3, 2024
  6. Junio C HamanoSep 3, 2024
  7. Patrick SteinhardtSep 4, 2024
  8. Junio C HamanoSep 4, 2024
  9. Patrick SteinhardtSep 4, 2024
  10. karthik nayakSep 5, 2024
  11. 0/3 refs/files: use heuristics to decide whether to repack with `--auto`Patrick Steinhardt, Sep 4, 2024
  12. 1/3 wrapper: introduce `log2u()`Patrick Steinhardt, Sep 4, 2024
  13. 2/3 t0601: merge tests for auto-packing of refsPatrick Steinhardt, Sep 4, 2024
  14. 3/3 refs/files: use heuristic to decide whether to repack with `--auto`Patrick Steinhardt, Sep 4, 2024
  15. Junio C HamanoSep 4, 2024
  16. Patrick SteinhardtSep 5, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.