Revert sharing subprograms across CUs

This patch is a revert of e08f205. In that patch, DW_TAG_subprograms
were permitted to be referenced across CU boundaries, to improve stack
trace construction using call site information. Unfortunately, as
documented in PR48790, the way that subprograms are "owned" by dwarf units
is sufficiently complicated that subprograms end up in unexpected units,
invalidating cross-unit references.

There's no obvious way to easily fix this, and several attempts have
failed. Revert this to ensure correct DWARF is always emitted.

Three tests change in addition to the reversion, but they're all very
light alterations.

Differential Revision: https://reviews.llvm.org/D107076

(cherry picked from commit d4ce9e4)
Signed-off-by: Jeremy Morse <[email protected]>

Conflicts:
	llvm/test/DebugInfo/X86/convert-loclist.ll

Author: jmorse

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

@@ -1162,7 +1162,7 @@ DwarfCompileUnit::getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const {
 }
 
 DIE &DwarfCompileUnit::constructCallSiteEntryDIE(DIE &ScopeDIE,
-                                                 DIE *CalleeDIE,
+                                                 const DISubprogram *CalleeSP,
                                                  bool IsTail,
                                                  const MCSymbol *PCAddr,
                                                  const MCSymbol *CallAddr,
@@ -1176,7 +1176,8 @@ DIE &DwarfCompileUnit::constructCallSiteEntryDIE(DIE &ScopeDIE,
     addAddress(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_target),
                MachineLocation(CallReg));
   } else {
-    assert(CalleeDIE && "No DIE for call site entry origin");
+    DIE *CalleeDIE = getOrCreateSubprogramDIE(CalleeSP);
+    assert(CalleeDIE && "Could not create DIE for call site entry origin");
     addDIEEntry(CallSiteDIE, getDwarf5OrGNUAttr(dwarf::DW_AT_call_origin),
                 *CalleeDIE);
   }

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h

