[CodeExtractor] Consider Value arguments of dbg.assign

[felipepiovezan]

Currently, the code extractor functionality deletes a debug intrinsic if its "Location" argument is not part of the extracted function. The location is the first argument (or the first few arguments in case of a DIArgList) of all debug intrinsics.

However, according to the docs, the signature of dbg.assign is:

void @llvm.dbg.assign(Value *Value,
                      DIExpression *ValueExpression,
                      DILocalVariable *Variable,
                      DIAssignID *ID,
                      Value *Address,
                      DIExpression *AddressExpression)

That is, there are two Value arguments to it: the usual location argument and an "Address" argument. This Address argument should also receive the same treatment.

[llvmbot]

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-llvm-transforms

Changes

Currently, the code extractor functionality deletes a debug intrinsic if its "Location" argument is not part of the extracted function. The location is the first argument (or the first few arguments in case of a DIArgList) of all debug intrinsics.

However, according to the docs, the signature of dbg.assign is:

void @<!-- -->llvm.dbg.assign(Value *Value,
                      DIExpression *ValueExpression,
                      DILocalVariable *Variable,
                      DIAssignID *ID,
                      Value *Address,
                      DIExpression *AddressExpression)

That is, there are two Value arguments to it: the usual location argument and an "Address" argument. This Address argument should also receive the same treatment.

---

Full diff: https://github.com/llvm/llvm-project/pull/67987.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Utils/CodeExtractor.cpp (+5)
• (added) llvm/test/Transforms/HotColdSplit/invalid-dbg-assign.ll (+37)

diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
index cafa99491f5b5f6..81d9d91d2e30b1f 100644
--- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -1579,6 +1579,11 @@ static void fixupDebugInfoPostExtraction(Function &OldFunc, Function &NewFunc,
       DebugIntrinsicsToDelete.push_back(DVI);
       continue;
     }
+    if (auto *DAI = dyn_cast<DbgAssignIntrinsic>(DVI);
+        DAI && IsInvalidLocation(DAI->getAddress())) {
+      DebugIntrinsicsToDelete.push_back(DVI);
+      continue;
+    }
     // If the variable was in the scope of the old function, i.e. it was not
     // inlined, point the intrinsic to a fresh variable within the new function.
     if (!DVI->getDebugLoc().getInlinedAt()) {
diff --git a/llvm/test/Transforms/HotColdSplit/invalid-dbg-assign.ll b/llvm/test/Transforms/HotColdSplit/invalid-dbg-assign.ll
new file mode 100644
index 000000000000000..3163fc7071e978a
--- /dev/null
+++ b/llvm/test/Transforms/HotColdSplit/invalid-dbg-assign.ll
@@ -0,0 +1,37 @@
+; RUN: opt -passes=hotcoldsplit -hotcoldsplit-threshold=-1 -S %s | FileCheck %s
+declare void @llvm.dbg.assign(metadata, metadata, metadata, metadata, metadata, metadata)
+
+; CHECK: define void @foo
+; CHECK-NOT: dbg.assign
+; CHECK: call void @foo.cold
+; CHECK-NOT: dbg.assign
+; CHECK: define internal void @foo.cold
+; CHECK-NOT: dbg.assign
+define void @foo() !dbg !10 {
+  %buf.i = alloca i32, align 4, !DIAssignID !8
+  br i1 false, label %if.else, label %if.then
+
+if.then:                                          ; preds = %0
+  call void @llvm.dbg.assign(metadata i1 undef, metadata !9, metadata !DIExpression(), metadata !8, metadata ptr %buf.i, metadata !DIExpression()), !dbg !14
+  unreachable
+
+if.else:                                          ; preds = %0
+  ret void
+}
+
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C11, file: !1, producer: "blah", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !2, globals: !2)
+!1 = !DIFile(filename: "blah", directory: "blah")
+!2 = !{}
+!3 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubroutineType(types: !7)
+!7 = !{null}
+!8 = distinct !DIAssignID()
+!9 = !DILocalVariable(name: "buf", scope: !10, file: !1, line: 1774, type: !13)
+!10 = distinct !DISubprogram(name: "blah", scope: !1, file: !1, line: 1771, type: !11, scopeLine: 1773, unit: !0, retainedNodes: !2)
+!11 = !DISubroutineType(cc: DW_CC_nocall, types: !7)
+!13 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_unsigned_char)
+!14 = !DILocation(line: 0, scope: !10)

[adrian-prantl]

The patch looks good, but it could use an explanation.

[adrian-prantl]

Can you add a comment explaining why?

[felipepiovezan]

There is a comment a few lines above, but sadly github doesn't show it. Does this address the concern?

[felipepiovezan]

Added a comment focusing on why dbg assign is different from the other intrinsics.