DI: Reverse direction of subprogram -> function edge.

Previously, subprograms contained a metadata reference to the function they
described. Because most clients need to get or set a subprogram for a given
function rather than the other way around, this created unneeded inefficiency.

For example, many passes needed to call the function llvm::makeSubprogramMap()
to build a mapping from functions to subprograms, and the IR linker needed to
fix up function references in a way that caused quadratic complexity in the IR
linking phase of LTO.

This change reverses the direction of the edge by storing the subprogram as
function-level metadata and removing DISubprogram's function field.

Since this is an IR change, a bitcode upgrade has been provided.

Fixes PR23367. An upgrade script for textual IR for out-of-tree clients is
attached to the PR.

Differential Revision: http://reviews.llvm.org/D14265

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@252219 91177308-0d34-0410-b5e6-96231b3b80d8

bindings/go/llvm/DIBuilderBindings.cpp

@@ -74,13 +74,13 @@ LLVMMetadataRef LLVMDIBuilderCreateFunction(
     LLVMDIBuilderRef Dref, LLVMMetadataRef Scope, const char *Name,
     const char *LinkageName, LLVMMetadataRef File, unsigned Line,
     LLVMMetadataRef CompositeType, int IsLocalToUnit, int IsDefinition,
-    unsigned ScopeLine, unsigned Flags, int IsOptimized, LLVMValueRef Func) {
+    unsigned ScopeLine, unsigned Flags, int IsOptimized) {
   DIBuilder *D = unwrap(Dref);
   return wrap(D->createFunction(unwrap<DIScope>(Scope), Name, LinkageName,
                                 File ? unwrap<DIFile>(File) : nullptr, Line,
                                 unwrap<DISubroutineType>(CompositeType),
                                 IsLocalToUnit, IsDefinition, ScopeLine, Flags,
-                                IsOptimized, unwrap<Function>(Func)));
+                                IsOptimized));
 }
 
 LLVMMetadataRef

bindings/go/llvm/DIBuilderBindings.h

@@ -55,7 +55,7 @@ LLVMMetadataRef LLVMDIBuilderCreateFunction(
     LLVMDIBuilderRef D, LLVMMetadataRef Scope, const char *Name,
     const char *LinkageName, LLVMMetadataRef File, unsigned Line,
     LLVMMetadataRef CompositeType, int IsLocalToUnit, int IsDefinition,
-    unsigned ScopeLine, unsigned Flags, int IsOptimized, LLVMValueRef Function);
+    unsigned ScopeLine, unsigned Flags, int IsOptimized);
 
 LLVMMetadataRef
 LLVMDIBuilderCreateAutoVariable(LLVMDIBuilderRef D, LLVMMetadataRef Scope,

bindings/go/llvm/IRBindings.cpp

@@ -98,3 +98,7 @@ void LLVMSetCurrentDebugLocation2(LLVMBuilderRef Bref, unsigned Line,
       DebugLoc::get(Line, Col, Scope ? unwrap<MDNode>(Scope) : nullptr,
                     InlinedAt ? unwrap<MDNode>(InlinedAt) : nullptr));
 }
+
+void LLVMSetSubprogram(LLVMValueRef Func, LLVMMetadataRef SP) {
+  unwrap<Function>(Func)->setSubprogram(unwrap<DISubprogram>(SP));
+}

bindings/go/llvm/IRBindings.h

@@ -55,6 +55,8 @@ void LLVMSetCurrentDebugLocation2(LLVMBuilderRef Bref, unsigned Line,
                                   unsigned Col, LLVMMetadataRef Scope,
                                   LLVMMetadataRef InlinedAt);
 
+void LLVMSetSubprogram(LLVMValueRef Fn, LLVMMetadataRef SP);
+
 #ifdef __cplusplus
 }
 

bindings/go/llvm/dibuilder.go

@@ -189,7 +189,6 @@ type DIFunction struct {
 	ScopeLine    int
 	Flags        int
 	Optimized    bool
-	Function     Value
 }
 
 // CreateCompileUnit creates function debug metadata.
@@ -211,7 +210,6 @@ func (d *DIBuilder) CreateFunction(diScope Metadata, f DIFunction) Metadata {
 		C.unsigned(f.ScopeLine),
 		C.unsigned(f.Flags),
 		boolToCInt(f.Optimized),
-		f.Function.C,
 	)
 	return Metadata{C: result}
 }

bindings/go/llvm/ir.go

