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

[Mehdi AMINI via llvm-commits]

On Tue, Jul 11, 2017 at 8:03 AM Teresa Johnson <tejohnson at google.com> wrote:

> On Mon, Jul 10, 2017 at 12:51 PM, Peter Collingbourne <peter at pcc.me.uk>
> wrote:
>
>> On Mon, Jul 10, 2017 at 12:37 PM, Teresa Johnson <tejohnson at google.com>
>> wrote:
>>
>>>
>>>
>>> On Mon, Jul 10, 2017 at 12:30 PM, Peter Collingbourne via Phabricator <
>>> reviews at reviews.llvm.org> wrote:
>>>
>>>> pcc added a comment.
>>>>
>>>> > the global VST (for the global values we will have in the index) does
>>>> not.
>>>>
>>>> Not quite, global VSTs include a reference to the symbol name in the
>>>> strtab.
>>>>
>>>
>>> That's what I was thinking when we chatted about this with Haojie, but
>>> it turns out I was wrong. The global VST includes the value id and the
>>> bitcode offset of the function definition in the IR:
>>>  http://llvm-cs.pcc.me.uk/lib/Bitcode/Writer/BitcodeWriter.cpp#2763
>>>
>>> The offset to the symbol name is included in the MODULE_CODE_FUNCTION
>>> entry:
>>> http://llvm-cs.pcc.me.uk/lib/Bitcode/Writer/BitcodeWriter.cpp#1170
>>> which is not really in the global VST.
>>>
>>
>> Of course, I totally forgot about that.
>>
>> I wonder whether it would be better to emit MODULE_CODE_FUNCTION (etc.)
>> records into the minimised bitcode file, but of length 5 so that they
>> include only the name and the linkage. Then there will be no special
>> handling required in the index bitcode reader.
>>
>
> That is possible too - we could emit the IRSymtab + strtab + MODULE_CODE_*
> records. But note that as soon as we get into the thin link, we just use
> these to compute the GUIDs and don't need the names at all. So it seems
> more straightforward to simply emit the ValueId->GUID mapping directly.
>
> There are a couple of tradeoffs here of course. When we are performing
> in-process ThinLTO backends, we only have the single original bitcode with
> IR + summary, and emitting this mapping adds some redundant info that could
> be computed from the names in the bitcode file. However, it should be a bit
> faster to simply get the mappings directly from the records being added
> here.
>
> In the distributed case, adding these records means the minimized bitcode
> file only needs the summary entries (+ these new GUID mapping records), and
> not the IRsymtab+strtab+ MODULE_CODE_* records. Also - since the summary in
> the original (IR) bitcode file (only fed to the ThinLTO backend processes)
> is never needed, when emitting a minimized bitcode file for the thin link
> we can simply skip emission of the summaries (+ these new records) at all
> in the IR bitcode file.
>

In the distributed case, isn't the minimum bitcode file the one you send to
the linker? While the thinlink does not need names, I thought that the
linker will use the symtab to perform its resolution. Am I missing
something?

-- 
Mehdi



> Teresa
>
>
>> Peter
>>
>>
>>>
>>>> > But I think infer it from the code in my previous inline comment, it
>>>> is "we need the name to recompute a GUID for internal symbol promoted to
>>>> global", correct?
>>>>
>>>> Names of globals are also necessary for symbol resolution.
>>>>
>>>>
>>>> https://reviews.llvm.org/D35189
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>>> 408-460-2413 <(408)%20460-2413>
>>>
>>
>>
>>
>> --
>> --
>> Peter
>>
>
>
>
> --
> 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/20170711/ac621634/attachment.html>