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

[Teresa Johnson via llvm-commits]

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.

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/8d2fa7a0/attachment.html>