@@ -1057,6 +1057,9 @@ func (v Value) AddTargetDependentFunctionAttr(attr, value string) {
 func (v Value) SetPersonality(p Value) {
 	C.LLVMSetPersonalityFn(v.C, p.C)
 }
+func (v Value) SetSubprogram(sp Metadata) {
+	C.LLVMSetSubprogram(v.C, sp.C)
+}
 
 // Operations on parameters
 func (v Value) ParamsCount() int { return int(C.LLVMCountParams(v.C)) }

include/llvm/IR/DIBuilder.h

@@ -515,33 +515,35 @@ namespace llvm {
     ///                      These flags are used to emit dwarf attributes.
     /// \param isOptimized   True if optimization is ON.
     /// \param Fn            llvm::Function pointer.
-    /// \param TParam        Function template parameters.
-    DISubprogram *
-    createFunction(DIScope *Scope, StringRef Name, StringRef LinkageName,
-                   DIFile *File, unsigned LineNo, DISubroutineType *Ty,
-                   bool isLocalToUnit, bool isDefinition, unsigned ScopeLine,
-                   unsigned Flags = 0, bool isOptimized = false,
-                   Function *Fn = nullptr, MDNode *TParam = nullptr,
-                   MDNode *Decl = nullptr);
+    /// \param TParams       Function template parameters.
+    DISubprogram *createFunction(DIScope *Scope, StringRef Name,
+                                 StringRef LinkageName, DIFile *File,
+                                 unsigned LineNo, DISubroutineType *Ty,
+                                 bool isLocalToUnit, bool isDefinition,
+                                 unsigned ScopeLine, unsigned Flags = 0,
+                                 bool isOptimized = false,
+                                 DITemplateParameterArray TParams = nullptr,
+                                 DISubprogram *Decl = nullptr);
 
     /// Identical to createFunction,
     /// except that the resulting DbgNode is meant to be RAUWed.
     DISubprogram *createTempFunctionFwdDecl(
         DIScope *Scope, StringRef Name, StringRef LinkageName, DIFile *File,
         unsigned LineNo, DISubroutineType *Ty, bool isLocalToUnit,
         bool isDefinition, unsigned ScopeLine, unsigned Flags = 0,
-        bool isOptimized = false, Function *Fn = nullptr,
-        MDNode *TParam = nullptr, MDNode *Decl = nullptr);
+        bool isOptimized = false, DITemplateParameterArray TParams = nullptr,
+        DISubprogram *Decl = nullptr);
 
     /// FIXME: this is added for dragonegg. Once we update dragonegg
     /// to call resolve function, this will be removed.
-    DISubprogram *
-    createFunction(DIScopeRef Scope, StringRef Name, StringRef LinkageName,
-                   DIFile *File, unsigned LineNo, DISubroutineType *Ty,
-                   bool isLocalToUnit, bool isDefinition, unsigned ScopeLine,
-                   unsigned Flags = 0, bool isOptimized = false,
-                   Function *Fn = nullptr, MDNode *TParam = nullptr,
-                   MDNode *Decl = nullptr);
+    DISubprogram *createFunction(DIScopeRef Scope, StringRef Name,
+                                 StringRef LinkageName, DIFile *File,
+                                 unsigned LineNo, DISubroutineType *Ty,
+                                 bool isLocalToUnit, bool isDefinition,
+                                 unsigned ScopeLine, unsigned Flags = 0,
+                                 bool isOptimized = false,
+                                 DITemplateParameterArray TParams = nullptr,
+                                 DISubprogram *Decl = nullptr);
 
     /// Create a new descriptor for the specified C++ method.
     /// See comments in \a DISubprogram* for descriptions of these fields.
@@ -561,14 +563,14 @@ namespace llvm {
     ///                      This flags are used to emit dwarf attributes.
     /// \param isOptimized   True if optimization is ON.
     /// \param Fn            llvm::Function pointer.
-    /// \param TParam        Function template parameters.
+    /// \param TParams       Function template parameters.
     DISubprogram *
     createMethod(DIScope *Scope, StringRef Name, StringRef LinkageName,
                  DIFile *File, unsigned LineNo, DISubroutineType *Ty,
                  bool isLocalToUnit, bool isDefinition, unsigned Virtuality = 0,
                  unsigned VTableIndex = 0, DIType *VTableHolder = nullptr,
                  unsigned Flags = 0, bool isOptimized = false,
-                 Function *Fn = nullptr, MDNode *TParam = nullptr);
+                 DITemplateParameterArray TParams = nullptr);
 
     /// This creates new descriptor for a namespace with the specified
     /// parent scope.

include/llvm/IR/DebugInfo.h

@@ -143,8 +143,6 @@ class DebugInfoFinder {
   bool TypeMapInitialized;
 };
 
-DenseMap<const Function *, DISubprogram *> makeSubprogramMap(const Module &M);
-
 } // end namespace llvm
 
 #endif

