[AsmPrinter][DwarfDebug] Skip vars with fragments in different location kinds

[fdeazeve]

Summary

The AsmPrinter currently assumes that a Debug Variable will have all of its
fragments with the same "kind" of location (i.e. all in the stack or all in
entry values). This is not enforced by the verifier, so it needs to be handled
properly. Until we do so, we conservatively drop one of the fragments.

[fdeazeve]

@scott.linder This is a patch I'd like to submit in the same area where your other patches are (https://reviews.llvm.org/D158675)
I don't mind rebasing if you think they will be merged soon!

[scott.linder]

In D159468#4639858, @fdeazeve wrote:

@scott.linder This is a patch I'd like to submit in the same area where your other patches are (https://reviews.llvm.org/D158675)
I don't mind rebasing if you think they will be merged soon!

I'm just running some final tests on our CI, and hopefully all goes well and they land today. Otherwise, I am also happy to rebase after you!

[fdeazeve]

In D159468#4641242, @scott.linder wrote:

In D159468#4639858, @fdeazeve wrote:

@scott.linder This is a patch I'd like to submit in the same area where your other patches are (https://reviews.llvm.org/D158675)
I don't mind rebasing if you think they will be merged soon!

I'm just running some final tests on our CI, and hopefully all goes well and they land today. Otherwise, I am also happy to rebase after you!

Awesome! I don't mind waiting an extra day or so! :)

[scott.linder]

In D159468#4641937, @fdeazeve wrote:

In D159468#4641242, @scott.linder wrote:

In D159468#4639858, @fdeazeve wrote:

@scott.linder This is a patch I'd like to submit in the same area where your other patches are (https://reviews.llvm.org/D158675)
I don't mind rebasing if you think they will be merged soon!

I'm just running some final tests on our CI, and hopefully all goes well and they land today. Otherwise, I am also happy to rebase after you!

Awesome! I don't mind waiting an extra day or so! :)

Thank you for waiting, and sorry it took through the weekend; I was out unexpectedly on Friday.

I just landed the series, ending with 43331461954939032a03621998c30ac90299ad40

Hopefully there are no regressions and it sticks, in which case you should be good to rebase now!

[fdeazeve]

In D159468#4643380, @scott.linder wrote:

In D159468#4641937, @fdeazeve wrote:

In D159468#4641242, @scott.linder wrote:

In D159468#4639858, @fdeazeve wrote:

@scott.linder This is a patch I'd like to submit in the same area where your other patches are (https://reviews.llvm.org/D158675)
I don't mind rebasing if you think they will be merged soon!

I'm just running some final tests on our CI, and hopefully all goes well and they land today. Otherwise, I am also happy to rebase after you!

Awesome! I don't mind waiting an extra day or so! :)

Thank you for waiting, and sorry it took through the weekend; I was out unexpectedly on Friday.

I just landed the series, ending with 43331461954939032a03621998c30ac90299ad40

Hopefully there are no regressions and it sticks, in which case you should be good to rebase now!

All good, thank you so much for driving this!

[fdeazeve]

Rebased

[dblaikie]

