sebpop added a comment.
In https://reviews.llvm.org/D22782#512337, @EricWF wrote:
> I would love to see a benchmark with this, but I've done enough investigating
> on my own that I *know* this patch is beneficial.
This patch was motivated by perf analysis we did on a proprietary benchmark in
This revision was automatically updated to reflect the committed changes.
Closed by commit rL278356: Add 'inline' attribute to __init to inline the
basic_string's constructor (authored by spop).
Changed prior to commit:
https://reviews.llvm.org/D22782?vs=67597&id=67699#toc
Repository:
rL LLV
On Wed, Aug 10, 2016 at 5:39 PM Eric Fiselier wrote:
> On Mon, Aug 8, 2016 at 9:32 AM, David Blaikie via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> I'm still (as per another similar thread) a bit concerned this is working
>> around a compiler optimizer bug - I'd love to see a standal
EricWF accepted this revision.
EricWF added a reviewer: EricWF.
EricWF added a comment.
This revision is now accepted and ready to land.
I would love to see a benchmark with this, but I've done enough investigating
on my own that I *know* this patch is beneficial.
https://reviews.llvm.org/D2278
On Mon, Aug 8, 2016 at 9:32 AM, David Blaikie via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> I'm still (as per another similar thread) a bit concerned this is working
> around a compiler optimizer bug - I'd love to see a standalone example of
> this behavior (that adding the inline keyword
laxmansole updated this revision to Diff 67597.
laxmansole added a comment.
Added inline attribute to the forward/input iterator __init's.
Thanks @EricWF for suggestion.
https://reviews.llvm.org/D22782
Files:
libcxx/include/string
Index: libcxx/include/string
===
I'm still (as per another similar thread) a bit concerned this is working
around a compiler optimizer bug - I'd love to see a standalone example of
this behavior (that adding the inline keyword to an already
inline/available definition is what's causing the inlining) so we can look
at what the comp
hiraditya added a comment.
In https://reviews.llvm.org/D22782#504416, @EricWF wrote:
> The change itself LGTM, although we probably want to inline the forward/input
> iterator __init's as well.
>
> However I would like to see a small benchmark that demonstrates the
> performance change. Please
EricWF added a subscriber: EricWF.
EricWF added a comment.
The change itself LGTM, although we probably want to inline the forward/input
iterator __init's as well.
However I would like to see a small benchmark that demonstrates the performance
change. Please try and write the benchmark using Go
laxmansole added a comment.
Ping
https://reviews.llvm.org/D22782
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
laxmansole added a comment.
In https://reviews.llvm.org/D22782#495436, @mclow.lists wrote:
> Do we have a test for the problem that this is solving?
$ cat foo.cpp
int foo(const std::string name);
int main(){
return foo("bar");
}
$clang++ -S -O3 -fno-exceptions foo.cpp
Assembly ou
hiraditya added a comment.
In https://reviews.llvm.org/D22782#495436, @mclow.lists wrote:
> Do we have a test for the problem that this is solving?
We are looking at the libcxx testsuite, it seems there are only
correctness/functionality tests. Do you have any pointers on how to add tests
to
sebpop added a comment.
In https://reviews.llvm.org/D22782#495436, @mclow.lists wrote:
> Do we have a test for the problem that this is solving?
I think we can write a testcase that shows that copy constructors are not
optimized away unless the string constructor is inlined.
This patch fixes
mclow.lists added a comment.
Do we have a test for the problem that this is solving?
https://reviews.llvm.org/D22782
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
14 matches
Mail list logo
