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

[Scott Linder via Phabricator via llvm-commits]

scott.linder added inline comments.


================
Comment at: llvm/test/DebugInfo/X86/dbg_value_list_emission.mir:51-55
+    ; (1) Check a single reg arg works.
+    DBG_VALUE_LIST !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_stack_value), $eax, debug-location !15
+    ; CHECK:      DW_TAG_variable
+    ;  CHECK-NEXT:   (DW_OP_breg0 RAX+0, DW_OP_stack_value)
+    ;  CHECK-NEXT:   DW_AT_name ("locala")
----------------
Why does this behave differently than (what I understand to be) the equivalent `DBG_VALUE` ?

```
DBG_VALUE $eax, $noreg, !12, !DIExpression(DW_OP_stack_value), debug-location !15
; becomes: DW_AT_location (DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_stack_value)
```

It seems like the `DW_OP_and` is there to select a subregister (I assume EAX), but oddly it comes after the value of the register is already read (i.e. after the DW_OP_breg). I'm lost on what the intended behavior is, and why it differs between `DBG_VALUE` and `DBG_VALUE_LIST`.

There is also the existing confusion around the "isIndirect" flag in `DBG_VALUE` which makes these two equivalent (and both seemingly wrong):

```
DBG_VALUE $eax, $noreg, !25, !DIExpression(DW_OP_stack_value), debug-location !15
; becomes: DW_AT_location    (DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_stack_value)
  
DBG_VALUE $eax, 0, !26, !DIExpression(DW_OP_stack_value), debug-location !15
; becomes: DW_AT_location    (DW_OP_breg0 RAX+0, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_stack_value)
```

which makes it harder still to compare.


Would it be more straightforward to always be explicit about indirection in the new form? Why does `DW_OP_stack_value` imply a `DW_OP_deref` at all? I.e. why do we not get:

```
DBG_VALUE_LIST !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_stack_value), $eax, debug-location !15
    ; CHECK:      DW_TAG_variable
    ;  CHECK-NEXT:   (DW_OP_reg RAX, DW_OP_stack_value)
    ;  CHECK-NEXT:   DW_AT_name ("locala")
```

which in this case I imagine would just be an error. I would expect the correct expression to generate the `DW_OP_breg` would be something like:

```
DBG_VALUE_LIST !12, !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_deref, DW_OP_stack_value), $eax, debug-location !15
    ; CHECK:      DW_TAG_variable
    ;  CHECK-NEXT:   (DW_OP_breg0 RAX+0, DW_OP_stack_value)
    ;  CHECK-NEXT:   DW_AT_name ("locala")
```

If we don't do this, we seem to retain some of the same ambiguity that makes the old "isIndirect" field so confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83495