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

[Teresa Johnson via llvm-commits]

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.

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 |
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181115/d73f31c7/attachment.html>