[libcxx-commits] [libcxx] [libc++] Refactor std::{fill_n, count} for vector<bool>::iterator (PR #120305)

[Nikolas Klauser via libcxx-commits]

philnik777 wrote:

Sorry, I thought I already commented. I don't remember the details, but I'm quite certain this has a non-trivial performance impact due to a significantly larger number of branches and clang probably not being able to vectorize the aligned case (haven't checked though). It also makes constant folding less likely, since the non-aligned code paths are much more complicated than the aligned ones and inlining being mostly done on function boundaries.

> In the meantime, I've double-checked the current implementation. While it uses the bool parameter as a NTTP, the if condition checks on the bool values are all performed at runtime. I think we may improve the current implementation in the following two directions:
> 
> 1. **Performance**: If we are concerned about the performance impact due to runtime if, we should leverage the compile-time benefits of NTTP by using `constexpr if` (C++17 and later) or templates with SFINAE (C++14 and earlier). Using NTTP while still performing runtime if still incurs unnecessary overhead.

They are technically runtime checks, but the compiler will constant fold it even at -O0. There is no real benefit to your suggested changes. They may ecen be worse, since sfinae is quite expensive.



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