<div dir="ltr">If SPs are now all finalized early - I'm assuming there's some other code that finalizes SPs that's no longer needed?<br><br>Also - this seems like a layering/separation issue - does other code call into the DI as casually/explicitly as this code being added to CGVTables? (Or should the callback go through some more generic entry point in CGDebugInfo to delegate/handle that?) & is there nothing in CGDebugInfo that is already called back in a timely fashion to handle this? I'm guessing what's needed is a "IR has finished being generated for this function, opt/codegen is about to start"? That seems like a good generic callback & probably shouldn't be coming from something called CGVTables? (presuming there's a more general place to put that)</div><br><div class="gmail_quote"><div dir="ltr">On Tue, May 30, 2017 at 5:30 PM Keno Fischer via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">loladiro created this revision.<br>
<br>
LLVM commit <a href="https://reviews.llvm.org/D33655" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33655</a> was reverted, because it exposed an assertion failure,<br>
in `GenerateVarArgsThunk`. That function attempts to clone a function<br>
before clang is entirely done generating the debug info for the current<br>
compliation unit. That is not a valid use of the API, because it expects<br>
that there are no temporary MD nodes in the nodes that it needs to clone.<br>
However, until now, this did not cause any problems (though could have<br>
caused assertions in the backend due to mismatched debug info metadata).<br>
<br>
Attempt to rectify this by finalizing the SP before attempting to clone it.<br>
The requisite LLVM API is being introduced in <a href="https://reviews.llvm.org/D33704" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33704</a>.<br>
<br>
<br>
<a href="https://reviews.llvm.org/D33705" rel="noreferrer" target="_blank">https://reviews.llvm.org/D33705</a><br>
<br>
Files:<br>
  lib/CodeGen/CGDebugInfo.h<br>
  lib/CodeGen/CGVTables.cpp<br>
<br>
<br>
Index: lib/CodeGen/CGVTables.cpp<br>
===================================================================<br>
--- lib/CodeGen/CGVTables.cpp<br>
+++ lib/CodeGen/CGVTables.cpp<br>
@@ -152,6 +152,12 @@<br>
   llvm::Value *Callee = CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);<br>
   llvm::Function *BaseFn = cast<llvm::Function>(Callee);<br>
<br>
+  // We may not clone a subprogram that contains temporary metadata, so finalize<br>
+  // it now - we will not be modifying it.<br>
+  if (CGDebugInfo *DI = CGM.getModuleDebugInfo())<br>
+    if (llvm::DISubprogram *SP = BaseFn->getSubprogram())<br>
+      DI->finalizeSP(SP);<br>
+<br>
   // Clone to thunk.<br>
   llvm::ValueToValueMapTy VMap;<br>
   llvm::Function *NewFn = llvm::CloneFunction(BaseFn, VMap);<br>
Index: lib/CodeGen/CGDebugInfo.h<br>
===================================================================<br>
--- lib/CodeGen/CGDebugInfo.h<br>
+++ lib/CodeGen/CGDebugInfo.h<br>
@@ -308,6 +308,7 @@<br>
   ~CGDebugInfo();<br>
<br>
   void finalize();<br>
+  void finalizeSP(llvm::DISubprogram *SP) { DBuilder.finalizeSP(SP); }<br>
<br>
   /// Module debugging: Support for building PCMs.<br>
   /// @{<br>
<br>
<br>
</blockquote></div>