[llvm] r266856 - ThinLTO: never promote as external weak

[Teresa Johnson via llvm-commits]

On Tue, Apr 19, 2016 at 9:39 PM, Mehdi Amini <mehdi.amini at apple.com> wrote:

>
> On Apr 19, 2016, at 9:36 PM, Teresa Johnson <tejohnson at google.com> wrote:
>
>
>
> On Tue, Apr 19, 2016 at 9:18 PM, Mehdi Amini via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: mehdi_amini
>> Date: Tue Apr 19 23:18:11 2016
>> New Revision: 266856
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=266856&view=rev
>> Log:
>> ThinLTO: never promote as external weak
>>
>> This linkage is *not* intended to express that a declaration refers
>> to a weak symbol, but that the symbol might not be present at link
>> time. I don't believe it was the intent.
>>
>
> In fact, this was the intent. WeakAny linkage is used for defined symbols
> with __attribute__((weak)), which may not have a definition. The
> corresponding declaration should be ExternalWeak.
>
>
> It is not clear to me: how can you during the link be in a situation where
> you have a weak definition that will not be present?
> I don't see a case where we *introduce* an external_weak that is not
> present in the original program.
>

Yep, I just responded to my own email after rereading it. I think this is
ok. Since WeakAny must be a definition, there is no need to indicate that
there might not be a definition. At the time when I wrote up the linkage
changes this linkage change made sense to me, but I think you are right.

Teresa


>
> CC Rafael.
>
>
> --
> Mehdi
>
>
>
>
>> From: Mehdi Amini <mehdi.amini at apple.com>
>>
>> Modified:
>>     llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
>>     llvm/trunk/test/Linker/funcimport.ll
>>
>> Modified: llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp?rev=266856&r1=266855&r2=266856&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp (original)
>> +++ llvm/trunk/lib/Transforms/Utils/FunctionImportUtils.cpp Tue Apr 19
>> 23:18:11 2016
>> @@ -132,7 +132,7 @@ FunctionImportGlobalProcessing::getLinka
>>      // linker. The module linking caller needs to enforce this.
>>      assert(!doImportAsDefinition(SGV));
>>      // If imported as a declaration, it becomes external_weak.
>> -    return GlobalValue::ExternalWeakLinkage;
>> +    return SGV->getLinkage();
>>
>>    case GlobalValue::WeakODRLinkage:
>>      // For weak_odr linkage, there is a guarantee that all copies will be
>>
>> Modified: llvm/trunk/test/Linker/funcimport.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/funcimport.ll?rev=266856&r1=266855&r2=266856&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/Linker/funcimport.ll (original)
>> +++ llvm/trunk/test/Linker/funcimport.ll Tue Apr 19 23:18:11 2016
>> @@ -98,7 +98,7 @@
>>  ; reference should turned into an external_weak declaration.
>>  ; RUN: llvm-link %t2.bc -summary-index=%t3.thinlto.bc
>> -import=callweakfunc:%t.bc -import=weakfunc:%t.bc -S 2>&1 | FileCheck %s
>> --check-prefix=IMPORTWEAKFUNC
>>  ; IMPORTWEAKFUNC-DAG: Ignoring import request for weak-any function
>> weakfunc
>> -; IMPORTWEAKFUNC-DAG: declare extern_weak void @weakfunc
>> +; IMPORTWEAKFUNC-DAG: declare void @weakfunc
>>  ; IMPORTWEAKFUNC-DAG: define available_externally void @callweakfunc
>>  ; IMPORTWEAKFUNC-NOT: @weakvar = extern_weak global i32, align 4
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-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/20160419/a520bc8c/attachment.html>