[libc-commits] [libc] [libc][stdlib] Optimize bsearch() by reordering comparison branches (PR #230823)

[via libc-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: Kuan-Wei Chiu (visitorckw)

<details>
<summary>Changes</summary>

During a binary search, compare_result == 0 is triggered at most once, or 0 times when the key is not found, whereas compare_result < 0 and compare_result > 0 are triggered multiple times across loop iterations. Check < 0 and > 0 before the equality case so the loop does not test for == 0 first on every iteration.

With clang -O2, the loop body changes as follows:

x86-64:
```diff
        movq    %rbp, %rsi
        callq   *(%rsp)
        testl   %eax, %eax
-       je      .LBB0_7
        js      .LBB0_5
+       je      .LBB0_7
        notq    %r13
        addq    %rbx, %r13
        addq    %r14, %rbp
```

riscv64:
```diff
        mv      a0, s3
        mv      a1, s6
        jalr    s2
-       beqz    a0, .LBB0_10
        bltz    a0, .LBB0_5
+       beqz    a0, .LBB0_10
        not     s0, s0
        add     s0, s0, s1
        add     s5, s6, s4
```

The generated code now branches on < 0 first, then checks == 0 to exit, and falls through to the > 0 update.

---
Full diff: https://github.com/llvm/llvm-project/pull/230823.diff


1 Files Affected:

- (modified) libc/src/stdlib/bsearch.cpp (+3-4) 


``````````diff
diff --git a/libc/src/stdlib/bsearch.cpp b/libc/src/stdlib/bsearch.cpp
index f0848051173ef..7e76f3112bfc2 100644
--- a/libc/src/stdlib/bsearch.cpp
+++ b/libc/src/stdlib/bsearch.cpp
@@ -26,19 +26,18 @@ LLVM_LIBC_FUNCTION(void *, bsearch,
     const void *elem =
         reinterpret_cast<const uint8_t *>(array) + mid * elem_size;
     int compare_result = compare(key, elem);
-    if (compare_result == 0)
-      return const_cast<void *>(elem);
-
     if (compare_result < 0) {
       // This means that key is less than the element at |mid|.
       // So, in the next iteration, we only compare elements less
       // than mid.
       array_size = mid;
-    } else {
+    } else if (compare_result > 0) {
       // |mid| is strictly less than |array_size|. So, the below
       // decrement in |array_size| will not lead to a wrap around.
       array_size -= (mid + 1);
       array = reinterpret_cast<const uint8_t *>(elem) + elem_size;
+    } else {
+      return const_cast<void *>(elem);
     }
   }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/230823