[libc-commits] [libc] [libc] memmove optimizations (PR #70043)

Guillaume Chatelet via libc-commits libc-commits at lists.llvm.org
Wed Oct 25 01:39:34 PDT 2023


gchatelet wrote:

> Oh, "Rebase and merge" is not enabled, that's sad. Do you know if I am supposed to apply this to main branch locally and try to push until it succeeds? Or there is a better way?

Yeah we'll need to push individual commits as separate PRs.

The first one -concerning the benchmark- LGTM can go in independently. A word of caution though on the sweeping benchmark. It exercises the same size many times in a row to accumulate measurement precision, as a consequence it will benefit from branch prediction and gives idealized results that may not be realized in production. It gives an upper bound though.

For the second one I think it's a good idea to factor in small sizes. The code duplication is suboptimal. But I'm not sure if introducing the `fast_only` argument is the way to go, it makes the logic spread across several files and is harder to reason about. I need to think about it.

The third patch looks good on the intent but I don't get the boolean addition in the `if`s. e.g.,
```
if (count < 16 + (vector_size <= sizeof(uint64_t)))
```
This is either `if (count < 16)` or `if (count < 17)` depending on vector size. Is this the logic you meant to implement? If so, it's a bit too cryptic as-is and would benefit from more comments or more self-explanatory code.

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


More information about the libc-commits mailing list