include/llvm/IR/DebugInfoMetadata.h

@@ -1237,14 +1237,13 @@ class DISubprogram : public DILocalScope {
           DISubroutineType *Type, bool IsLocalToUnit, bool IsDefinition,
           unsigned ScopeLine, DITypeRef ContainingType, unsigned Virtuality,
           unsigned VirtualIndex, unsigned Flags, bool IsOptimized,
-          Constant *Function, DITemplateParameterArray TemplateParams,
-          DISubprogram *Declaration, DILocalVariableArray Variables,
-          StorageType Storage, bool ShouldCreate = true) {
+          DITemplateParameterArray TemplateParams, DISubprogram *Declaration,
+          DILocalVariableArray Variables, StorageType Storage,
+          bool ShouldCreate = true) {
     return getImpl(Context, Scope, getCanonicalMDString(Context, Name),
                    getCanonicalMDString(Context, LinkageName), File, Line, Type,
                    IsLocalToUnit, IsDefinition, ScopeLine, ContainingType,
                    Virtuality, VirtualIndex, Flags, IsOptimized,
-                   Function ? ConstantAsMetadata::get(Function) : nullptr,
                    TemplateParams.get(), Declaration, Variables.get(), Storage,
                    ShouldCreate);
   }
@@ -1253,17 +1252,16 @@ class DISubprogram : public DILocalScope {
           MDString *LinkageName, Metadata *File, unsigned Line, Metadata *Type,
           bool IsLocalToUnit, bool IsDefinition, unsigned ScopeLine,
           Metadata *ContainingType, unsigned Virtuality, unsigned VirtualIndex,
-          unsigned Flags, bool IsOptimized, Metadata *Function,
-          Metadata *TemplateParams, Metadata *Declaration, Metadata *Variables,
-          StorageType Storage, bool ShouldCreate = true);
+          unsigned Flags, bool IsOptimized, Metadata *TemplateParams,
+          Metadata *Declaration, Metadata *Variables, StorageType Storage,
+          bool ShouldCreate = true);
 
   TempDISubprogram cloneImpl() const {
-    return getTemporary(getContext(), getScope(), getName(), getLinkageName(),
-                        getFile(), getLine(), getType(), isLocalToUnit(),
-                        isDefinition(), getScopeLine(), getContainingType(),
-                        getVirtuality(), getVirtualIndex(), getFlags(),
-                        isOptimized(), getFunctionConstant(),
-                        getTemplateParams(), getDeclaration(), getVariables());
+    return getTemporary(
+        getContext(), getScope(), getName(), getLinkageName(), getFile(),
+        getLine(), getType(), isLocalToUnit(), isDefinition(), getScopeLine(),
+        getContainingType(), getVirtuality(), getVirtualIndex(), getFlags(),
+        isOptimized(), getTemplateParams(), getDeclaration(), getVariables());
   }
 
 public:
@@ -1273,25 +1271,24 @@ class DISubprogram : public DILocalScope {
                      bool IsLocalToUnit, bool IsDefinition, unsigned ScopeLine,
                      DITypeRef ContainingType, unsigned Virtuality,
                      unsigned VirtualIndex, unsigned Flags, bool IsOptimized,
-                     Constant *Function = nullptr,
                      DITemplateParameterArray TemplateParams = nullptr,
                      DISubprogram *Declaration = nullptr,
                      DILocalVariableArray Variables = nullptr),
                     (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit,
                      IsDefinition, ScopeLine, ContainingType, Virtuality,
-                     VirtualIndex, Flags, IsOptimized, Function, TemplateParams,
+                     VirtualIndex, Flags, IsOptimized, TemplateParams,
                      Declaration, Variables))
   DEFINE_MDNODE_GET(
       DISubprogram,
       (Metadata * Scope, MDString *Name, MDString *LinkageName, Metadata *File,
        unsigned Line, Metadata *Type, bool IsLocalToUnit, bool IsDefinition,
        unsigned ScopeLine, Metadata *ContainingType, unsigned Virtuality,
        unsigned VirtualIndex, unsigned Flags, bool IsOptimized,
-       Metadata *Function = nullptr, Metadata *TemplateParams = nullptr,
-       Metadata *Declaration = nullptr, Metadata *Variables = nullptr),
+       Metadata *TemplateParams = nullptr, Metadata *Declaration = nullptr,
+       Metadata *Variables = nullptr),
       (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit, IsDefinition,
        ScopeLine, ContainingType, Virtuality, VirtualIndex, Flags, IsOptimized,
-       Function, TemplateParams, Declaration, Variables))
+       TemplateParams, Declaration, Variables))
 
   TempDISubprogram clone() const { return cloneImpl(); }
 
