[PATCH] D69886: [DebugInfo] Support for DW_OP_implicit_pointer (Post IR transformation phase)

[Alok Kumar Sharma via Phabricator via llvm-commits]

alok marked 4 inline comments as done.
alok added inline comments.


================
Comment at: llvm/test/DebugInfo/dwarfdump-implicit_pointer_mem2reg.ll:7
+; 1. Test if More than one member location list is printed 
+;    (for pointer to scalar)
+; CHECK:       DW_AT_location        (
----------------
vsk wrote:
> Please only capitalize words where necessary.
Sure. I shall incorporate this in next version.


================
Comment at: llvm/test/DebugInfo/dwarfdump-implicit_pointer_mem2reg.ll:9
+; CHECK:       DW_AT_location        (
+; CHECK-NEXT:     : DW_OP_lit0, DW_OP_stack_value
+; CHECK-NEXT:     : DW_OP_implicit_pointer 0x53 +0
----------------
vsk wrote:
> "arr1" is metadata !18, right? Where does lit0 come from?
In the source program -

  int *ptr2 = 0;

In IR it is represented as - 

  call void @llvm.dbg.value(metadata i32* null, metadata !24, metadata !DIExpression()), !dbg !30
  !24 = !DILocalVariable(name: "ptr2", scope: !12, file: !3, line: 51, type: !23)

Without current patch DIE for ptr2 would look like -

0x00000068:     DW_TAG_variable
                  DW_AT_const_value     (0)
                  DW_AT_name    ("ptr2")
                  DW_AT_decl_file       ("/home/alok/mem2reg.c")
                  DW_AT_decl_line       (8)
                  DW_AT_type    (0x000000ab "int*")

* Please note that ptr2 is initialized to null (DW_AT_const_value     (0))

With current patch -

0x00000068:     DW_TAG_variable
                  DW_AT_location        (0x00000019
                     [0x0000000000400480, 0x0000000000400487): DW_OP_lit0, DW_OP_stack_value
                     [0x0000000000400487, 0x0000000000400494): DW_OP_implicit_pointer 0x53 +0)
                  DW_AT_name    ("ptr2")
                  DW_AT_decl_file       ("Inputs/dwarfdump-implicit_pointer_mem2reg.c")
                  DW_AT_decl_line       (8)
                  DW_AT_type    (0x000000c1 "int*")

* Please note that, now DIE has two values (in different code ranges), so in place of initializing it uses Location list.


================
Comment at: llvm/test/DebugInfo/dwarfdump-implicit_pointer_mem2reg.ll:11
+; CHECK-NEXT:     : DW_OP_implicit_pointer 0x53 +0
+; CHECK-NEXT:  DW_AT_name    ("ptr2")
+
----------------
vsk wrote:
> Please structure your checks as:
> 
> ```
> ; <description of test 1>
> ; CHECK-LABEL: DW_AT_name  (<variable name>)
> ; CHECK-NEXT: ...
> 
> ; <description of test 2>
> ...
> ```
Thanks for the input, I will incorporate this in next version.


================
Comment at: llvm/test/DebugInfo/dwarfdump-implicit_pointer_sroa_inline.ll:9
+; CHECK-NEXT:    DW_AT_location
+; CHECK-NEXT:     : DW_OP_implicit_pointer 0x5d +0
+; CHECK-NEXT:     : DW_OP_implicit_pointer 0x5d +4)
----------------
vsk wrote:
> Please avoid hardcoding constants in check lines. If the specific constant isn't important, don't match it. Otherwise, you can capture it with a filecheck variable, e.g.:
> 
> ```
> DW_OP_implicit_pointer [[DIE:0x.+]] +0
> DW_OP_implicit_pointer [[DIE]] +4
> ```
Thanks for the input. I shall incorporate this in next version.


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

https://reviews.llvm.org/D69886