[llvm] [CodeExtractor] Copy live-in dbg records whole instead of rebuilding (PR #227794)

[Oliver Hunt via llvm-commits]

================
@@ -1309,33 +1373,30 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
       /*LineNo=*/0, SPType, /*ScopeLine=*/0, DINode::FlagZero, SPFlags);
   NewFunc.setSubprogram(NewSP);
 
-  auto UpdateOrInsertDebugRecord = [&](auto *DR, Value *OldLoc, Value *NewLoc,
-                                       DIExpression *Expr, bool Declare) {
-    if (DR->getParent()->getParent() == &NewFunc) {
-      DR->replaceVariableLocationOp(OldLoc, NewLoc);
-      return;
-    }
-    if (Declare) {
-      DIB.insertDeclare(NewLoc, DR->getVariable(), Expr, DR->getDebugLoc(),
-                        &NewFunc.getEntryBlock());
-      return;
-    }
-    DIB.insertDbgValue(NewLoc, DR->getVariable(), Expr, DR->getDebugLoc(),
-                       NewFunc.getEntryBlock().getTerminator()->getIterator());
-  };
+  // Records are copied whole and only ever have their operands substituted, so
+  // a location list and the expression that indexes into it stay consistent.
+  SmallSetVector<DbgVariableRecord *, 8> LiveIn;
+  collectLiveInDbgRecords(TheCall, Inputs, LiveIn);
+  BasicBlock &NewEntry = NewFunc.getEntryBlock();
+  for (DbgVariableRecord *DVR : LiveIn) {
+    // The stores an assignment ID links to stay in the parent, so a copied
+    // #dbg_assign keeps only the value it describes.
+    DbgVariableRecord *Copy =
+        DVR->isDbgAssign()
+            ? new DbgVariableRecord(DVR->getRawLocation(), DVR->getVariable(),
+                                    DVR->getExpression(),
+                                    DVR->getDebugLoc().get())
+            : DVR->clone();
+    NewEntry.insertDbgRecordBefore(Copy,
+                                   NewEntry.getTerminator()->getIterator());
----------------
ojhunt wrote:

I believe that this is wrong - in the isDbgAssign path I can't see how we're reconstructing the correctly typed node.

But more importantly I can't see how this is correctly reparenting - the existence of helper functions makes me think that copying these nodes is not trivial and involves significant state managing work. I suspect that this is going to result in the new function referencing nodes in the old function.


https://github.com/llvm/llvm-project/pull/227794