@@ -1350,11 +1347,6 @@ class DISubprogram : public DILocalScope {
     return DITypeRef(getRawContainingType());
   }
 
-  Constant *getFunctionConstant() const {
-    if (auto *C = cast_or_null<ConstantAsMetadata>(getRawFunction()))
-      return C->getValue();
-    return nullptr;
-  }
   DITemplateParameterArray getTemplateParams() const {
     return cast_or_null<MDTuple>(getRawTemplateParams());
   }
@@ -1368,28 +1360,9 @@ class DISubprogram : public DILocalScope {
   Metadata *getRawScope() const { return getOperand(1); }
   Metadata *getRawType() const { return getOperand(5); }
   Metadata *getRawContainingType() const { return getOperand(6); }
-  Metadata *getRawFunction() const { return getOperand(7); }
-  Metadata *getRawTemplateParams() const { return getOperand(8); }
-  Metadata *getRawDeclaration() const { return getOperand(9); }
-  Metadata *getRawVariables() const { return getOperand(10); }
-
-  /// \brief Get a pointer to the function this subprogram describes.
-  ///
-  /// This dyn_casts \a getFunctionConstant() to \a Function.
-  ///
-  /// FIXME: Should this be looking through bitcasts?
-  Function *getFunction() const;
-
-  /// \brief Replace the function.
-  ///
-  /// If \a isUniqued() and not \a isResolved(), this could node will be
-  /// RAUW'ed and deleted out from under the caller.  Use a \a TrackingMDRef if
-  /// that's a problem.
-  /// @{
-  void replaceFunction(Function *F);
-  void replaceFunction(ConstantAsMetadata *MD) { replaceOperandWith(7, MD); }
-  void replaceFunction(std::nullptr_t) { replaceOperandWith(7, nullptr); }
-  /// @}
+  Metadata *getRawTemplateParams() const { return getOperand(7); }
+  Metadata *getRawDeclaration() const { return getOperand(8); }
+  Metadata *getRawVariables() const { return getOperand(9); }
 
   /// \brief Check if this subprogram describes the given function.
   ///

lib/AsmParser/LLParser.cpp

@@ -3807,8 +3807,8 @@ bool LLParser::ParseDICompileUnit(MDNode *&Result, bool IsDistinct) {
 ///                     isDefinition: true, scopeLine: 8, containingType: !3,
 ///                     virtuality: DW_VIRTUALTIY_pure_virtual,
 ///                     virtualIndex: 10, flags: 11,
-///                     isOptimized: false, function: void ()* @_Z3foov,
-///                     templateParams: !4, declaration: !5, variables: !6)
+///                     isOptimized: false, templateParams: !4, declaration: !5,
+///                     variables: !6)
 bool LLParser::ParseDISubprogram(MDNode *&Result, bool IsDistinct) {
   auto Loc = Lex.getLoc();
 #define VISIT_MD_FIELDS(OPTIONAL, REQUIRED)                                    \
@@ -3826,7 +3826,6 @@ bool LLParser::ParseDISubprogram(MDNode *&Result, bool IsDistinct) {
   OPTIONAL(virtualIndex, MDUnsignedField, (0, UINT32_MAX));                    \
   OPTIONAL(flags, DIFlagField, );                                              \
   OPTIONAL(isOptimized, MDBoolField, );                                        \
-  OPTIONAL(function, MDConstant, );                                            \
   OPTIONAL(templateParams, MDField, );                                         \
   OPTIONAL(declaration, MDField, );                                            \
   OPTIONAL(variables, MDField, );
@@ -3839,11 +3838,11 @@ bool LLParser::ParseDISubprogram(MDNode *&Result, bool IsDistinct) {
         "missing 'distinct', required for !DISubprogram when 'isDefinition'");
 
   Result = GET_OR_DISTINCT(
-      DISubprogram, (Context, scope.Val, name.Val, linkageName.Val, file.Val,
-                     line.Val, type.Val, isLocal.Val, isDefinition.Val,
-                     scopeLine.Val, containingType.Val, virtuality.Val,
-                     virtualIndex.Val, flags.Val, isOptimized.Val, function.Val,
-                     templateParams.Val, declaration.Val, variables.Val));
+      DISubprogram,
+      (Context, scope.Val, name.Val, linkageName.Val, file.Val, line.Val,
+       type.Val, isLocal.Val, isDefinition.Val, scopeLine.Val,
+       containingType.Val, virtuality.Val, virtualIndex.Val, flags.Val,
+       isOptimized.Val, templateParams.Val, declaration.Val, variables.Val));
   return false;
 }
 

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -232,6 +232,10 @@ class BitcodeReader : public GVMaterializer {
 
   bool StripDebugInfo = false;
 
+  /// Functions that need to be matched with subprograms when upgrading old
+  /// metadata.
+  SmallDenseMap<Function *, DISubprogram *, 16> FunctionsWithSPs;
+
   std::vector<std::string> BundleTags;
 
 public:
@@ -2182,20 +2186,33 @@ std::error_code BitcodeReader::parseMetadata() {
       break;
     }
     case bitc::METADATA_SUBPROGRAM: {
-      if (Record.size() != 19)
-        return error("Invalid record");
-
-      MDValueList.assignValue(
-          GET_OR_DISTINCT(
-              DISubprogram,
-              Record[0] || Record[8], // All definitions should be distinct.
-              (Context, getMDOrNull(Record[1]), getMDString(Record[2]),
-               getMDString(Record[3]), getMDOrNull(Record[4]), Record[5],
-               getMDOrNull(Record[6]), Record[7], Record[8], Record[9],
-               getMDOrNull(Record[10]), Record[11], Record[12], Record[13],
-               Record[14], getMDOrNull(Record[15]), getMDOrNull(Record[16]),
-               getMDOrNull(Record[17]), getMDOrNull(Record[18]))),
-          NextMDValueNo++);
+      if (Record.size() != 18 && Record.size() != 19)
+        return error("Invalid record");
+
+      bool HasFn = Record.size() == 19;
+      DISubprogram *SP = GET_OR_DISTINCT(
+          DISubprogram,
+          Record[0] || Record[8], // All definitions should be distinct.
+          (Context, getMDOrNull(Record[1]), getMDString(Record[2]),
+           getMDString(Record[3]), getMDOrNull(Record[4]), Record[5],
+           getMDOrNull(Record[6]), Record[7], Record[8], Record[9],
+           getMDOrNull(Record[10]), Record[11], Record[12], Record[13],
+           Record[14], getMDOrNull(Record[15 + HasFn]),
+           getMDOrNull(Record[16 + HasFn]), getMDOrNull(Record[17 + HasFn])));
+      MDValueList.assignValue(SP, NextMDValueNo++);
+
+      // Upgrade sp->function mapping to function->sp mapping.
+      if (HasFn && Record[15]) {
+        if (auto *CMD = dyn_cast<ConstantAsMetadata>(getMDOrNull(Record[15])))
+          if (auto *F = dyn_cast<Function>(CMD->getValue())) {
+            if (F->isMaterializable())
+              // Defer until materialized; unmaterialized functions may not have
+              // metadata.
+              FunctionsWithSPs[F] = SP;
+            else if (!F->empty())
+              F->setSubprogram(SP);
+          }
+      }
       break;
     }
     case bitc::METADATA_LEXICAL_BLOCK: {
@@ -5139,6 +5156,10 @@ std::error_code BitcodeReader::materialize(GlobalValue *GV) {
     }
   }
 
+  // Finish fn->subprogram upgrade for materialized functions.
+  if (DISubprogram *SP = FunctionsWithSPs.lookup(F))
+    F->setSubprogram(SP);
+
   // Bring in any functions that this function forward-referenced via
   // blockaddresses.
   return materializeForwardReferencedFunctions();

lib/Bitcode/Writer/BitcodeWriter.cpp

@@ -1024,7 +1024,6 @@ static void WriteDISubprogram(const DISubprogram *N, const ValueEnumerator &VE,
   Record.push_back(N->getVirtualIndex());
   Record.push_back(N->getFlags());
   Record.push_back(N->isOptimized());
-  Record.push_back(VE.getMetadataOrNullID(N->getRawFunction()));
   Record.push_back(VE.getMetadataOrNullID(N->getTemplateParams().get()));
   Record.push_back(VE.getMetadataOrNullID(N->getDeclaration()));
   Record.push_back(VE.getMetadataOrNullID(N->getVariables().get()));