[PATCH] D53994: Fixing lower bound regression in certain situations.
Denis Yaroshevskiy via Phabricator
reviews at reviews.llvm.org
Wed Nov 14 05:49:12 PST 2018
dyaroshev added inline comments.
================
Comment at: include/algorithm:3231
{
- difference_type __l2 = __len / 2;
+ difference_type __l2 = _VSTD::__half_positive(__len);
_ForwardIterator __m = __first;
----------------
ldionne wrote:
> I'll admit to being extremely surprised this makes any difference whatsoever. In my experience, Clang always emits the same instruction no matter what: `shr`. Adding @jfb to take a look. If he's OK with this, I'm OK with this.
That's a great idea, I was as surprised as you are.
I don't think you can just shr - you don't know if the distance is positive: (l - f) kinda doesn't have to be (though std::distance can assert that as a post condition).
Here are a few ways to find the middle: https://godbolt.org/z/csAbsx
signed, unsigned and right shift (this is what libstdc++ does: https://github.com/gcc-mirror/gcc/blob/58fddb25d239007c7a9bff0d0969322d9c1709a8/libstdc%2B%2B-v3/include/bits/stl_algobase.h#L972)
They all produce a different codegen.
The unsigned one was the fastest (slightly faster than >>) and I wasn't 100% sure that calling >> was OK, while the way this patch does it should be.
https://reviews.llvm.org/D53994
More information about the libcxx-commits
mailing list