Specialize find and count for vector<bool>

[miscco]

This builds uppon the recently merged specialization of fill for vector<bool>.

Note that the specializations require __cpp_lib_bitops

[barcharcraz]

reviewed use of bit, and some on functionality. I need to become more familiar with how vector works before I can give more functional feedback, but this should outline next steps.

Sorry it took me so long to get to this.

[miscco]

@barcharcraz can you give an update on the status of the builtins?

[barcharcraz]

@barcharcraz can you give an update on the status of the builtins?

we have no intention of adding the builtins, and they have been removed. We found the current way we implement these functions to be better given how our compiler is put together.

You'll have to move popcount to some header that vector can include.

[miscco]

Rebased and squashed down now using std::popcount

[miscco]

@barcharcraz I believe this is ready for another round of review

[StephanTLavavej]

I've pushed a merge with master. There was a merge conflict in <bit> caused by clang-format 11 adjusting whitespace in code you removed; I accepted your change.

[miscco]

Thanks a lot, I was 99% sure that i already merge master.
Seems like not pushing is a bad idea

[miscco]

Urgh, Seems I has an unclean state and the use cbegin for fill commit is still there 🤦

[miscco]

@barcharcraz are there any further blockers here?

[barcharcraz]

Approved with suggestions (it still needs final review). Also: before / after benchmarks would be awesome.

[barcharcraz]

this is not a super helpful comment and could be removed (we generally remove these sorts of comments when we see them, as there are many in older code). It doesn't need to block this PR though.

[miscco]

I would keep it as we have it in all others

[miscco]

So I ran the itsy.bitsy benchmarks from the wonderfull @ThePhD

The relevant results are for current version:

Running C:\ws\itsy_bitsy\build\x64\bin\Release\itsy.bitsy.benchmarks.exe
Run on (8 X 1992 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x4)
  L1 Instruction 32 KiB (x4)
  L2 Unified 256 KiB (x4)
  L3 Unified 8192 KiB (x1)
--------------------------------------------------------------------------
Benchmark                                Time             CPU   Iterations
--------------------------------------------------------------------------
count_vector_bool                    95765 ns        96257 ns         7467
count_itsy_bitsy                       842 ns          854 ns       896000
find_vector_bool                     84993 ns        85449 ns         8960
find_itsy_bitsy                      49712 ns        50000 ns        10000

With this PR:

Running C:\ws\itsy_bitsy\build\x64\bin\Release\itsy.bitsy.benchmarks.exe
Run on (8 X 1992 MHz CPU s)
CPU Caches:
  L1 Data 32 KiB (x4)
  L1 Instruction 32 KiB (x4)
  L2 Unified 256 KiB (x4)
  L3 Unified 8192 KiB (x1)
--------------------------------------------------------------------------
Benchmark                                Time             CPU   Iterations
--------------------------------------------------------------------------
count_vector_bool                     4666 ns         4353 ns       186667
count_itsy_bitsy                      1607 ns         1562 ns       640000
find_vector_bool                      3205 ns         3209 ns       224000
find_itsy_bitsy                      50459 ns        51562 ns        10000

So we are still losing out against itsy.bitsy, which is not unexpected. But the performance increase is

count:    95'000 ns -> 5'000ns -> 19x improvement
find:     85'000ns  -> 3'200ns -> 26x improvement

I believe this is no too shabby

[miscco]

@barcharcraz I added the optimizations to ranges::count and ranges::find

[StephanTLavavej]

Internal testing revealed that /clr:pure rejects this intrinsic (as it does for almost all intrinsics). I belatedly realized that CUDA and the Intel C++ Compiler may also be incompatible. I've extracted these conditions into a macro and verified that /clr:pure is happy now.

_Countr_zero's usage of tzcnt is unaffected because it behaves differently - it uses the constexpr fallback for C++17 mode, and only for C++20 mode (when is_constant_evaluated() is available) does it consider activating the runtime optimization. This inherently excludes /clr:pure and also excludes CUDA for the time being (and possibly the Intel C++ Compiler).

[StephanTLavavej]

Thanks for these order-of-magnitude speedups! 🚀 🚀 🚀