<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">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-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="gmail-">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-width:1px;border-left-style:solid;border-left-color: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-"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color: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><div>What kind? I thought we had a mechanism for this with some metadata recording the PGO guid or something like that.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color: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::<wbr>getGlobalIdentifier).</div></div></div></div></blockquote><div><br></div><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><br></div><div>-- </div><div>Mehdi</div><div><br></div><div><br></div><div><br></div></div></div></div>