Probably worth a FIXME and a bug filed about supporting different location kinds? (like, really - we can't describe a variable as being partly in memory, and partly in a register? That doesn't sound right to me... I'd have thought we would've had support for that a while ago, but I don't follow the variable location parts of DWARF in as much detail, so maybe it is)

[fdeazeve]

In D159468#4646851, @dblaikie wrote:

Probably worth a FIXME and a bug filed about supporting different location kinds? (like, really - we can't describe a variable as being partly in memory, and partly in a register? That doesn't sound right to me... I'd have thought we would've had support for that a while ago, but I don't follow the variable location parts of DWARF in as much detail, so maybe it is)

The whole support for entry-value registers being stored in the MF side-table is new, and this multi-location scenario is something I had not anticipated. We'll get there though! But I agree that a FIXME comment is worth it!

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

for (const auto &VI : Asm->MF->getVariableDbgInfo()) {
     if (!Scope) {
       LLVM_DEBUG(dbgs() << "Dropping debug info for " << VI.Var->getName()
                         << ", no variable scope found\n");
       continue;
     }
 
     ensureAbstractEntityIsCreatedIfScoped(TheCU, Var.first, Scope->getScopeNode());
 
-    if (DbgVariable *DbgVar = MFVars.lookup(Var)) {
-      if (auto *MMI = std::get_if<Loc::MMI>(DbgVar))
-        MMI->addFrameIndexExpr(VI.Expr, VI.getStackSlot());
-      else
-        DbgVar->get<Loc::EntryValue>().addExpr(VI.getEntryValueRegister(),
+    // If we have already seen information for this variable, add to what we
+    // already know.
+    if (DbgVariable *PreviousLoc = MFVars.lookup(Var)) {
+      auto *PreviousMMI = std::get_if<Loc::MMI>(PreviousLoc);
+      auto *PreviousEntryValue = std::get_if<Loc::EntryValue>(PreviousLoc);
+      // Previous and new locations are both stack slots (MMI).
+      if (PreviousMMI && VI.inStackSlot())
+        PreviousMMI->addFrameIndexExpr(VI.Expr, VI.getStackSlot());
+      // Previous and new locations are both entry values.
+      else if (PreviousEntryValue && VI.inEntryValueRegister())
+        PreviousEntryValue->addExpr(VI.getEntryValueRegister(), *VI.Expr);
+      else {
+        // Locations differ, this should (rarely) happen in optimized async
+        // coroutines.
+        // Prefer whichever location has an EntryValue.
+        if (PreviousLoc->holds<Loc::MMI>())
+          PreviousLoc->emplace<Loc::EntryValue>(VI.getEntryValueRegister(),
                                                 *VI.Expr);
+        LLVM_DEBUG(dbgs() << "Dropping debug info for " << VI.Var->getName()
+                          << ", conflicting fragment location types\n");
+      }
       continue;
     }
 
     auto RegVar = std::make_unique<DbgVariable>(
                     cast<DILocalVariable>(Var.first), Var.second);
     if (VI.inStackSlot())
       RegVar->emplace<Loc::MMI>(VI.Expr, VI.getStackSlot());
     else

llvm/test/DebugInfo/AArch64/dbg-entry-value-swiftasync.mir

 # RUN: llc -start-after=machineverifier -O0 %s -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
 
 # CHECK:      DW_TAG_variable
 # CHECK-NEXT:   DW_AT_location        (DW_OP_GNU_entry_value(DW_OP_reg22 W22))
 # CHECK-NEXT:   DW_AT_name    ("a")
 # CHECK:      DW_TAG_variable
 # CHECK-NEXT:   DW_AT_location
 # CHECK-SAME:     DW_OP_GNU_entry_value(DW_OP_reg22 W22), DW_OP_piece 0x8,
 # CHECK-SAME:     DW_OP_GNU_entry_value(DW_OP_reg22 W22), DW_OP_plus_uconst 0x2a, DW_OP_piece 0x8)
 # CHECK-NEXT:   DW_AT_name    ("fragmented_var")
+# CHECK:      DW_TAG_variable
+# CHECK-NEXT:   DW_AT_location       (DW_OP_piece 0x8, DW_OP_GNU_entry_value(DW_OP_reg22 W22), DW_OP_piece 0x8)
+# CHECK-NEXT:   DW_AT_name    ("multi_type_fragmented_var")
 
 
 --- |
   target triple = "aarch64--"
   define void @foo(ptr %unused_arg, ptr swiftasync %async_arg) !dbg !4 {
+    %storage = alloca i64
     call void @llvm.dbg.declare(metadata ptr %async_arg, metadata !10, metadata !DIExpression(DW_OP_LLVM_entry_value, 1)), !dbg !12
     ; A two fragment variable.
     ; Fragments intentionally out of order to ensure the code can handle this.
     call void @llvm.dbg.declare(metadata ptr %async_arg, metadata !10, metadata !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 42, DW_OP_LLVM_fragment, 64, 64)), !dbg !12
     call void @llvm.dbg.declare(metadata ptr %async_arg, metadata !11, metadata !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_LLVM_fragment, 0, 64)), !dbg !12
+    ; A fragmented variable mixing entry values and stack slot locations.
+    call void @llvm.dbg.declare(metadata ptr %async_arg, metadata !13, metadata !DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_LLVM_fragment, 64, 64)), !dbg !12
+    call void @llvm.dbg.declare(metadata ptr %storage, metadata !13, metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)), !dbg !12
     ret void, !dbg !12
   }
   declare void @llvm.dbg.declare(metadata, metadata, metadata)
   declare void @consume(ptr)
   !llvm.dbg.cu = !{!0}
   !llvm.module.flags = !{!2, !3}
   !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
   !1 = !DIFile(filename: "x.c", directory: "/")
   !2 = !{i32 2, !"Dwarf Version", i32 4}
   !3 = !{i32 2, !"Debug Info Version", i32 3}
   !4 = distinct !DISubprogram(name: "f", scope: !1, file: !1, line: 1, type: !5, scopeLine: 1, spFlags: DISPFlagDefinition, unit: !0)
   !5 = !DISubroutineType(types: !6)
   !6 = !{null, !7, !7, !7}
   !7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 128)
   !9 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char)
   !10 = !DILocalVariable(name: "a", scope: !4, file: !1, line: 1, type: !7)
   !11 = !DILocalVariable(name: "fragmented_var", scope: !4, file: !1, line: 1, type: !7)
   !12 = !DILocation(line: 1, column: 37, scope: !4)
+  !13 = !DILocalVariable(name: "multi_type_fragmented_var", scope: !4, file: !1, line: 1, type: !7)
 ...
 ---
 name:            foo
 liveins:
   - { reg: '$x0', virtual-reg: '' }
   - { reg: '$x22', virtual-reg: '' }
 stack:
   - { id: 0, name: '', type: spill-slot, offset: -16, size: 8, alignment: 16,
       stack-id: default, callee-saved-register: '$lr', callee-saved-restored: true }
+  - { id: 1, name: '', type: default, offset: 0, size: 1, alignment: 8,
+      stack-id: default, callee-saved-register: '', callee-saved-restored: true,
+      debug-info-variable: '!13', debug-info-expression: '!DIExpression(DW_OP_LLVM_fragment, 0, 64)',
+      debug-info-location: '!12' }
+
 entry_values:
   - { entry-value-register: '$x22', debug-info-variable: '!10', debug-info-expression: '!DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_deref)',
       debug-info-location: '!12' }
   - { entry-value-register: '$x22', debug-info-variable: '!11', debug-info-expression: '!DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_plus_uconst, 42, DW_OP_deref, DW_OP_LLVM_fragment, 64, 64)',
       debug-info-location: '!12' }
   - { entry-value-register: '$x22', debug-info-variable: '!11', debug-info-expression: '!DIExpression(DW_OP_LLVM_entry_value, 1,  DW_OP_deref, DW_OP_LLVM_fragment, 0, 64)',
       debug-info-location: '!12' }
+  - { entry-value-register: '$x22', debug-info-variable: '!13', debug-info-expression: '!DIExpression(DW_OP_LLVM_entry_value, 1, DW_OP_deref, DW_OP_LLVM_fragment, 64, 64)',
+      debug-info-location: '!12' }
 body:             |
   bb.0 (%ir-block.0):
     liveins: $x0, $x22, $lr
     RET undef $lr, debug-location !12
 ...