[PATCH] D75036: [DebugInfo] Describe call site values for chains of expression producing instrs

[Djordje Todorovic via Phabricator via llvm-commits]

djtodoro added a comment.

This looks good to me with the comments addressed. Thanks!



================
Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-chain.mir:116
+# CHECK-NEXT: DW_AT_location (DW_OP_reg2 W2)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19+700, DW_OP_plus_uconst 0x9, DW_OP_plus_uconst 0x50)
+
----------------
dstenb wrote:
> djtodoro wrote:
> > Have you tried debugging this with GDB/LLDB? I'm wondering if debuggers are missing the support for that.
> > 
> > These expressions seems valid to me, but I'm not sure even GCC generate call values with "combined" expressions so far.
> I unfortunately don't have an AArch64 setup I can run GDB on, so I have mostly been testing on our downstream target.
> 
> I now also tried this on x86-64 by using the following C code:
> 
> ```
> extern int call(int);
> 
> int foo = 1000;
> 
> int main() {
>   int local = foo + 789;
>   call(local);
>   return local;
> }
> ```
> 
> in which I changed the parameter setup code to the following (and described `ADD32ri` in `isAddImmediate()`):
> 
> ```
>     renamable $ebx = MOV32rm $rip, 1, $noreg, @foo, $noreg, debug-location !25 :: (dereferenceable load 4 from @foo, !tbaa !21)
>     $edi = MOV32rr $ebx                                                                                                                                                                                                                                                                                                        
>     renamable $edi = ADD32ri $edi, 700, implicit-def $eflags
>     renamable $edi = ADD32ri $edi, 80, implicit-def $eflags
>     renamable $edi = ADD32ri $edi, 9, implicit-def $eflags
> ```
> 
> resulting in the following expression:
> 
> ```
>                     DW_AT_location      (DW_OP_reg5 RDI)
>                     DW_AT_GNU_call_site_value   (DW_OP_breg3 RBX+700, DW_OP_constu 0xffffffff, DW_OP_and, DW_OP_plus_uconst 0x50, DW_OP_plus_uconst 0x9)
> ```
> 
> which prints fine in LLDB trunk:
> 
> ```
>    3   	int call(int param) {
>    4   	  side_effect = param;
>    5   	  __asm volatile("" : : : "rdi"); // clobber register.
> -> 6   	  return 0; // print param here.
>    7   	}
> (lldb) print param
> (int) $0 = 1789
> ```
> 
> and GDB 8.2.1:
> 
> ```
> (gdb) bt
> #0  call (param=param at entry=1789) at call.c:6
> #1  0x000000000020110a in main () at main.c:7
> ```
Nice!

I think this should have good impact on the number of call_site_params (`call site param DIEs` field from //llvm-dwarfdump stats// output) in general.


================
Comment at: llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-entry-value.mir:56
+  !17 = !DILocation(line: 0, scope: !12)
+  !18 = !DILocation(line: 4, scope: !12)
+  !19 = !DILocation(line: 5, scope: !12)
----------------
nit: We can attach to only `!17`, and get rid of the `!18` and `!19`.


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

https://reviews.llvm.org/D75036