Reapply "Linker: Drop superseded subprograms"
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 18 Dec 2014 01:05:33 +0000 (01:05 +0000)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 18 Dec 2014 01:05:33 +0000 (01:05 +0000)
This reverts commit r224416, reapplying r224389.  The buildbots hadn't
recovered after my revert, waiting until David reverted a couple of his
commits.  It looks like it was just bad timing (where we were both
modifying code related to the same assertion).  Trying again...

Here's the original text:

    When a function gets replaced by `ModuleLinker`, drop superseded
    subprograms.  This ensures that the "first" subprogram pointing at a
    function is the same one that `!dbg` references point at.

    This is a stop-gap fix for PR21910.  Notably, this fixes Release+Asserts
    bootstraps that are currently asserting out in
    `LexicalScopes::initialize()` due to the explicit instantiations in
    `lib/IR/Dominators.cpp` eventually getting replaced by -argpromotion.

llvm-svn: 224487

llvm/lib/Linker/LinkModules.cpp
llvm/test/Linker/Inputs/replaced-function-matches-first-subprogram.ll [new file with mode: 0644]
llvm/test/Linker/replaced-function-matches-first-subprogram.ll [new file with mode: 0644]

index 0e14415..7f3e5b3 100644 (file)
@@ -19,6 +19,7 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 #include "llvm/IR/DiagnosticPrinter.h"
 #include "llvm/IR/LLVMContext.h"
@@ -416,6 +417,9 @@ class ModuleLinker {
   // Vector of GlobalValues to lazily link in.
   std::vector<GlobalValue *> LazilyLinkGlobalValues;
 
+  /// Functions that have replaced other functions.
+  SmallPtrSet<const Function *, 16> OverridingFunctions;
+
   Linker::DiagnosticHandlerFunction DiagnosticHandler;
 
 public:
@@ -494,6 +498,7 @@ private:
   bool linkGlobalValueBody(GlobalValue &Src);
 
   void linkNamedMDNodes();
+  void stripReplacedSubprograms();
 };
 }
 
@@ -1078,6 +1083,10 @@ bool ModuleLinker::linkGlobalValueProto(GlobalValue *SGV) {
     }
 
     NewGV = copyGlobalValueProto(TypeMap, *DstM, SGV);
+
+    if (DGV && isa<Function>(DGV))
+      if (auto *NewF = dyn_cast<Function>(NewGV))
+        OverridingFunctions.insert(NewF);
   }
 
   NewGV->setUnnamedAddr(HasUnnamedAddr);
@@ -1244,6 +1253,48 @@ void ModuleLinker::linkNamedMDNodes() {
   }
 }
 
