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

[Marshall Clow via Phabricator]

mclow.lists added a comment.

In general, this is the kind of optimization that I would rather see the compiler do (everywhere, automatically) than libc++ (here and there, manually).



================
Comment at: benchmarks/algorithms.bench.cpp:90
+BENCHMARK_CAPTURE(BM_LowerBound, random_int32,
+    getRandomIntegerInputs<int32_t>)->Arg(TestNumInputs);
 
----------------
Seems like we need more than 32-bit ints here.



================
Comment at: include/algorithm:4162
+
+    _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_AFTER_CXX11
+    bool operator()(_Reference __u)
----------------
Why constexpr after C++11?


================
Comment at: test/std/algorithms/alg.modifying.operations/alg.partitions/partition_point.pass.cpp:92
+    {
+        typedef typename std::__difference_type_to_size_t_cast<int>::__type cast_int;
+        static_assert(std::is_same<cast_int, size_t>::value, "");
----------------
You can't test the functionality of `__difference_type_to_size_t_cast` (a libc++-specific type) in the `test/std` hierarchy. 

That's what `test/libc++` tests are for.



================
Comment at: test/std/algorithms/alg.modifying.operations/alg.partitions/partition_point.pass.cpp:102
+
+        typedef typename std::__difference_type_to_size_t_cast<__int128_t>::__type cast_int128_t;
+        static_assert(std::is_same<cast_int128_t, __int128_t>::value, "");
----------------
This needs to be wrapped in a conditional checking to see if we have `__int128_t`


Repository:
  rCXX libc++

https://reviews.llvm.org/D52697