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

[Teresa Johnson via llvm-commits]

On Tue, Jul 11, 2017 at 9:08 AM, Mehdi AMINI <joker.eph at gmail.com> wrote:

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


Ack - yes. You are right. I was thinking about the thin link in isolation.
In that case, we need the IRSymtab and strtab otherwise. Haojie - let's
chat about it then as we'll need the info to compute the GUIDs in any case.

Teresa


> --
> 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 <(408)%20460-2413>
>>
>


-- 
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/2816635e/attachment.html>