[PATCH] D83495: [DebugInfo] Add DWARF emission for DBG_VALUE_LIST

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Hey, just noticed a couple of comments to remove from the tests.



================
Comment at: llvm/test/DebugInfo/X86/dbg_value_list_clobbers.mir:41
+  bb.0.entry:
+    ; XXX NOTE that I've added an explicit DW_OP_stack_value to the expressions. REMOVE THESE when the
+    ;          implied operator is added to the expressions automatically.
----------------
You can remove this XXX note.


================
Comment at: llvm/test/DebugInfo/X86/dbg_value_list_clobbers.mir:67
+    DBG_VALUE_LIST !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value), $eax, $ecx, debug-location !15
+    ; XXX THIS CHECK FAILS
+    ; CHECK-NEXT: [{{.*}}): DW_OP_breg0 RAX+0, DW_OP_breg2 RCX+0, DW_OP_plus, DW_OP_stack_value
----------------
I assume this no longer fails?


================
Comment at: llvm/test/DebugInfo/X86/dbg_value_list_emission.mir:63
+    ; (3) Check that multiple references to one reg arg works.
+    ;     XXX What was the consensus on this - are we allowing it?
+    DBG_VALUE_LIST !25, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 0, DW_OP_minus, DW_OP_stack_value), $eax, debug-location !15
----------------
Can remove this XXX note. As you mentioned offline, having multiple references to the same arg (i.e. multiple `DW_OP_LLVM_arg, 0` in the expr) is never a problem.

Though, slightly tangentially, I'm still a little unclear on what the final decision was on how to handle duplicate register arg operands. In D82363 you said 'always treat DBG_VALUE_LISTs as potentially having them'. Please could you explain a little further? (i.e. is it an error state, do we need to add extra checks when dealing with DBG_VALUE_LISTs etc).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83495/new/

https://reviews.llvm.org/D83495