On Sat, Mar 21, 2009 at 4:51 PM, Chris Lattner <span dir="ltr"><<a href="mailto:sabre@nondot.org">sabre@nondot.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im"><br>
On Mar 21, 2009, at 3:35 PM, Daniel Dunbar wrote:<br>
<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
On 3/21/09, Chris Lattner <<a href="mailto:sabre@nondot.org" target="_blank">sabre@nondot.org</a>> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
Author: lattner<br>
Date: Sat Mar 21 02:48:31 2009<br>
New Revision: 67438<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=67438&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=67438&view=rev</a><br>
Log:<br>
simplify and comment some code better.  Make BindRuntimeGlobals<br>
more optimistic that it will work (optimizing for the common case).<br>
</blockquote>
<br>
I may be missing something, but wouldn't this be cleaner if we just<br>
provided a Module method to return a reference to the entry for a<br>
name? This would avoiding the shuffling with names and only do one<br>
lookup.<br>
</blockquote>
<br></div>
We already have this through ValueSymbolTable.  I added this comment, though I don't really know if it is true:</blockquote><div class="im"><br>I guess I wasn't clear... I was referring to this code:<br>--<br>    // See if there is a conflict against a function by setting the name and<br>
    // seeing if we got the desired name.<br>    GV->setName(Name);<br>    if (GV->isName(Name.c_str()))<br>      continue;  // Yep, it worked!<br>    <br>    GV->setName(""); // Zap the bogus name until we work out the conflict.<br>
    llvm::GlobalValue *Conflict = TheModule.getNamedValue(Name);<br>    assert(Conflict && "Must have conflicted!");<br>--,<br>and meant, why not use the common trick of only looking up the cell entry once. Of course this is complicated because the symbol table and the names are tied together. Something along the lines of:<br>
--<br>  iterator Entry = TheModule.getValueSymbolTable().lookup(Name);<br>  if (Entry == TheModule.getValueSymbolTable().end()) {<br>    // There is no conflict.<br>    GV->setName(Name, Entry);<br>    continue;<br>  } <br>
<br>  // There is a conflict with *Entry.<br>--<br>Of course, now that I've typed it out I realize the API overhead to allow this probably isn't worth it.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">

I would also like to eliminate RuntimeGlobals at some point.</blockquote><div><br>Ok! Although I'm curious how...<br><br> - Daniel<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<font color="#888888"><br>
-Chris<br>
</font></blockquote></div><br>