<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 10, 2017 at 9:16 AM, Mehdi AMINI via Phabricator <span dir="ltr"><<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mehdi_amini added a comment.<br>
<br>
<br>
<br>
<br>
<br>
================<br>
Comment at: lib/Bitcode/Writer/<wbr>BitcodeWriter.cpp:3437<br>
<span class="">+  // For local linkage, we also emit the original name separately<br>
+  // immediately after the record.<br>
</span>+  auto MaybeEmitOriginalName = [&](const GlobalValue &GV) {<br>
----------------<br>
Please add here *why* this is done, other the comment is fairly not helpful: it just tells me wha the function is doing.<br>
I think the reason is "This is needed because promoting internal symbol to global requires generating a new GUID from the original name".<br>
That said, unless there is another reason I missed, we could also immediately instead store two GUID for internal symbols: the local and the global.<br></blockquote><div><br></div><div>I suggested Haojie use the same mechanism we use in the combined index for storing the original GUID (which is why the FS_COMBINED_ORIGINAL_NAME was renamed/repurposed to FS_ORIGINAL_NAME in this patch). Otherwise we should rework the combined index for consistency, and I'm not sure that is worthwhile.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Also, alternatively and even more simpler I think, we could just always use the global GUID for internal symbols, whether they're promoted or not.<br></blockquote><div><br></div><div>This will cause issues with PGO indirect call promotion + ThinLTO. And would result in name clashes in the combined index (the internal symbols get a GUID computed from the hash of "modulename:valuename", see GlobalValue::getGlobalIdentifier).</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
<a href="https://reviews.llvm.org/D35189" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D35189</a><br>
<br>
<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> 408-460-2413</td></tr></tbody></table></span></div>
</div></div>