[llvm] r251965 - Revert "Move metadata linking after lazy global materialization/linking."

Thu Nov 5 17:17:59 PST 2015

On Thu, Nov 5, 2015 at 5:09 PM, Duncan P. N. Exon Smith
<dexonsmith at apple.com> wrote:
>
>> On 2015-Nov-03, at 11:36, Teresa Johnson via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>>
>> Author: tejohnson
>> Date: Tue Nov  3 13:36:04 2015
>> New Revision: 251965
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=251965&view=rev
>> Log:
>> Revert "Move metadata linking after lazy global materialization/linking."
>>
>> This reverts commit r251926. I believe this is causing an LTO
>> bootstrapping bot failure
>> (http://lab.llvm.org:8080/green/job/llvm-stage2-cmake-RgLTO_build/3669/).
>>
>> Haven't been able to repro it yet, but after looking at the metadata I
>> am pretty sure I know what is going on.
>
> Were you able to track this down?  (Maybe I missed it, but I didn't see
> the re-commit...)

Thanks for asking. Yes, in fact I just put an update on D14387 that
describes what I ran into.

Interestingly, I can't reproduce this LTO bootstrapping failure easily
with gold as it is resolving the symbols differently. But I was able
to provoke it by temporarily hacking gold-plugin to do something
slightly different. It relates to references from metadata to
otherwise unreferenced and therefore not yet materialized global
values. With this patch since global value body linking is already
complete it just links in a decl. The error message in this case was
for an alias that was in this situation (only referenced via metadata)
- it is illegal for an alias to be a decl. I can fix that the same way
I handle it for ThinLTO by turning it into a non-alias decl. Then I
hit an issue with decls in comdats, also illegal. Granted this would
not occur with ld64 since MachO doesn't have comdats. But it seems
like something that should be handled, and was part of the motivation
for D14387. See the comments I just added there on this issue an
possible alternatives. Any suggestions if we don't want to do D14387
are appreciated. Thanks!

>
>> Removed:
>>    llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll
>>    llvm/trunk/test/Linker/only-needed-named-metadata.ll
>> Modified:
>>    llvm/trunk/lib/Linker/LinkModules.cpp
>>
>> Modified: llvm/trunk/lib/Linker/LinkModules.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/LinkModules.cpp?rev=251965&r1=251964&r2=251965&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Linker/LinkModules.cpp (original)
>> +++ llvm/trunk/lib/Linker/LinkModules.cpp Tue Nov  3 13:36:04 2015
>> @@ -1940,6 +1940,15 @@ bool ModuleLinker::run() {
>>     linkGlobalValueBody(Src);
>>   }
>>
>> +  // Remap all of the named MDNodes in Src into the DstM module. We do this
>> +  // after linking GlobalValues so that MDNodes that reference GlobalValues
>> +  // are properly remapped.
>> +  linkNamedMDNodes();
>> +
>> +  // Merge the module flags into the DstM module.
>> +  if (linkModuleFlagsMetadata())
>> +    return true;
>> +
>>   // Update the initializers in the DstM module now that all globals that may
>>   // be referenced are in DstM.
>>   for (GlobalVariable &Src : SrcM->globals()) {
>> @@ -1966,15 +1975,6 @@ bool ModuleLinker::run() {
>>       return true;
>>   }
>>
>> -  // Remap all of the named MDNodes in Src into the DstM module. We do this
>> -  // after linking GlobalValues so that MDNodes that reference GlobalValues
>> -  // are properly remapped.
>> -  linkNamedMDNodes();
>> -
>> -  // Merge the module flags into the DstM module.
>> -  if (linkModuleFlagsMetadata())
>> -    return true;
>> -
>>   return false;
>> }
>>
>>
>> Removed: llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll?rev=251964&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll (original)
>> +++ llvm/trunk/test/Linker/Inputs/only-needed-named-metadata.ll (removed)
>> @@ -1,9 +0,0 @@
>> - at X = external global i32
>> -
>> -declare i32 @foo()
>> -
>> -define void @bar() {
>> -     load i32, i32* @X
>> -     call i32 @foo()
>> -     ret void
>> -}
>>
>> Removed: llvm/trunk/test/Linker/only-needed-named-metadata.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/only-needed-named-metadata.ll?rev=251964&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Linker/only-needed-named-metadata.ll (original)
>> +++ llvm/trunk/test/Linker/only-needed-named-metadata.ll (removed)
>> @@ -1,16 +0,0 @@
>> -; RUN: llvm-as %S/only-needed-named-metadata.ll -o %t.bc
>> -; RUN: llvm-as %S/Inputs/only-needed-named-metadata.ll -o %t2.bc
>> -; RUN: llvm-link -S -only-needed %t2.bc %t.bc | FileCheck %s
>> -; RUN: llvm-link -S -internalize -only-needed %t2.bc %t.bc | FileCheck %s
>> -
>> -; CHECK: @U = external global i32
>> -; CHECK: declare i32 @unused()
>> -
>> - at X = global i32 5
>> - at U = global i32 6
>> -define i32 @foo() { ret i32 7 }
>> -define i32 @unused() { ret i32 8 }
>> -
>> -!llvm.named = !{!0, !1}
>> -!0 = !{i32 ()* @unused}
>> -!1 = !{i32* @U}
>>
>>
>> _______________________________________________
>> 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