[libcxx-commits] [PATCH] D63063: Bug 42208: speeding up std::merge

[Roman Lebedev via Phabricator via libcxx-commits]

lebedev.ri resigned from this revision.
lebedev.ri added a comment.

In D63063#1539131 <https://reviews.llvm.org/D63063#1539131>, @dyaroshev wrote:

> In D63063#1536895 <https://reviews.llvm.org/D63063#1536895>, @lebedev.ri wrote:
>
> > > Binary size increase (godbolt: https://godbolt.org/z/b1ZFTA):
> > >  For std::string the size grows from 394 assembly instructions to 465 instructions (18%).
> > >  For int - from 62 to 64 (3%).
> >
> > Uhm.
> >  I have a question:
> >  you did notice that you are looking at libstdc++ implementation there?
> >  https://godbolt.org/z/WGSQ6r
>
>
> My bad - keep forgetting about that by default libstdc++ is used.


K.
I've messed around with that code, this *seems* like the right direction,
but this really screams optimizer failure. Regardless of whether or not
it should be workarounded in the library's code, it should be reported as such first

> Performance measurements were done using libc++ benchmarks and the results are correct.
>  Quickbench link is also good
> 
> Seems like libstdc++ does smth weird for coping strings, which leads to doubling of the size.
> 
> Binary size, std::string: 125 vs 184 (40%) - (much less than std::merge on libstdc++, which is 384), int : 54 vs 64  (18%)
> 
> Seems like since the copy produces less code, my collapsing of two memmoves into one brings less size decrease.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63063/new/

https://reviews.llvm.org/D63063