[libc-commits] [libc] [libc] Speed up memmove overlapping check (PR #70017)
Guillaume Chatelet via libc-commits
libc-commits at lists.llvm.org
Tue Oct 24 03:03:42 PDT 2023
================
@@ -89,16 +89,10 @@ template <size_t alignment, typename T> LIBC_INLINE T *assume_aligned(T *ptr) {
// Returns true iff memory regions [p1, p1 + size] and [p2, p2 + size] are
// disjoint.
LIBC_INLINE bool is_disjoint(const void *p1, const void *p2, size_t size) {
- const char *a = static_cast<const char *>(p1);
- const char *b = static_cast<const char *>(p2);
- if (a > b) {
- // Swap a and b, this compiles down to conditionnal move for aarch64, x86
- // and RISCV with zbb extension.
- const char *tmp = a;
- a = b;
- b = tmp;
- }
- return a + size <= b;
+ const size_t diff =
+ static_cast<const char *>(p1) - static_cast<const char *>(p2);
+ // This is expected to compile to conditional move.
+ return static_cast<ptrdiff_t>(diff) >= 0 ? size <= diff : size <= -diff;
----------------
gchatelet wrote:
Thx for the improved codegen!
There are a number of assumptions that I would like to clear out explicitly.
- The result type of `static_cast<const char *>(p1) - static_cast<const char *>(p2)` is `ptrdiff_t` so the first line goes through integer conversion from signed to unsigned. The standard definition [is convoluted](https://en.cppreference.com/w/cpp/language/implicit_conversion#:~:text=If%20the%20destination,or%20truncated%20respectively.) but [practically speaking](https://en.cppreference.com/w/cpp/language/types#:~:text=Prior%20to%20C,char%20type%20object.) this is a `bit_cast` (although LLVM libc is c++17 and not c++20).
- The unary minus goes through [integer promotion](https://en.cppreference.com/w/cpp/language/operator_arithmetic#:~:text=For%20the%20built%2Din%20unary%20minus,the%20promoted%20type%20of%20expression.). It should be fine on all platforms but I'm not entirely convinced.
I think we can clear them by rewriting the code like so:
```
const ptrdiff_t sdiff =
static_cast<const char *>(p1) - static_cast<const char *>(p2);
const size_t udiff = cpp::bit_cast<size_t>(sdiff);
const size_t neg_udiff = cpp::bit_cast<size_t>(-sdiff); // integer promition would be caught here.
// This is expected to compile a conditional move.
return sdiff >=0 ? size <= udiff : size <= neg_udiff;
```
https://github.com/llvm/llvm-project/pull/70017
More information about the libc-commits
mailing list