<html><head><meta content="text/html; charset=utf-8" http-equiv="Content-Type"></head><body><div><div style="font-family:Calibri,sans-serif;font-size:11pt">(I rather like strict const but it is apparently a deliberate non goal of the code base so I'd say it's probably idiomatic to just revert)<br>
</div></div><hr><span style="font-family:Tahoma,sans-serif;font-size:10pt;font-weight:bold">From: </span><span style="font-family:Tahoma,sans-serif;font-size:10pt">Chandler Carruth</span><br><span style="font-family:Tahoma,sans-serif;font-size:10pt;font-weight:bold">Sent: </span><span style="font-family:Tahoma,sans-serif;font-size:10pt">12/21/2011 12:36 PM</span><br>
<span style="font-family:Tahoma,sans-serif;font-size:10pt;font-weight:bold">To: </span><span style="font-family:Tahoma,sans-serif;font-size:10pt">Jakub Staszak</span><br><span style="font-family:Tahoma,sans-serif;font-size:10pt;font-weight:bold">Cc: </span><span style="font-family:Tahoma,sans-serif;font-size:10pt"><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>; David Blaikie</span><br>
<span style="font-family:Tahoma,sans-serif;font-size:10pt;font-weight:bold">Subject: </span><span style="font-family:Tahoma,sans-serif;font-size:10pt">Re: [llvm-commits] [llvm] r147090 - /llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp</span><br>
<br></body></html><div class="gmail_quote">On Wed, Dec 21, 2011 at 2:34 PM, Jakub Staszak <span dir="ltr"><<a href="mailto:kubastaszak@gmail.com">kubastaszak@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word">I have to agree with operator[] vs lookup(). We don't get any benefit here, so there is no point to make code less readable. In this case I'd have to revert the whole patch, because most of these operator[] won't work because of its lack of constantness.</div>
</blockquote><div><br></div><div>For reference, I'm fine with that. I don't think constantness is helping this code.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div><br></div><div>- Kuba</div><div><div class="h5"><div><br><div><div>On Dec 21, 2011, at 11:11 PM, Chandler Carruth wrote:</div><br><blockquote type="cite"><div class="gmail_quote">On Wed, Dec 21, 2011 at 2:00 PM, Jakub Staszak <span dir="ltr"><<a href="mailto:kubastaszak@gmail.com" target="_blank">kubastaszak@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
operator[] inserts an object for key if it doesn't exist.<br>
<br>
// X is empty<br>
if (X[0] == a) { }<br>
if (X[1] == b) { }<br>
if (X[2] == c) { }<br>
// X.size() = 3<br>
<br>
This is the way that std::map works. I believe we want to be quite compatible here.</blockquote><div><br></div><div>Certainly, but is it actually cheaper if you know ahead of time that no such insertion will occur? I would expect the lookup path to a hot path through the [] operators.</div>
<div><br></div><div>To be honest, I'm beginning to question the entire patch. No insertion ever happens with these [] uses, and they seem more readable to me than lookup... I don't feel strongly about the lookup, but I do feel strongly about the const thing. I'd really rather you strip back out all the const here.</div>
</div>
</blockquote></div><br></div></div></div></div></blockquote></div><br>