[PATCH] D35189: Add GUID-ValueID map and original name to ThinLTO summary.

[Teresa Johnson via llvm-commits]

On Mon, Jul 10, 2017 at 9:16 AM, Mehdi AMINI via Phabricator <
reviews at reviews.llvm.org> wrote:

> mehdi_amini added a comment.
>
>
>
>
>
> ================
> Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:3437
> +  // For local linkage, we also emit the original name separately
> +  // immediately after the record.
> +  auto MaybeEmitOriginalName = [&](const GlobalValue &GV) {
> ----------------
> Please add here *why* this is done, other the comment is fairly not
> helpful: it just tells me wha the function is doing.
> I think the reason is "This is needed because promoting internal symbol to
> global requires generating a new GUID from the original name".
> 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.
>

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.

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.
>

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).


>
> https://reviews.llvm.org/D35189
>
>
>
>


-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170710/82feabaf/attachment.html>