+/// Drop DISubprograms that have been superseded.
+///
+/// FIXME: this creates an asymmetric result: we strip losing subprograms from
+/// DstM, but leave losing subprograms in SrcM.  Instead we should also strip
+/// losers from SrcM, but this requires extra plumbing in MapValue.
+void ModuleLinker::stripReplacedSubprograms() {
+  // Avoid quadratic runtime by returning early when there's nothing to do.
+  if (OverridingFunctions.empty())
+    return;
+
+  // Move the functions now, so the set gets cleared even on early returns.
+  auto Functions = std::move(OverridingFunctions);
+  OverridingFunctions.clear();
+
+  // Drop subprograms whose functions have been overridden by the new compile
+  // unit.
+  NamedMDNode *CompileUnits = DstM->getNamedMetadata("llvm.dbg.cu");
+  if (!CompileUnits)
+    return;
+  for (unsigned I = 0, E = CompileUnits->getNumOperands(); I != E; ++I) {
+    DICompileUnit CU(CompileUnits->getOperand(I));
+    assert(CU && "Expected valid compile unit");
+
+    DITypedArray<DISubprogram> SPs(CU.getSubprograms());
+    assert(SPs && "Expected valid subprogram array");
+
+    SmallVector<Metadata *, 16> NewSPs;
+    NewSPs.reserve(SPs.getNumElements());
+    for (unsigned S = 0, SE = SPs.getNumElements(); S != SE; ++S) {
+      DISubprogram SP = SPs.getElement(S);
+      if (SP && SP.getFunction() && Functions.count(SP.getFunction()))
+        continue;
+
+      NewSPs.push_back(SP);
+    }
+
+    // Redirect operand to the overriding subprogram.
+    if (NewSPs.size() != SPs.getNumElements())
+      CU.replaceSubprograms(DIArray(MDNode::get(DstM->getContext(), NewSPs)));
+  }
+}
+
 /// Merge the linker flags in Src into the Dest module.
 bool ModuleLinker::linkModuleFlagsMetadata() {
   // If the source module has no module flags, we are done.
@@ -1509,6 +1560,9 @@ bool ModuleLinker::run() {
     linkGlobalValueBody(Src);
   }
 
+  // Strip replaced subprograms before linking together compile units.
+  stripReplacedSubprograms();
+
   // 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.
diff --git a/llvm/test/Linker/Inputs/replaced-function-matches-first-subprogram.ll b/llvm/test/Linker/Inputs/replaced-function-matches-first-subprogram.ll
new file mode 100644 (file)
index 0000000..fb7b634
--- /dev/null
@@ -0,0 +1,27 @@
+%struct.Class = type { i8 }
+
+define weak_odr i32 @_ZN5ClassIiE3fooEv(%struct.Class* %this) align 2 {
+entry:
+  %this.addr = alloca %struct.Class*, align 8
+  store %struct.Class* %this, %struct.Class** %this.addr, align 8
+  %this1 = load %struct.Class** %this.addr
+  ret i32 0, !dbg !12
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !{!"0x11\004\00clang version 3.6.0 (trunk 224193) (llvm/trunk 224197)\000\00\000\00\002", !1, !2, !2, !3, !2, !2} ; [ DW_TAG_compile_unit ] [/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d2/t2.cpp] [DW_LANG_C_plus_plus]
+!1 = !{!"t2.cpp", !"/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d2"}
+!2 = !{}
+!3 = !{!4}
+!4 = !{!"0x2e\00foo\00foo\00\002\000\001\000\000\00256\000\002", !5, !6, !7, null, i32 (%struct.Class*)* @_ZN5ClassIiE3fooEv, null, null, !2} ; [ DW_TAG_subprogram ] [line 2] [def] [foo]
+!5 = !{!"../t.h", !"/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d2"}
+!6 = !{!"0x29", !5}    ; [ DW_TAG_file_type ] [/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d2/../t.h]
+!7 = !{!"0x15\00\000\000\000\000\000\000", null, null, null, !2, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!8 = !{i32 2, !"Dwarf Version", i32 2}
+!9 = !{i32 2, !"Debug Info Version", i32 2}
+!10 = !{i32 1, !"PIC Level", i32 2}
+!11 = !{!"clang version 3.6.0 (trunk 224193) (llvm/trunk 224197)"}
+!12 = !{i32 2, i32 15, !4, null}
diff --git a/llvm/test/Linker/replaced-function-matches-first-subprogram.ll b/llvm/test/Linker/replaced-function-matches-first-subprogram.ll
new file mode 100644 (file)
index 0000000..6511384
--- /dev/null
@@ -0,0 +1,75 @@
+; RUN: llvm-link %s %S/Inputs/replaced-function-matches-first-subprogram.ll -S | FileCheck %s
+
+; Generated from C++ source:
+;
+; // repro/t.h
+; template <class T> struct Class {
+;   int foo() { return 0; }
+; };
+; // repro/d1/t1.cpp
+; #include "t.h"
+; int foo() { return Class<int>().foo(); }
+; // repro/d2/t2.cpp
+; #include "t.h"
+; template struct Class<int>;
+
+%struct.Class = type { i8 }
+
+define i32 @_Z3foov() {
+entry:
+  %tmp = alloca %struct.Class, align 1
+  %call = call i32 @_ZN5ClassIiE3fooEv(%struct.Class* %tmp), !dbg !14
+  ret i32 %call, !dbg !14
+}
+
+; CHECK: define weak_odr i32 @_ZN5ClassIiE3fooEv(%struct.Class* %this){{.*}}{
+; CHECK-NOT: }
+; CHECK: !dbg ![[LOC:[0-9]+]]
+define linkonce_odr i32 @_ZN5ClassIiE3fooEv(%struct.Class* %this) align 2 {
+entry:
+  %this.addr = alloca %struct.Class*, align 8
+  store %struct.Class* %this, %struct.Class** %this.addr, align 8
+  %this1 = load %struct.Class** %this.addr
+  ret i32 0, !dbg !15
+}
+
+; CHECK: !llvm.dbg.cu = !{![[CU1:[0-9]+]], ![[CU2:[0-9]+]]}
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!10, !11, !12}
+!llvm.ident = !{!13}
+
+; Extract out the list of subprograms from each compile unit.
+; CHECK-DAG: ![[CU1]] = !{!"0x11{{[^"]+}}", {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, ![[SPs1:[0-9]+]],
+; CHECK-DAG: ![[CU2]] = !{!"0x11{{[^"]+}}", {{[^,]+}}, {{[^,]+}}, {{[^,]+}}, ![[SPs2:[0-9]+]],
+!0 = !{!"0x11\004\00clang version 3.6.0 (trunk 224193) (llvm/trunk 224197)\000\00\000\00\002", !1, !2, !2, !3, !2, !2} ; [ DW_TAG_compile_unit ] [/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1/t1.cpp] [DW_LANG_C_plus_plus]
+!1 = !{!"t1.cpp", !"/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1"}
+!2 = !{}
+
+; Extract out each compile unit's single subprogram.  The replaced subprogram
+; should be dropped by the first compile unit.
+; CHECK-DAG: ![[SPs1]] = !{![[SP1:[0-9]+]]}
+; CHECK-DAG: ![[SPs2]] = !{![[SP2:[0-9]+]]}
+!3 = !{!4, !7}
+!4 = !{!"0x2e\00foo\00foo\00\002\000\001\000\000\00256\000\002", !1, !5, !6, null, i32 ()* @_Z3foov, null, null, !2} ; [ DW_TAG_subprogram ] [line 2] [def] [foo]
+!5 = !{!"0x29", !1}    ; [ DW_TAG_file_type ] [/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1/t1.cpp]
+!6 = !{!"0x15\00\000\000\000\000\000\000", null, null, null, !2, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+
+; Extract out the file from the replaced subprogram.  Confirm that each
+; subprogram is pointing at the correct function.
+; CHECK-DAG: ![[SP1]] = !{!"0x2e{{[^"]+}}", {{.*}}, i32 ()* @_Z3foov,
+; CHECK-DAG: ![[SP2]] = !{!"0x2e{{[^"]+}}", ![[FILE:[0-9]+]], {{.*}}, i32 (%struct.Class*)* @_ZN5ClassIiE3fooEv,
+!7 = !{!"0x2e\00foo\00foo\00\002\000\001\000\000\00256\000\002", !8, !9, !6, null, i32 (%struct.Class*)* @_ZN5ClassIiE3fooEv, null, null, !2} ; [ DW_TAG_subprogram ] [line 2] [def] [foo]
+
+; The new subprogram should be pointing at the new directory.
+; CHECK-DAG: ![[FILE]] = !{!"../t.h", !"/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d2"}
+!8 = !{!"../t.h", !"/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1"}
+!9 = !{!"0x29", !8}    ; [ DW_TAG_file_type ] [/Users/dexonsmith/data/llvm/staging/test/Linker/repro/d1/../t.h]
+!10 = !{i32 2, !"Dwarf Version", i32 2}
+!11 = !{i32 2, !"Debug Info Version", i32 2}
+!12 = !{i32 1, !"PIC Level", i32 2}
+!13 = !{!"clang version 3.6.0 (trunk 224193) (llvm/trunk 224197)"}
+!14 = !{i32 2, i32 20, !4, null}
+
+; The same subprogram should be pointed to by inside the !dbg reference.
+; CHECK: ![[LOC]] = !{i32 2, i32 15, ![[SP2]], null}
+!15 = !{i32 2, i32 15, !7, null}