[PATCH] D52697: Bug 39129: Speeding up partition_point/lower_bound/upper_bound/ by using unsigned division by 2 when possible.

[Denis Yaroshevskiy via Phabricator]

dyaroshev added inline comments.


================
Comment at: include/algorithm:754
+// Perform division by two quickly for positive integers (llvm.org/PR39129)
+inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 ptrdiff_t
+__half_positive(ptrdiff_t __value)
----------------
EricWF wrote:
> I think we can use `_LIBCPP_CONSTEXPR` here and get better behavior in C++03. 
Sure, will do.

Why _LIBCPP_CONSTEXPR helps in C++03?


================
Comment at: include/algorithm:760
+
+template <typename _Tp>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 _Tp
----------------
EricWF wrote:
> While this version is certainly simplier,  I would still like to have this work for all of the builtin signed integral types. 
> 
Will do.
I tried experimenting with this a bit more, but it's quite hard to tell what divisions are actually faster.

The problem with my previous benchmark is that I cast from 64bit pointer to 32bit to do division.
While the question that I really want to answer is - is it a better to cast 32 bit thingy to 32 unsigned or to 64 bit unsigned to do the division.

Measuring it in isolation seems shady: http://quick-bench.com/Dn1QDyTLbYjH68qDydX3rM6Azko 

I tried to write an implementation of binary search that would reliably use 32bit arithmetics, but it doesn't really work out, since the pointer dereference is still 64 bit.

It's also important (I've seen a similar issue) that the unsigned division doesn't occupy an extra register, but definetely not when you measure this in isolation (see line seven in godbolt: https://gcc.godbolt.org/z/Fx5M6l)

My conclusion is:
Are you OK if I just do make_unsigned? Seems to be better than signed all the time but not 100% that it's the best cast.


================
Comment at: test/libcxx/algorithms/half_positive.pass.cpp:27
+int main()
+{
+    {
----------------
EricWF wrote:
> Please make sure this test compiles in C++03, C++11, ect. 
> 
> You'll want to use `TEST_STD_VER` from `test_macros.h` to guard the constexpr tests.
Will do. Can you please point me to how do I compile in C++03 mode?


https://reviews.llvm.org/D52697