On 18/09/17 16:54 -0700, Tim Shen wrote:
On Mon, Sep 18, 2017 at 4:01 PM, Jonathan Wakely <jwak...@redhat.com> wrote:
On 18/09/17 10:58 -0700, Tim Shen via libstdc++ wrote:
On Mon, Sep 18, 2017 at 10:26 AM, Jonathan Wakely <jwak...@redhat.com>
wrote:
We need to rewrite this to check the lengths are equal first, and then
call the 3-argument version of std::equal.
Alternatively, we could move the implementation of the C++14
std::equal overloads to __equal and make that available for C++11.
I'll try that.
Here's a proof of concept patch for that. It's a bit ugly.
Instead of having iterator tags in the interface, we can probe the
random-access-ness inside __equal4/__equal4_p, can't we? It's similar
to the existing "if (_RAIters()) { ... }".
I'd expect the patches to be renaming the current implementations and
adding wrappers, instead of adding new implementations.
Well I decided to split the existing functions up and use tag
dispatching, which is conceptually cleaner anyway. But as the
RandomAccessIterator version doesn't need any operations that aren't
valid for other categories, it's not strictly necessary. The tag
dispatching version should generate slightly smaller code for
unoptimized builds, but that's not very important.
Unoptimized builds don't inline small functions, therefore the first
patch generate two weak symbols, instead of one by the second patch.
Two small functions that only do the necessary work, rather than one
large function that has a branch for RAIters even when it can never be
taken.
It's unclear to me how would number of symbols penalize the
performance/binary size.
People who care about performance or binary size should be optimizing,
and in that case the RAIters branch will be known at compile-time and
the dead code should get removed, and the wrapper functions inlined.
Here's the patch doing it as you suggest. We can't call the new
functions __equal because t hat name is already taken by a helper
struct, hence __equal4.
Do you prefer this version?
Yes, I prefer this version for readability reasons:
1) subjectively, less scattered code; and
2) ideally I want `if constexpr (...)`), the if version is closer.
Yes, we could add _GLIBCXX17_CONSTEXPR there, but I'm not sure it's
worth doing.
3) The calls to __equal4 in _Backref_matcher are simpler.
I agree that it's not a big difference. I just wanted to point out the
small difference. I'm fine with either version.
I'll commit the second version.
Thanks for the prototyping!
--
Regards,
Tim Shen
