[DIBuilder] Add a more fine-grained finalization method

[loladiro]

Summary

Clang wants to clone a function before it is done building the entire
compilation unit. As of now, there is no good way to do that, because
CloneFunction doesn't like dealing with temporary metadata. However,
as long as clang doesn't want to add any variables to this SP, it
should be fine to just prematurely finalize it. Add an API to allow this.

This is done in preparation of a clang commit to fix the assertion that
necessitated the revert of D33655.

[aprantl]

I was a bit scared by the description at first, but this is safe. Will you only invoke it on functions to be cloned in clang or on all subprograms?

include/llvm/IR/DIBuilder.h

91

I wonder if finalizeVariable(DISubprogram *SP) would be a more accurate name?

91

finalizeVariable*s*.

[loladiro]

Will you only invoke it on functions to be cloned in clang or on all subprograms?

D33705 is the clang part of this. @dblaikie had some concerns (on the mailing list) - I had it only call it on cloned subprogram, but @dblaikie thought it might be more natural to call it immediately when irgen is done.

As for the name, I don't think finalizeVariables is necessarily a great name, because the semantics I want is that there's no temporary nodes left in that SP. Right now that might only be variables, but in the feature if there are ever others, I'd want this function to finalize those as well. I'm fine either way though.

[aprantl]

https://reviews.llvm.org/D33705 is the clang part of this. @dblaikie had some concerns (on the mailing list) - I had it only call it on cloned subprogram, but @dblaikie thought it might be more natural to call it immediately when irgen is done.

I couldn't find the thread — yes that was what I was wondering, too. It looks like AllSubprograms is only used for this purpose and if there is a natural point where to invoke finalize on each SP after IRGen, then this may be better. But it may also complicate all frontends needlessly so I am not sure whether this is a good trade-off. I guess I can be swayed either way.

include/llvm/IR/DIBuilder.h

91

Ok, then how about finalizeSubprogram()?

[loladiro]

finalizeSP -> finalizeSubprogram

I don't think there's really a natural place in DIBuilder to do this, but
I suspect that most frontends would have one.

[aprantl]

I'm fine with this change. I'm fine with either expecting the frontends to manually call finalize on each subprogram or with having DIBuilder do it. But let's have that discussion over in https://reviews.llvm.org/D33705.

include/llvm/IR/DIBuilder.h

public:
     /// in order to resolve cycles during \a finalize().
     explicit DIBuilder(Module &M, bool AllowUnresolved = true);
     DIBuilder(const DIBuilder &) = delete;
     DIBuilder &operator=(const DIBuilder &) = delete;
 
     /// Construct any deferred debug info descriptors.
     void finalize();
 
+    /// Finalize a specific subprogram - no new variables may be added to this
+    /// subprogram afterwards.
+    void finalizeSubprogram(DISubprogram *SP);
+
     /// A CompileUnit provides an anchor for all debugging
     /// information generated during this instance of compilation.
     /// \param Lang          Source programming language, eg. dwarf::DW_LANG_C99
     /// \param File          File info.
     /// \param Producer      Identify the producer of debugging information
     ///                      and code.  Usually this is a compiler
     ///                      version string.
     /// \param isOptimized   A boolean flag which indicates whether optimization

lib/IR/DIBuilder.cpp

if (!N)
     return;
   if (N->isResolved())
     return;
 
   assert(AllowUnresolvedNodes && "Cannot handle unresolved nodes");
   UnresolvedNodes.emplace_back(N);
 }
 
+void DIBuilder::finalizeSubprogram(DISubprogram *SP) {
+  MDTuple *Temp = SP->getVariables().get();
+  if (!Temp || !Temp->isTemporary())
+    return;
+
+  SmallVector<Metadata *, 4> Variables;
+
+  auto PV = PreservedVariables.find(SP);
+  if (PV != PreservedVariables.end())
+    Variables.append(PV->second.begin(), PV->second.end());
+
+  DINodeArray AV = getOrCreateArray(Variables);
+  TempMDTuple(Temp)->replaceAllUsesWith(AV.get());
+}
+
 void DIBuilder::finalize() {
   if (!CUNode) {
     assert(!AllowUnresolvedNodes &&
            "creating type nodes without a CU is not supported");
     return;
   }
 
   CUNode->replaceEnumTypes(MDTuple::get(VMContext, AllEnumTypes));
 
   SmallVector<Metadata *, 16> RetainValues;
   // Declarations and definitions of the same type may be retained. Some
   // clients RAUW these pairs, leaving duplicates in the retained types
   // list. Use a set to remove the duplicates while we transform the
   // TrackingVHs back into Values.
   SmallPtrSet<Metadata *, 16> RetainSet;
   for (unsigned I = 0, E = AllRetainTypes.size(); I < E; I++)
     if (RetainSet.insert(AllRetainTypes[I]).second)
       RetainValues.push_back(AllRetainTypes[I]);
 
   if (!RetainValues.empty())
     CUNode->replaceRetainedTypes(MDTuple::get(VMContext, RetainValues));
 
   DISubprogramArray SPs = MDTuple::get(VMContext, AllSubprograms);
-  auto resolveVariables = [&](DISubprogram *SP) {
-    MDTuple *Temp = SP->getVariables().get();
-    if (!Temp)
-      return;
-
-    SmallVector<Metadata *, 4> Variables;
-
-    auto PV = PreservedVariables.find(SP);
-    if (PV != PreservedVariables.end())
-      Variables.append(PV->second.begin(), PV->second.end());
-
-    DINodeArray AV = getOrCreateArray(Variables);
-    TempMDTuple(Temp)->replaceAllUsesWith(AV.get());
-  };
   for (auto *SP : SPs)
-    resolveVariables(SP);
+    finalizeSubprogram(SP);
   for (auto *N : RetainValues)
     if (auto *SP = dyn_cast<DISubprogram>(N))
-      resolveVariables(SP);
+      finalizeSubprogram(SP);
 
   if (!AllGVs.empty())
     CUNode->replaceGlobalVariables(MDTuple::get(VMContext, AllGVs));
 
   if (!AllImportedModules.empty())
     CUNode->replaceImportedEntities(MDTuple::get(
         VMContext, SmallVector<Metadata *, 16>(AllImportedModules.begin(),
                                                AllImportedModules.end())));