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

Teresa Johnson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 10 10:53:08 PDT 2017


tejohnson added a comment.

In https://reviews.llvm.org/D35189#803919, @haojiewang wrote:

> In https://reviews.llvm.org/D35189#803619, @tejohnson wrote:
>
> > In https://reviews.llvm.org/D35189#803332, @mehdi_amini wrote:
> >
> > > The context and the motivation is *still* not clear to me. To begin why do we need the *names* when doing the thin link?
> >
> >
> > Right - I think we don't need the IRSymtab or strtab with this approach since the valueID->GUID correspondence is being recorded directly. We had talked about emitting the value IDs via the VST and the names via the symtab/strtab, but that doesn't work well (it's not that the VST has redundant info, as Haojie indicated in his comment, but rather that it doesn't have the right info - it no longer maps the value id to the name or even directly to a symtab entry). This issue just affects the summary description of this patch since the actual minimization is being left for a follow-on - Haojie, can you update the summary description of the patch?
>
>
> I think the VST is still holding the right info now. If I understand the code correctly, VST will record the mapping between ValueId and ValueName. Then in thin link phase, GUID and OriginalNameGUID will be computed by ValueName along with linkage and module path, so we can get the ValueID to GUID map, and also the value of OriginalGUID. But the ValueName may be too large(since it is a string), so we can replace it with GUID and OriginalNameGUID(if it is needed).


Just talked to Haojie offline, but for everyone else following along - the function level VST (for instructions, bbs, etc) still includes the name, but the global VST (for the global values we will have in the index) does not.


https://reviews.llvm.org/D35189





More information about the llvm-commits mailing list