Linker: Remove unnecessary call to copyMetadata in IRLinker::linkGlobalVariable.

[pcc]

Summary

This was causing us to create duplicate metadata on global variables.
Test case by Adrian Prantl.

Fixes PR31012.

[aprantl]

Thanks for the quick turnaround. One question inline, but otherwise this looks good.

llvm/lib/Linker/IRMover.cpp

953 ↗

(On Diff #77845)

So, is the metadata attached to Dst always identical to the one in Src, or could there be a situation where one is more complete / otherwise better than the other?

[pcc]

Actually I'm not sure that this is quite right yet. It isn't clear to me that we're doing the right thing in the case where we link a declaration followed by a definition. We at least need test cases covering that case.

[pcc]

Add tests cases for declaration/definition and weak/strong

[pcc]

One more test case

llvm/lib/Linker/IRMover.cpp

953 ↗

(On Diff #77845)

As far as I know for our current uses of metadata on globals we only need to keep the metadata for whichever definition "wins". At this point Dst will always be a fresh global variable with metadata cloned from Src's metadata, as far as I can tell from following the logic around IRLinker. The actual replacement of the definitions happens on line 943 after which we delete the old definition.

llvm/trunk/lib/Linker/IRMover.cpp

Expected<Constant *> IRLinker::linkGlobalValueProto(GlobalValue *SGV,
   }
 
   return C;
 }
 
 /// Update the initializers in the Dest module now that all globals that may be
 /// referenced are in Dest.
 void IRLinker::linkGlobalVariable(GlobalVariable &Dst, GlobalVariable &Src) {
-  Dst.copyMetadata(&Src, 0);
-
   // Figure out what the initializer looks like in the dest module.
   Mapper.scheduleMapGlobalInitializer(Dst, *Src.getInitializer());
 }
 
 /// Copy the source function over into the dest function and fix up references
 /// to values. At this point we know that Dest is an external function, and
 /// that Src is not.
 Error IRLinker::linkFunctionBody(Function &Dst, Function &Src) {

llvm/trunk/test/Linker/Inputs/metadata-attach.ll

+@g1 = external global i32, !attach !0
+
+@g2 = global i32 1, !attach !0
+
+@g3 = global i32 2, !attach !0
+
+declare !attach !0 void @f1()
+
+define void @f2() !attach !0 {
+  call void @f1()
+  store i32 0, i32* @g1
+  ret void
+}
+
+define void @f3() !attach !0 {
+  ret void
+}
+
+!0 = !{i32 1}

llvm/trunk/test/Linker/debug-info-global-var.ll

+; RUN: llvm-link -S %s %S/debug-info-version-a.ll | FileCheck %s
+source_filename = "debug-info-global-var.c"
+target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-apple-macosx10.12.0"
+
+; CHECK: @g = global i32 0, align 4, !dbg ![[G:[0-9]+]]{{$}}
+@g = global i32 0, align 4, !dbg !0
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!6, !7, !8}
+!llvm.ident = !{!9}
+
+; CHECK: ![[G]] = distinct !DIGlobalVariable(name: "g"
+!0 = distinct !DIGlobalVariable(name: "g", scope: !1, file: !2, line: 1, type: !5, isLocal: false, isDefinition: true)
+!1 = distinct !DICompileUnit(language: DW_LANG_C99, file: !2, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !3, globals: !4)
+!2 = !DIFile(filename: "debug-info-global-var.c", directory: "/")
+!3 = !{}
+!4 = !{!0}
+!5 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!6 = !{i32 2, !"Dwarf Version", i32 4}
+!7 = !{i32 2, !"Debug Info Version", i32 3}
+!8 = !{i32 1, !"PIC Level", i32 2}
+!9 = !{!"clang version 4.0.0 (trunk 286129) (llvm/trunk 286128)"}

llvm/trunk/test/Linker/metadata-attach.ll

 ; RUN: llvm-link %s -S -o - | FileCheck %s
+; RUN: llvm-link %s %S/Inputs/metadata-attach.ll -S -o - | FileCheck --check-prefix=CHECK-LINKED1 %s
+; RUN: llvm-link %S/Inputs/metadata-attach.ll %s -S -o - | FileCheck --check-prefix=CHECK-LINKED2 %s
 
-; CHECK: @g1 = global i32 0, !attach !0
+; CHECK: @g1 = global i32 0, !attach !0{{$}}
+; CHECK-LINKED1: @g1 = global i32 0, !attach !0{{$}}
 @g1 = global i32 0, !attach !0
 
-; CHECK: @g2 = external global i32, !attach !0
+; CHECK: @g3 = weak global i32 1, !attach !0{{$}}
+; CHECK: @g2 = external global i32, !attach !0{{$}}
+; CHECK-LINKED1: @g2 = global i32 1, !attach !1{{$}}
 @g2 = external global i32, !attach !0
 
-; CHECK: define void @f1() !attach !0
+; CHECK-LINKED1: @g3 = global i32 2, !attach !1{{$}}
+@g3 = weak global i32 1, !attach !0
+
+; CHECK-LINKED2: @g2 = global i32 1, !attach !0{{$}}
+; CHECK-LINKED2: @g3 = global i32 2, !attach !0{{$}}
+; CHECK-LINKED2: @g1 = global i32 0, !attach !1{{$}}
+
+; CHECK: define void @f1() !attach !0 {
+; CHECK-LINKED1: define void @f1() !attach !0 {
 define void @f1() !attach !0 {
   call void @f2()
   store i32 0, i32* @g2
   ret void
 }
 
 ; CHECK: declare !attach !0 void @f2()
+; CHECK-LINKED1: define void @f2() !attach !1 {
 declare !attach !0 void @f2()
 
-!0 = !{}
+; CHECK: define weak void @f3() !attach !0 {
+; CHECK-LINKED1: define void @f3() !attach !1 {
+define weak void @f3() !attach !0 {
+  ret void
+}
+
+; CHECK-LINKED2: define void @f2() !attach !0 {
+; CHECK-LINKED2: define void @f3() !attach !0 {
+; CHECK-LINKED2: define void @f1() !attach !1 {
+
+; CHECK-LINKED1: !0 = !{i32 0}
+; CHECK-LINKED1: !1 = !{i32 1}
+
+; CHECK-LINKED2: !0 = !{i32 1}
+; CHECK-LINKED2: !1 = !{i32 0}
+
+!0 = !{i32 0}