[PATCH] D49362: [ThinLTO] Internalize read only globals

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 15 06:44:00 PST 2018


On Thu, Nov 15, 2018 at 6:35 AM Teresa Johnson <tejohnson at google.com> wrote:

>
>
> On Thu, Nov 15, 2018 at 6:22 AM Evgeny Leviant <eleviant at accesssoftek.com>
> wrote:
>
>> Thanks for making reproducer, Steven!
>>
>>
>> It turned out cache key is computed differently by newer API (LTO.cpp)
>> and legacy API (ThinLTOCodeGenerator).
>>
> Arg - such a simple reason! I didn't remember that they were using
> separate code. Looking at what appears to be the cache key computation for
> the old API (ModuleCacheEntry constructor), I don't see it hashing other
> things we compute during the thin link beyond import/exports/resolved
> linkage types. I.e. I don't see it hashing the isLive bit. So I'm surprised
> we haven't hit an issue here before. Wonder if this code and
> computeCacheKey can be refactored to do the same computation for most of
> the info?? Maybe as a follow on patch. But I'm really curious how things
> are working given e.g. the missing hash of the live bit etc.
>

Answering my own question: The isLive bit is only used downstream of the
thin link in the CFI/WPD case, which aren't supported via the old LTO API.
Ditto for the DSOLocal bit, which is only set in the new LTO API.

Anyway, I think it would be good to refactor and call common code for the
cache key computation, but this can be done as a follow on. Steven - is
this something you would have time to do? I can do it, but can't test with
ld64. If you want I can create a patch and you can test it there.

Teresa


>
> I was always wondering what piece of software uses legacy API and finally
>> found this out - it is Mac OS X linker ld64.
>>
>
> Yep, along with at least one proprietary linker that I am aware of.
>
> Teresa
>
>>
>> On Linux lld and gold plugin use newer API, so the bug can't be
>> reproduced.
>> ------------------------------
>> *От:* stevenwu at apple.com <stevenwu at apple.com> от имени Steven Wu <
>> stevenwu at apple.com>
>> *Отправлено:* 15 ноября 2018 г. 3:34
>> *Кому:* Teresa Johnson
>> *Копия:* Evgeny Leviant; Mehdi AMINI;
>> reviews+D49362+public+6f0316d7b1456727 at reviews.llvm.org; Xin Tong; Alex
>> L; Daniel Grumberg; JF Bastien; Anton Korobeynikov; Bob Haarman; Easwaran
>> Raman; Duncan Exon Smith; llvm-commits; George Rimar; Igor Kudrin;
>> jun.l at samsung.com
>> *Тема:* Re: [PATCH] D49362: [ThinLTO] Internalize read only globals
>>
>> CAUTION: This email originated from outside of the organization. Do not
>> click links or open attachments unless you recognize the sender and know
>> the content is safe.
>>
>>
>> On Nov 14, 2018, at 4:28 PM, Teresa Johnson <tejohnson at google.com> wrote:
>>
>>
>>
>> On Wed, Nov 14, 2018 at 1:47 PM Steven Wu <stevenwu at apple.com> wrote:
>>
>>> Here is a small reduced test case using the same main.c and foo.c from
>>> the review:
>>> cat main.c
>>>
>>> int foo();
>>> int main() {
>>>   return foo();
>>> }
>>>
>>>
>>> cat foo.c
>>>
>>> #include <stdlib.h>
>>> static int gFoo = 1;
>>> int foo() {
>>>   return gFoo;
>>> }
>>> void bar() {
>>>   gFoo = rand();
>>> }
>>>
>>>
>>> cat test1.c
>>>
>>> int foo();
>>> int test() {
>>>   return foo();
>>> }
>>>
>>>
>>> cat test2.c
>>>
>>> int foo();
>>> int bar();
>>> int test() {
>>>   return foo() + bar();
>>> }
>>>
>>>
>>> To reproduce:
>>> $ clang -O3 -flto=thin -c main.c test1.c test2.c foo.c
>>> $ clang -O3 -flto=thin -Wl,-cache_path_lto,lto.cache main.o test2.o foo.o
>>> $ clang -O3 -flto=thin -Wl,-cache_path_lto,lto.cache main.o test1.o foo.o
>>> Undefined symbols for architecture x86_64:
>>>   "_gFoo.llvm.17695746433417383459", referenced from:
>>>       _main in lto.o
>>>
>>
>> I can't reproduce this, although I am using gold
>> (with  -Wl,-plugin-opt,cache-dir=lto.cache) since I don't have ld64. I
>> notice that test() is dead, so with gold at least it is removed completely.
>>
>> Do you have any temp files you can share (i.e. IR before and after
>> importing)?
>>
>>
>> Attach all the intermediate file. I haven't had time to look in detail
>> yet.
>>
>>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>


-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181115/0ef8a83a/attachment.html>


More information about the llvm-commits mailing list