[llvm] r290685 - [NewGVN] replace emplace_back with push_back

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 28 17:02:03 PST 2016


> On Dec 28, 2016, at 2:34 PM, Piotr Padlewski via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 
> 2016-12-28 22:09 GMT+01:00 Daniel Berlin <dberlin at dberlin.org <mailto:dberlin at dberlin.org>>:
> 
> 
> On Wed, Dec 28, 2016 at 12:36 PM, Piotr Padlewski via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
> Author: prazek
> Date: Wed Dec 28 14:36:08 2016
> New Revision: 290685
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=290685&view=rev <http://llvm.org/viewvc/llvm-project?rev=290685&view=rev>
> Log:
> [NewGVN] replace emplace_back with push_back
> 
> I'm somewhat against this change.
> It would be nice to have coding guidelines here.
>  
> True, I gonna start thread about it soon. 
> 
> emplace_back is not faster if it is equivalent to push_back. In this cases emplaced value had the
> same type that the one stored in container. It is ugly and it might be even slower (see
> Scott Meyers presentation about emplacement).

From what I understand in the video you’re linking, the issue detailed is specifically with *emplace* and not *emplace_back*. 
It is acknowledge in the video that emplace_back is a potential win under some condition ( https://youtu.be/smqT9Io_bKo?t=2576 ) and I didn’t see any mention where emplace_back could be slower?


> 
> I'm going to disagree about "ugly", i don't think it's ugly or not ugly.
> If it's really slower, i'd like to see numbers.
> Otherwise, i think we should just be consistent if we can.
> 
> 
> Some numbers:
> https://youtu.be/smqT9Io_bKo?t=29m16s <https://youtu.be/smqT9Io_bKo?t=29m16s>
> 
> It is for map insert/emplace. It probably doesn't matter much for emplace_back, it only can add extra instantiations, 
> but I guess we don't want to have diffrent rules for using insert/emplace and push_back/emplace_back
> 
> There is also some useful info here:
> http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html <http://clang.llvm.org/extra/clang-tidy/checks/modernize-use-emplace.html>

I don’t see anything about emplace*_back* being slower here.

That said, I tend to restrict my use of emplace_back when I actually pass argument(s) of type different than T (i.e. I’m targeting another constructor than the copy-constructor).
I’d be fine with codifying something in http://llvm.org/docs/CodingStandards.html about this (push_back vs emplace_back).

— 
Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161228/9102eaf0/attachment.html>


More information about the llvm-commits mailing list