@@ -249,16 +249,14 @@ class DwarfCompileUnit final : public DwarfUnit {
   dwarf::LocationAtom getDwarf5OrGNULocationAtom(dwarf::LocationAtom Loc) const;
 
   /// Construct a call site entry DIE describing a call within \p Scope to a
-  /// callee described by \p CalleeDIE.
-  /// \p CalleeDIE is a declaration or definition subprogram DIE for the callee.
-  /// For indirect calls \p CalleeDIE is set to nullptr.
+  /// callee described by \p CalleeSP.
   /// \p IsTail specifies whether the call is a tail call.
   /// \p PCAddr points to the PC value after the call instruction.
   /// \p CallAddr points to the PC value at the call instruction (or is null).
   /// \p CallReg is a register location for an indirect call. For direct calls
   /// the \p CallReg is set to 0.
-  DIE &constructCallSiteEntryDIE(DIE &ScopeDIE, DIE *CalleeDIE, bool IsTail,
-                                 const MCSymbol *PCAddr,
+  DIE &constructCallSiteEntryDIE(DIE &ScopeDIE, const DISubprogram *CalleeSP,
+                                 bool IsTail, const MCSymbol *PCAddr,
                                  const MCSymbol *CallAddr, unsigned CallReg);
   /// Construct call site parameter DIEs for the \p CallSiteDIE. The \p Params
   /// were collected by the \ref collectCallSiteParameters.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

@@ -587,14 +587,6 @@ void DwarfDebug::constructAbstractSubprogramScopeDIE(DwarfCompileUnit &SrcCU,
   }
 }
 
-DIE &DwarfDebug::constructSubprogramDefinitionDIE(const DISubprogram *SP) {
-  DICompileUnit *Unit = SP->getUnit();
-  assert(SP->isDefinition() && "Subprogram not a definition");
-  assert(Unit && "Subprogram definition without parent unit");
-  auto &CU = getOrCreateDwarfCompileUnit(Unit);
-  return *CU.getOrCreateSubprogramDIE(SP);
-}
-
 /// Represents a parameter whose call site value can be described by applying a
 /// debug expression to a register in the forwarded register worklist.
 struct FwdRegParamInfo {
@@ -945,7 +937,7 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
         continue;
 
       unsigned CallReg = 0;
-      DIE *CalleeDIE = nullptr;
+      const DISubprogram *CalleeSP = nullptr;
       const Function *CalleeDecl = nullptr;
       if (CalleeOp.isReg()) {
         CallReg = CalleeOp.getReg();
@@ -955,19 +947,7 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
         CalleeDecl = dyn_cast<Function>(CalleeOp.getGlobal());
         if (!CalleeDecl || !CalleeDecl->getSubprogram())
           continue;
-        const DISubprogram *CalleeSP = CalleeDecl->getSubprogram();
-
-        if (CalleeSP->isDefinition()) {
-          // Ensure that a subprogram DIE for the callee is available in the
-          // appropriate CU.
-          CalleeDIE = &constructSubprogramDefinitionDIE(CalleeSP);
-        } else {
-          // Create the declaration DIE if it is missing. This is required to
-          // support compilation of old bitcode with an incomplete list of
-          // retained metadata.
-          CalleeDIE = CU.getOrCreateSubprogramDIE(CalleeSP);
-        }
-        assert(CalleeDIE && "Must have a DIE for the callee");
+        CalleeSP = CalleeDecl->getSubprogram();
       }
 
       // TODO: Omit call site entries for runtime calls (objc_msgSend, etc).
@@ -1004,7 +984,7 @@ void DwarfDebug::constructCallSiteEntryDIEs(const DISubprogram &SP,
                         << (IsTail ? " [IsTail]" : "") << "\n");
 
       DIE &CallSiteDIE = CU.constructCallSiteEntryDIE(
-          ScopeDIE, CalleeDIE, IsTail, PCAddr, CallAddr, CallReg);
+          ScopeDIE, CalleeSP, IsTail, PCAddr, CallAddr, CallReg);
 
       // Optionally emit call-site-param debug info.
       if (emitDebugEntryValues()) {
@@ -1121,6 +1101,11 @@ DwarfDebug::getOrCreateDwarfCompileUnit(const DICompileUnit *DIUnit) {
     NewCU.setSection(Asm->getObjFileLowering().getDwarfInfoSection());
   }
 
+  // Create DIEs for function declarations used for call site debug info.
+  for (auto Scope : DIUnit->getRetainedTypes())
+    if (auto *SP = dyn_cast_or_null<DISubprogram>(Scope))
+      NewCU.getOrCreateSubprogramDIE(SP);
+
   CUMap.insert({DIUnit, &NewCU});
   CUDieMap.insert({&NewCU.getUnitDie(), &NewCU});
   return NewCU;

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.h

@@ -471,9 +471,6 @@ class DwarfDebug : public DebugHandlerBase {
   /// Construct a DIE for this abstract scope.
   void constructAbstractSubprogramScopeDIE(DwarfCompileUnit &SrcCU, LexicalScope *Scope);
 
-  /// Construct a DIE for the subprogram definition \p SP and return it.
-  DIE &constructSubprogramDefinitionDIE(const DISubprogram *SP);
-
   /// Construct DIEs for call site entries describing the calls in \p MF.
   void constructCallSiteEntryDIEs(const DISubprogram &SP, DwarfCompileUnit &CU,
                                   DIE &ScopeDIE, const MachineFunction &MF);

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

@@ -186,17 +186,18 @@ int64_t DwarfUnit::getDefaultLowerBound() const {
 
 /// Check whether the DIE for this MDNode can be shared across CUs.
 bool DwarfUnit::isShareableAcrossCUs(const DINode *D) const {
-  // When the MDNode can be part of the type system (this includes subprogram
-  // declarations *and* subprogram definitions, even local definitions), the
-  // DIE must be shared across CUs.
+  // When the MDNode can be part of the type system, the DIE can be shared
+  // across CUs.
   // Combining type units and cross-CU DIE sharing is lower value (since
   // cross-CU DIE sharing is used in LTO and removes type redundancy at that
   // level already) but may be implementable for some value in projects
   // building multiple independent libraries with LTO and then linking those
   // together.
   if (isDwoUnit() && !DD->shareAcrossDWOCUs())
     return false;
-  return (isa<DIType>(D) || isa<DISubprogram>(D)) && !DD->generateTypeUnits();
+  return (isa<DIType>(D) ||
+          (isa<DISubprogram>(D) && !cast<DISubprogram>(D)->isDefinition())) &&
+         !DD->generateTypeUnits();
 }
 
 DIE *DwarfUnit::getDIE(const DINode *D) const {

llvm/test/DebugInfo/AArch64/unretained-declaration-subprogram.ll

@@ -21,6 +21,10 @@
 
 # After w0 is clobbered, we should get an indirect parameter entry value for "f".
 
+# DWARF-LABEL: DW_TAG_subprogram
+# DWARF:       DW_AT_name ("baz")
+
+# DWARF-LABEL: DW_TAG_subprogram
 # DWARF-LABEL: DW_TAG_formal_parameter
 # DWARF-NEXT: DW_AT_location
 # DWARF-NEXT: [0x0000000000000000, 0x0000000000000010): DW_OP_breg0 W0+0

llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-indirect-param.mir

@@ -159,7 +159,7 @@ body:             |
 ...
 
 # CHECK: DW_TAG_GNU_call_site
-# CHECK-NEXT: DW_AT_abstract_origin ({{.*}} "call_int")
+# CHECK-NEXT: DW_AT_abstract_origin (0x0000002a "call_int")
 #
 # CHECK: DW_TAG_GNU_call_site_parameter
 # CHECK-NEXT: DW_AT_location      (DW_OP_reg0 W0)
@@ -205,7 +205,7 @@ body:             |
 ...
 
 # CHECK: DW_TAG_GNU_call_site
-# CHECK-NEXT: DW_AT_abstract_origin ({{.*}} "call_long")
+# CHECK-NEXT: DW_AT_abstract_origin (0x0000003e "call_long")
 #
 # CHECK: DW_TAG_GNU_call_site_parameter
 # CHECK-NEXT: DW_AT_location      (DW_OP_reg0 W0)
@@ -265,7 +265,7 @@ body:             |
 ...
 
 # CHECK: DW_TAG_GNU_call_site
-# CHECK-NEXT: DW_AT_abstract_origin ({{.*}} "call_int_int")
+# CHECK-NEXT: DW_AT_abstract_origin (0x00000052 "call_int_int")
 #
 # CHECK: DW_TAG_GNU_call_site_parameter
 # CHECK-NEXT: DW_AT_location      (DW_OP_reg0 W0)

llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-orr-moves.mir

@@ -1,8 +1,12 @@
 # RUN: llc -start-after=livedebugvalues -mtriple=x86_64-apple-darwin -o - %s -filetype=obj \
 # RUN:   -emit-call-site-info | llvm-dwarfdump - | FileCheck %s -implicit-check-not=call_site_parameter
 
-# CHECK: DW_TAG_formal_parameter
-# CHECK-NEXT: DW_AT_location (DW_OP_reg17 XMM0)
+# CHECK-LABEL: DW_TAG_subprogram
+# CHECK:       DW_AT_name ("f")
+# CHECK-LABEL: DW_TAG_subprogram
+# CHECK:       DW_AT_name ("g")
+# CHECK:       DW_TAG_formal_parameter
+# CHECK-NEXT:  DW_AT_location (DW_OP_reg17 XMM0)
 
 # struct S {
 #   float w;

llvm/test/DebugInfo/MIR/X86/callsite-stack-value.mir

@@ -39,11 +39,17 @@
 # CHECK-GNU-NEXT:      DW_AT_location      (DW_OP_reg8 R8)
 # CHECK-GNU-NEXT:      DW_AT_GNU_call_site_value   (DW_OP_breg14 R14+3)
 
-# CHECK-DWARF5: DW_TAG_call_site
-# CHECK-DWARF5:   DW_AT_call_origin ([[getValue_SP:.*]])
+# CHECK-DWARF5: [[getValue_SP:.*]]: DW_TAG_subprogram
+# CHECK-DWARF5-NEXT: DW_AT_name      ("getVal")
 
+# CHECK-DWARF5: [[foo_SP:.*]]: DW_TAG_subprogram
+# CHECK-DWARF5-NEXT: DW_AT_name      ("foo")
+
+# CHECK-DWARF5: DW_TAG_call_site
+# CHECK-DWARF5:   DW_AT_call_origin ([[getValue_SP]])
+#
 # CHECK-DWARF5: DW_TAG_call_site
-# CHECK-DWARF5:   DW_AT_call_origin ([[foo_SP:.*]])
+# CHECK-DWARF5:   DW_AT_call_origin ([[foo_SP]])
 # CHECK-DWARF5:   DW_AT_call_return_pc {{.*}}
 # CHECK-DWARF5-EMPTY:
 # CHECK-DWARF5:         DW_TAG_call_site_parameter
@@ -65,12 +71,6 @@
 # CHECK-DWARF5-NEXT:      DW_AT_location      (DW_OP_reg8 R8)
 # CHECK-DWARF5-NEXT:      DW_AT_call_value   (DW_OP_breg14 R14+3)
 
-# CHECK-DWARF5: [[getValue_SP]]: DW_TAG_subprogram
-# CHECK-DWARF5-NEXT: DW_AT_name      ("getVal")
-
-# CHECK-DWARF5: [[foo_SP]]: DW_TAG_subprogram
-# CHECK-DWARF5-NEXT: DW_AT_name      ("foo")
-
 --- |
   ; ModuleID = 'test.c'
   source_filename = "test.c"