<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 10, 2017 at 11:04 AM, Mehdi AMINI <span dir="ltr"><<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="gmail-">2017-07-10 9:58 GMT-07:00 Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="gmail-m_7290611505653752587gmail-">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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">mehdi_amini added a comment.<br>
<br>
<br>
<br>
<br>
<br>
================<br>
Comment at: lib/Bitcode/Writer/BitcodeWrit<wbr>er.cpp:3437<br>
<span>+  // 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></span><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><span class="gmail-m_7290611505653752587gmail-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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></span><div>This will cause issues with PGO indirect call promotion + ThinLTO. </div></div></div></div></blockquote><div><br></div></span><div>What kind? I thought we had a mechanism for this with some metadata recording the PGO guid or something like that.</div></div></div></div></blockquote><div><br></div><div>The indirect call metadata records the GUID of the callee. We need to match that GUID to one in the index so we can import the right function. The PGO GUID uses the same naming scheme of "modulename:valuename".</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-"><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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::getGlobalIdentifi<wbr>er).</div></div></div></div></blockquote><div><br></div></span><div>This isn't clear to me: I was suggesting using *only* the GUID that can't clash (i.e. the one computed from "modulename:valuename").</div></div></div></div></blockquote><div><br></div><div>Then the GUID will not match the PGO data for global indirect callees.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-HOEnZb"><font color="#888888"><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div><br></div><div><br></div></font></span></div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="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:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"> 408-460-2413</td></tr></tbody></table></span></div>
</div></div>