[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 marked 3 inline comments as done.
dyaroshev added inline comments.


================
Comment at: include/algorithm:760
+
+template <typename _Tp>
+_LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11 _Tp
----------------
dyaroshev wrote:
> 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.
Just used make_unsigned.


================
Comment at: test/libcxx/algorithms/half_positive.pass.cpp:27
+int main()
+{
+    {
----------------
dyaroshev wrote:
> 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?
found in the docs.

Instead of feature tests, used internal macroses.


https://reviews.llvm.org/D52697